Re: Review request: 8003562: Provide a command-line tool to find static dependencies
Looks good. /Erik On 2012-12-07 22:55, Mandy Chung wrote: This is the updated webrev. It includes Erik's makefile changes and small change - be consistent with javap, jdeps can take classnames as the input argument or wildcard * to analyze all class files that replaces the -all option. Webrev: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/jdeps/webrev.03/ Mandy On 12/5/12 5:36 PM, Mandy Chung wrote: This review request (add a new jdeps tool in the JDK) include changes in langtools and the jdk build. I would need reviewers from the langtools and the build-infra team. Webrev for review: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/jdeps/langtools-webrev.02/ http://cr.openjdk.java.net/~mchung/jdk8/webrevs/jdeps/jdk-webrev.02/ The jdeps source is in the langtools repo. The change in the jdk repo is to add the new jdeps executable. I made change in the old and new build for the addition of this new jdeps tool. I discussed with Jon whether I should modify langtools/make/build.xml to add a new jdeps target. Since the old build will be replaced by the new build soon, I simply add com/sun/tools/jdeps as part of javap.includes. Alan, The -d option is kept as a hidden option and use as implementation. We can remove it when it's determined not useful in the future. I also rename -v:summary to -summary. Thanks Mandy $ jdep -h Usage: jdeps options files where possible options include: -version Version information -classpath pathSpecify where to find class files -summary Print dependency summary only -v:class Print class-level dependencies -v:package Print package-level dependencies -p package nameRestrict analysis to classes in this package (may be given multiple times) -e regex Restrict analysis to packages matching pattern (-p and -e are exclusive) -P --profileShow profile or the file containing a package -R --recursive Traverse all dependencies recursively -all Process all classes specified in -classpath $ jdep Notepad.jar Ensemble.jar Notepad.jar - D:\tools\devtools\jdk8\windows-i586\jre\lib\rt.jar unnamed (Notepad.jar) - java.awt - java.awt.event - java.beans - java.io - java.lang - java.net - java.util - java.util.logging - javax.swing - javax.swing.border - javax.swing.event - javax.swing.text - javax.swing.tree - javax.swing.undo Ensemble.jar - D:\tools\devtools\jdk8\windows-i586\jre\lib\jfxrt.jar Ensemble.jar - D:\tools\devtools\jdk8\windows-i586\jre\lib\rt.jar com.javafx.main (Ensemble.jar) - java.applet - java.awt - java.awt.event - java.io - java.lang - java.lang.reflect - java.net - java.security - java.util - java.util.jar - javax.swing - sun.misc JDK internal API (rt.jar)
Re: Request for Review : CR#8004015 : [final (?) pass] Add interface extends and defaults for basic functional interfaces
On 07/12/2012 21:02, Mike Duigou wrote: ... 5) I agree with David, NPE should be defined where applicable. I am adding these though I am still somewhat resistant for reasons I will mention in next review cycle thread I don't want to to get bogged down in a debate over this, I'm really happy too see these changes making their way into jdk8. Maybe some text in the package description that indicates that an exception will be thrown in cases where the API specifies a non-null argument? I'll leave it to you to decide. All in all, this is looking good. -Chris. Mike
Re: RFR: 8003246: Add Supplier to ThreadLocal
On 07/12/2012 15:46, Remi Forax wrote:
On 12/07/2012 04:34 PM, Chris Hegarty wrote:
I filed 8004707: Remove superfluous access$000 methods in java.util,
to track this issue. I can file a separate issue for java.lang.
yes, please do that.
I filed 8004797: Remove superfluous access$000 methods in java.lang
-Chris.
I'm sure there are other ways, but a simple find reports them:
:pwd
build/solaris-i586/classes/java/util
: find . -name *.class -exec javap -v {} \; | grep '\.access\$00'
Technically, javac also generate constructor accessors that doesn't
start by access00 but this should catch most of the generated accessors.
once changeset for 8003246 will be pushed, I will send two patches one
for java.lang and one for java.util.
-Chris
Rémi
On 07/12/2012 14:27, Doug Lea wrote:
On 12/06/12 18:44, Remi Forax wrote:
BTW, at some point, it will be a good idea to get ride of all these
method
access$000 in java.lang/java.util at least.
Maybe this is out of scope for this CR, but while in the
neighborhood of ThreadLocal, it could be done here.
There are a couple of annoying ones that kick in on any
compile of anything involving ThreadLocals (try running with
-XX:+PrintCompilation). To remove, replace private with
final for methods, and just kill private for fields.
-Doug
Re: RFR: JDK-8003890: modify test scripts to pass VMOPTIONS
On 10/12/2012 11:13, Chris Hegarty wrote: Thank you Mark, this is a really useful contribution. I noticed that a few tests (below) specify either -client or -server. If the vmoptions contain either of these arguments, and this seems to be the way our test/Makefile is passing them, then it may result in the test using the wrong vm. I think we should remove TESTVMOPTS from these tests, but then we lose other potential opts, like heap size, etc. java/rmi/reliability/benchmark/runRmiBench.sh java/rmi/reliability/benchmark/runSerialBench.sh sun/management/jmxremote/startstop/JMXStartStopTest.sh You're right, this will cause conflict. In the case of JMXStartStopTest.sh and runSerialBench.sh then it might be better to remove the -server. Stuart might have an opinion on runRmiBench.sh but one idea is to change is so that when run interactively (TESTJAVA not set) then it works as it does now and runs the benchmark twice. If TESTJAVA is not set then it runs the benchmark once, with whatever VM options are in use. -Alan
Re: Review needed: 8004374 : Fwd: JDBC bug: Incorrect number of conflicts is reported by CachedRowSetWriter.writeData
Hi Frank, As I explained in one of my earlier emails, tests that require a database will not be added to jtreg. I have a unit test suite which i use for that but that is not external Best Lance On Dec 10, 2012, at 2:44 AM, Frank Ding wrote: Hi Lance, The code refactory looks good. By the way, the newly added unit test is not jtreg test case? Best regards, Frank On 12/5/2012 4:38 AM, Lance Andersen - Oracle wrote: All, Attached is the patch for: 8004374 based off the issue that Frank reported. for http://cr.openjdk.java.net/~lancea/8004374/webrev.00/ http://cr.openjdk.java.net/%7Elancea/8004374/webrev.00/ The TCK, SQE and the JDBC Unit Tests run clean. I added a new Unit Test to validate the issue. Frank, I did not use your fix as I was able to clean the code up a bit more and get rid of more crud while addressing it. It is similar though. Best Lance http://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com mailto:lance.ander...@oracle.com Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
hg: jdk8/tl/langtools: 8004094: Javac compiler error - synthetic method accessor generated with duplicate name
Changeset: c78acf6c2f3e Author:mcimadamore Date: 2012-12-10 12:10 + URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/c78acf6c2f3e 8004094: Javac compiler error - synthetic method accessor generated with duplicate name Summary: method clash check logic should skip methods marked with ACC_SYNTHETIC Reviewed-by: jjg ! src/share/classes/com/sun/tools/javac/comp/Check.java + test/tools/javac/generics/8004094/B.java + test/tools/javac/generics/8004094/T8004094.java
Re: RFR: 8001647: In-place methods on Collection/List
On 08/12/2012 01:42, Akhil Arora wrote: As part of the Library Lambdafication, this patch adds the following default methods to Collections - Iterable.forEach(BlockT) Collection.removeAll(PredicateT) List.sort(Comparator) List.replaceAll(UnaryOperatorT) It also provides more efficient implementations of these methods for ArrayList, Vector and CopyOnWriteArrayList. Please review. http://cr.openjdk.java.net/~akhil/8001647.1/webrev/ Thanks to the many people who have already contributed to this patch. This may be bikeshed territory but we usually don't use the public modifier on methods defined by interfaces as they are public anyway. It seems inconsistent to me to have it on the default methods. Perhaps this has been discussed before, in which case ignore this. BTW: The only reason I'm bringing this up is because there are lots of default methods to come and it would be nice to establish a convention and consistency from the start. One small comment on ArrayList.removeAll is that it might be more efficient to keep a count of the number of elements to remove rather than using the cardinality method (to avoid the bit scan). For the supporting classes in testlibrary then is @library needed? (I haven't fully groked how jtreg supports TestNG so perhaps @library without specifying a location or JAR file has some meaning). -Alan.
Re: Proposal: Fully Concurrent ClassLoading
On 12/09/2012 10:03 PM, David Holmes wrote: Hi David, On 7/12/2012 1:06 AM, David M. Lloyd wrote: On 12/06/2012 05:35 AM, Alan Bateman wrote: On 05/12/2012 11:59, David Holmes wrote: Java 7 introduced support for parallel classloading by adding to each class loader a ConcurrentHashMap, referenced through a new field, parallelLockMap. This contains a mapping from class names to Objects to use as a classloading lock for that class name. This scheme has a number of inefficiencies. To address this we propose for Java 8 the notion of a fully concurrent classloader ... This is a fairly simple proposal that I've written up as a blog entry: https://blogs.oracle.com/dholmes/entry/parallel_classloading_revisited_fully_concurrent The jdk7 implementation is very unfortunate, it's a pity this wasn't caught before 7 shipped. I think the proposal is good, it preserves compatibility, and if there are loaders calling registerAsParallelCapable now (probably very few) then it they may be able to change to using registerAsFullyConcurrent without too much work. I am tempted to suggest that registerAsParallelCapable should be deprecated too. I'm sorry I missed the original post, and I just want to add my wholehearted support for this idea. Our application server's class loader implementation can be configured (on certain JVMs) to run in a lock-free mode and we have seen good performance and memory footprint as a result on these particular JVMs. That sounds promising. Are you in a position to try out this proposal on a testbed with your app server? Sure, I'd love to. What tree should I build? -- - DML
Re: RFR: 8003246: Add Supplier to ThreadLocal
On 12/10/2012 11:51 AM, Chris Hegarty wrote: On 07/12/2012 15:46, Remi Forax wrote: On 12/07/2012 04:34 PM, Chris Hegarty wrote: I filed 8004707: Remove superfluous access$000 methods in java.util, to track this issue. I can file a separate issue for java.lang. yes, please do that. I filed 8004797: Remove superfluous access$000 methods in java.lang -Chris. thanks. Rémi
Re: bottleneck by java.lang.Class.getAnnotations() - rebased patch
Hi David,
It would be informative to look at how java.lang.Class evolved during
history. The oldest revision I can access is from 1st of Dec. 2007,
which already contains Java 1.5 code (annotations) and is more or less
unchanged until now. The most interesting changesets would be those that
introduced annotations.
Regards, Peter
On 12/10/2012 03:59 PM, Peter Levart wrote:
On 12/10/2012 07:18 AM, David Holmes wrote:
Hi Peter,
Sorry for the delay on this.
Thank you for taking it into consideration. I can only imagine how
busy you are with other things.
Generally your VolatileData and my ReflectionHelper are doing a
similar job. But I agree with your reasoning that all of the cached
SoftReferences are likely to be cleared at once, and so a
SoftReference to a helper object with direct references, is more
effective than a direct reference to a helper object with
SoftReferences. My initial stance with this was very conservative as
the more change that is introduced the more uncertainty there is
about the impact.
I say the above primarily because I think, if I am to proceed with
this, I will need to separate out the general reflection caching
changes from the annotation changes. There are a number of reasons
for this:
First, I'm not at all familiar with the implementation of annotations
at the VM or Java level, and the recent changes in this area just
exacerbate my ignorance of the mechanics. So I don't feel qualified
to evaluate that aspect.
Second, the bulk of the reflection caching code is simplified by the
fact that due to current constraints on class redefinition the
caching is effectively idempotent for fields/methods/constructors.
But that is not the case for annotations.
I think that on the Class-level these two aspects can be separated.
But not on the Field/Method/Constructor level, because annotations are
referenced by Field/Method/Constructor objects and there is no
invalidation logic - like on the Class-level - that would just
invalidate the sets of annotations on Field/Method/Constructor,
leaving Field/Method/Constructor objects still valid. So these two
aspects are connected - it may be - I haven't looked at history yet -
that invalidation of Field/Method/Constructor was introduced just
because of annotations.
Also, the following bug (currently not accessible):
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5002251 has
references to docs that suggest that current class redefinition can
introduce new methods, so If this is correct, then redefinition is not
idempotent for basic reflection data.
Finally, the use of synchronization with the annotations method is
perplexing me. I sent Joe a private email on this but I may as well
raise it here - and I think you have alluded to this in your earlier
emails as well: initAnnotationsIfNecessary() is a synchronized
instance method but I can not find any other code in the VM that
synchronizes on the Class object's monitor. So if this
synchronization is trying to establish consistency in the face of
class redefinition, I do not see where class redefinition is
participating in the synchronization!
I think that the intent was merely synchronized access to / lazy
initialization of cached 'annotations' and 'declaredAnnotations' Maps.
Field[], Method[], Constructor[] arrays are published to other threads
via volatile fields one field at a time, but
initAnnotationsIfNecessary() was designed to publish two references
('annotations' and 'declaredAnnotations') atomically at the same time,
so the author of the code choose to use synchronized block. I also
have a feeling that this might have simply been the author's preferred
style of synchronization, since the same approach is used also in
Field/Method/Constructor/Executable although there's just one field of
annotations that is published at a time.
It is indicative that initAnnotationsIfNecessary() synchronized method
also contains the call to clearCachesOnClassRedefinition(), so at
first it seems that the synchronization is also meant to serialize
access to invalidation logic which invalidates both
Field/Method/Constructor and annotation fields, but that all
falls-apart because clearCachesOnClassRedefinition() is also called
from elsewhere, not guarded by the synchronization.
So all in all the two aspects - annotations and basic reflection stuff
- are quite intermingled in current code, unfortunately very
inconsistently. I'm afraid that doing one thing and not touching the
other is possible, but that would not solve the problems that this
thread was starting to address (bottleneck by
java.lang.Class.getAnnotations()) and evident dead-lock bugs.
We can approach the problem so that we separate the aspects of caching
Class-level annotations and Field/Method/Constructor arrays but using
the same approach for both. And that would only make sense if there
was a reason to separately cache Class-level annotations and
Re: bottleneck by java.lang.Class.getAnnotations() - rebased patch
Ok, I've got it. Downloaded j2sdk1.4_19 and unpacked src.zip ...
There are SoftReferences for individual Field/Method/Constructor arrays:
// Caches for certain reflective results
private static boolean useCaches = true;
private volatile transient SoftReference declaredFields;
private volatile transient SoftReference publicFields;
private volatile transient SoftReference declaredMethods;
private volatile transient SoftReference publicMethods;
private volatile transient SoftReference declaredConstructors;
private volatile transient SoftReference publicConstructors;
// Intermediate results for getFields and getMethods
private volatile transient SoftReference declaredPublicFields;
private volatile transient SoftReference declaredPublicMethods;
...but there's no mechanism yet for invalidation. So my assumption that
invalidation was introduced just because of annotations might be
correct. Annotations were just glued on top of the existing unchanged
structures.
Regards, Peter
P.S. I've got an Idea how to approach another variant where Class-level
annotations would still be directly referenced from the Class instance
and Field/Method/Constructor arrays would hang off a single
SoftReference in a way that would have same space efficiency that my
current integrated approach. The idea is to subclass the SoftReference
and have the additional fields on it:
static class VolatileData extends SoftReferenceMemberData {
volatile Map annotations, declaredAnnotations;
final int redefinedCount;
}
...and MemberData only containing volatile Fileld[], Method[],
Constructor[] fields...
I also know how to correctly synchronize access to this structure so
that annotations are not invalidated when SoftReference is cleared. Let
this be variant a2).
I rest now and wait for your pointers.
Regards, Peter
On 12/10/2012 05:13 PM, Peter Levart wrote:
Hi David,
It would be informative to look at how java.lang.Class evolved during
history. The oldest revision I can access is from 1st of Dec. 2007,
which already contains Java 1.5 code (annotations) and is more or less
unchanged until now. The most interesting changesets would be those
that introduced annotations.
Regards, Peter
On 12/10/2012 03:59 PM, Peter Levart wrote:
On 12/10/2012 07:18 AM, David Holmes wrote:
Hi Peter,
Sorry for the delay on this.
Thank you for taking it into consideration. I can only imagine how
busy you are with other things.
Generally your VolatileData and my ReflectionHelper are doing a
similar job. But I agree with your reasoning that all of the cached
SoftReferences are likely to be cleared at once, and so a
SoftReference to a helper object with direct references, is more
effective than a direct reference to a helper object with
SoftReferences. My initial stance with this was very conservative as
the more change that is introduced the more uncertainty there is
about the impact.
I say the above primarily because I think, if I am to proceed with
this, I will need to separate out the general reflection caching
changes from the annotation changes. There are a number of reasons
for this:
First, I'm not at all familiar with the implementation of
annotations at the VM or Java level, and the recent changes in this
area just exacerbate my ignorance of the mechanics. So I don't feel
qualified to evaluate that aspect.
Second, the bulk of the reflection caching code is simplified by the
fact that due to current constraints on class redefinition the
caching is effectively idempotent for fields/methods/constructors.
But that is not the case for annotations.
I think that on the Class-level these two aspects can be separated.
But not on the Field/Method/Constructor level, because annotations
are referenced by Field/Method/Constructor objects and there is no
invalidation logic - like on the Class-level - that would just
invalidate the sets of annotations on Field/Method/Constructor,
leaving Field/Method/Constructor objects still valid. So these two
aspects are connected - it may be - I haven't looked at history yet -
that invalidation of Field/Method/Constructor was introduced just
because of annotations.
Also, the following bug (currently not accessible):
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5002251 has
references to docs that suggest that current class redefinition can
introduce new methods, so If this is correct, then redefinition is
not idempotent for basic reflection data.
Finally, the use of synchronization with the annotations method is
perplexing me. I sent Joe a private email on this but I may as well
raise it here - and I think you have alluded to this in your earlier
emails as well: initAnnotationsIfNecessary() is a synchronized
instance method but I can not find any other code in the VM that
synchronizes on the Class object's monitor. So if this
synchronization is trying to establish consistency in the face of
Re: Request for review: 8000525: Java.net.httpcookie api does not support 2-digit year format
Shouldn't 'cal.set(1970, 0, 1, 0, 0, 0)' be inside the for loop? The test can simply throw Exception, rather can catching. Otherwise, looks fine to me. -Crhis. On 06/12/2012 21:19, Rob McKenna wrote: Hi folks, According to HttpCookie.java: There are 3 http cookie specifications: Netscape draft RFC 2109 -/http://www.ietf.org/rfc/rfc2109.txt/ RFC 2965 -/http://www.ietf.org/rfc/rfc2965.txt/ HttpCookie class can accept all these 3 forms of syntax. According to http://www.ietf.org/rfc/rfc2109.txt section 10.1.2: Netscape's original proposal defined an Expires header that took a date value in a fixed-length variant format in place of Max-Age: Wdy, DD-Mon-YY HH:MM:SS GMT Thats in the Historical section provided to allow for compatibility with Netscape's implementation, which we also support: (as per http://docs.oracle.com/javase/6/docs/api/java/net/HttpCookie.html ) While we don't support the rfc explicitly, this change follows the format specified in section 5.1.1 of rfc 6265: 3. If the year-value is greater than or equal to 70 and less than or equal to 99, increment the year-value by 1900. 4. If the year-value is greater than or equal to 0 and less than or equal to 69, increment the year-value by 2000. 1. NOTE: Some existing user agents interpret two-digit years differently. The webrev is at: http://cr.openjdk.java.net/~robm/8000525/webrev.01/ http://cr.openjdk.java.net/%7Erobm/8000525/webrev.01/ Note: The addition of setLenient(false) has required changes to two existing testcases. -Rob
Re: CFR: javax.xml.parsers: Using ServiceLoader to load JAXP parser factories (7169894: JAXP Plugability Layer: using service loader)
Hi, After further discussion with Joe Alan, I changed the call to ServiceLoader to simply return the first provider it finds, or null. This is closer to what was present in JDK 7 - and looping is not necessary in JDK 8 since the default implementation is not returned as a service provider. So here is a new - and hopefully simpler webrev: http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.parsers/webrev.04/ best regards, -- daniel On 12/7/12 7:37 PM, Mandy Chung wrote: On 12/7/12 8:32 AM, Alan Bateman wrote: On 07/12/2012 15:15, Daniel Fuchs wrote: Hi Alan, I have updated the webrev according to your suggestion. I think it makes things much clearer. The new version is there: http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.parsers/webrev.03/ This looks good to me except that implementation system default should be system-default implementation. Looks good to me too with the change to system-default implementation. Mandy
Re: CFR: javax.xml.parsers: Using ServiceLoader to load JAXP parser factories (7169894: JAXP Plugability Layer: using service loader)
Hi Daniel, Looks good! Joe On 12/10/2012 9:12 AM, Daniel Fuchs wrote: Hi, After further discussion with Joe Alan, I changed the call to ServiceLoader to simply return the first provider it finds, or null. This is closer to what was present in JDK 7 - and looping is not necessary in JDK 8 since the default implementation is not returned as a service provider. So here is a new - and hopefully simpler webrev: http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.parsers/webrev.04/ best regards, -- daniel On 12/7/12 7:37 PM, Mandy Chung wrote: On 12/7/12 8:32 AM, Alan Bateman wrote: On 07/12/2012 15:15, Daniel Fuchs wrote: Hi Alan, I have updated the webrev according to your suggestion. I think it makes things much clearer. The new version is there: http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.parsers/webrev.03/ This looks good to me except that implementation system default should be system-default implementation. Looks good to me too with the change to system-default implementation. Mandy
Re: bottleneck by java.lang.Class.getAnnotations() - rebased patch
Yes; the OpenJDK sources only go back to circa 2007, which was part-way
into the JDK 7 release. The exact changesets of how annotations were
added back in JDK 5 are not available publicly. However, the final
results of that process may be mostly visible in the src.zip from JDK 5.
HTH,
-Joe
On 12/10/2012 8:13 AM, Peter Levart wrote:
Hi David,
It would be informative to look at how java.lang.Class evolved during
history. The oldest revision I can access is from 1st of Dec. 2007,
which already contains Java 1.5 code (annotations) and is more or less
unchanged until now. The most interesting changesets would be those
that introduced annotations.
Regards, Peter
On 12/10/2012 03:59 PM, Peter Levart wrote:
On 12/10/2012 07:18 AM, David Holmes wrote:
Hi Peter,
Sorry for the delay on this.
Thank you for taking it into consideration. I can only imagine how
busy you are with other things.
Generally your VolatileData and my ReflectionHelper are doing a
similar job. But I agree with your reasoning that all of the cached
SoftReferences are likely to be cleared at once, and so a
SoftReference to a helper object with direct references, is more
effective than a direct reference to a helper object with
SoftReferences. My initial stance with this was very conservative as
the more change that is introduced the more uncertainty there is
about the impact.
I say the above primarily because I think, if I am to proceed with
this, I will need to separate out the general reflection caching
changes from the annotation changes. There are a number of reasons
for this:
First, I'm not at all familiar with the implementation of
annotations at the VM or Java level, and the recent changes in this
area just exacerbate my ignorance of the mechanics. So I don't feel
qualified to evaluate that aspect.
Second, the bulk of the reflection caching code is simplified by the
fact that due to current constraints on class redefinition the
caching is effectively idempotent for fields/methods/constructors.
But that is not the case for annotations.
I think that on the Class-level these two aspects can be separated.
But not on the Field/Method/Constructor level, because annotations
are referenced by Field/Method/Constructor objects and there is no
invalidation logic - like on the Class-level - that would just
invalidate the sets of annotations on Field/Method/Constructor,
leaving Field/Method/Constructor objects still valid. So these two
aspects are connected - it may be - I haven't looked at history yet -
that invalidation of Field/Method/Constructor was introduced just
because of annotations.
Also, the following bug (currently not accessible):
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5002251 has
references to docs that suggest that current class redefinition can
introduce new methods, so If this is correct, then redefinition is
not idempotent for basic reflection data.
Finally, the use of synchronization with the annotations method is
perplexing me. I sent Joe a private email on this but I may as well
raise it here - and I think you have alluded to this in your earlier
emails as well: initAnnotationsIfNecessary() is a synchronized
instance method but I can not find any other code in the VM that
synchronizes on the Class object's monitor. So if this
synchronization is trying to establish consistency in the face of
class redefinition, I do not see where class redefinition is
participating in the synchronization!
I think that the intent was merely synchronized access to / lazy
initialization of cached 'annotations' and 'declaredAnnotations'
Maps. Field[], Method[], Constructor[] arrays are published to other
threads via volatile fields one field at a time, but
initAnnotationsIfNecessary() was designed to publish two references
('annotations' and 'declaredAnnotations') atomically at the same
time, so the author of the code choose to use synchronized block. I
also have a feeling that this might have simply been the author's
preferred style of synchronization, since the same approach is used
also in Field/Method/Constructor/Executable although there's just one
field of annotations that is published at a time.
It is indicative that initAnnotationsIfNecessary() synchronized
method also contains the call to clearCachesOnClassRedefinition(), so
at first it seems that the synchronization is also meant to serialize
access to invalidation logic which invalidates both
Field/Method/Constructor and annotation fields, but that all
falls-apart because clearCachesOnClassRedefinition() is also called
from elsewhere, not guarded by the synchronization.
So all in all the two aspects - annotations and basic reflection
stuff - are quite intermingled in current code, unfortunately very
inconsistently. I'm afraid that doing one thing and not touching the
other is possible, but that would not solve the problems that this
thread was starting to address (bottleneck by
Re: bottleneck by java.lang.Class.getAnnotations() - rebased patch
Hi David, Joe,
I unpacked the src.zip of the first release of JDK 1.5.0. Here's the
relevant part:
private transient MapClass, Annotation annotations;
private transient MapClass, Annotation declaredAnnotations;
private synchronized void initAnnotationsIfNecessary() {
if (annotations != null)
return;
declaredAnnotations = AnnotationParser.parseAnnotations(
getRawAnnotations(), getConstantPool(), this);
Class? superClass = getSuperclass();
if (superClass == null) {
annotations = declaredAnnotations;
} else {
annotations = new HashMapClass, Annotation();
superClass.initAnnotationsIfNecessary();
for (Map.EntryClass, Annotation e :
superClass.annotations.entrySet()) {
Class annotationClass = e.getKey();
if
(AnnotationType.getInstance(annotationClass).isInherited())
annotations.put(annotationClass, e.getValue());
}
annotations.putAll(declaredAnnotations);
}
}
...there's no sign of invalidation logic here yet. Neither for
annotations nor for fields/methods/constructors. This appears to have
been added later, presumably to accommodate class redefinition changing
annotations.
I would also like to see the source of AnnotationType to see if it used
the same logic and synchronization, but that's not part of src.zip
sources...
Regards, Peter
On 12/10/2012 09:52 PM, Joe Darcy wrote:
Yes; the OpenJDK sources only go back to circa 2007, which was
part-way into the JDK 7 release. The exact changesets of how
annotations were added back in JDK 5 are not available publicly.
However, the final results of that process may be mostly visible in
the src.zip from JDK 5.
HTH,
-Joe
On 12/10/2012 8:13 AM, Peter Levart wrote:
Hi David,
It would be informative to look at how java.lang.Class evolved during
history. The oldest revision I can access is from 1st of Dec. 2007,
which already contains Java 1.5 code (annotations) and is more or
less unchanged until now. The most interesting changesets would be
those that introduced annotations.
Regards, Peter
On 12/10/2012 03:59 PM, Peter Levart wrote:
On 12/10/2012 07:18 AM, David Holmes wrote:
Hi Peter,
Sorry for the delay on this.
Thank you for taking it into consideration. I can only imagine how
busy you are with other things.
Generally your VolatileData and my ReflectionHelper are doing a
similar job. But I agree with your reasoning that all of the cached
SoftReferences are likely to be cleared at once, and so a
SoftReference to a helper object with direct references, is more
effective than a direct reference to a helper object with
SoftReferences. My initial stance with this was very conservative
as the more change that is introduced the more uncertainty there is
about the impact.
I say the above primarily because I think, if I am to proceed with
this, I will need to separate out the general reflection caching
changes from the annotation changes. There are a number of reasons
for this:
First, I'm not at all familiar with the implementation of
annotations at the VM or Java level, and the recent changes in this
area just exacerbate my ignorance of the mechanics. So I don't feel
qualified to evaluate that aspect.
Second, the bulk of the reflection caching code is simplified by
the fact that due to current constraints on class redefinition the
caching is effectively idempotent for fields/methods/constructors.
But that is not the case for annotations.
I think that on the Class-level these two aspects can be separated.
But not on the Field/Method/Constructor level, because annotations
are referenced by Field/Method/Constructor objects and there is no
invalidation logic - like on the Class-level - that would just
invalidate the sets of annotations on Field/Method/Constructor,
leaving Field/Method/Constructor objects still valid. So these two
aspects are connected - it may be - I haven't looked at history yet
- that invalidation of Field/Method/Constructor was introduced just
because of annotations.
Also, the following bug (currently not accessible):
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5002251 has
references to docs that suggest that current class redefinition can
introduce new methods, so If this is correct, then redefinition is
not idempotent for basic reflection data.
Finally, the use of synchronization with the annotations method is
perplexing me. I sent Joe a private email on this but I may as well
raise it here - and I think you have alluded to this in your
earlier emails as well: initAnnotationsIfNecessary() is a
synchronized instance method but I can not find any other code in
the VM that synchronizes on the Class object's monitor. So if this
synchronization is trying to establish consistency in the face of
class redefinition, I do not see where class redefinition
RFR (ultra-trivial review): Typo in http://java.sun.com/j2se/1.4.1/docs/api/java/util/logging/LogManager.html
Please review and push this ridiculously trivial fix: http://cr.openjdk.java.net/~jgish/Bug4819681-Day1-typo-in-LogManager/ http://cr.openjdk.java.net/%7Ejgish/Bug4819681-Day1-typo-in-LogManager/ I'd love to know how many people have stumbled on this ridiculous one-letter capitalization bug which has been around since day-1 of the introduction of logging and kept pushing it under the rug for others to stumble on instead of just fixing the thing and getting it out of the way. Why do we continue to carry this kind of nonsense around? I can just imagine the number of reports that have been generated over the years for every single release since 1.4, where someone has just said Ah that one. It's trivial. Just forget it. Jeez! (rant over :-) ) Thanks, Jim -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com
Re: RFR (ultra-trivial review): Typo in http://java.sun.com/j2se/1.4.1/docs/api/java/util/logging/LogManager.html
Approved! -Joe On 12/10/2012 02:44 PM, Jim Gish wrote: Please review and push this ridiculously trivial fix: http://cr.openjdk.java.net/~jgish/Bug4819681-Day1-typo-in-LogManager/ http://cr.openjdk.java.net/%7Ejgish/Bug4819681-Day1-typo-in-LogManager/ I'd love to know how many people have stumbled on this ridiculous one-letter capitalization bug which has been around since day-1 of the introduction of logging and kept pushing it under the rug for others to stumble on instead of just fixing the thing and getting it out of the way. Why do we continue to carry this kind of nonsense around? I can just imagine the number of reports that have been generated over the years for every single release since 1.4, where someone has just said Ah that one. It's trivial. Just forget it. Jeez! (rant over :-) ) Thanks, Jim
Re: RFR (ultra-trivial review): Typo in http://java.sun.com/j2se/1.4.1/docs/api/java/util/logging/LogManager.html
I am pushing the typo fix for you - Jim. Mandy On 12/10/2012 2:59 PM, Joe Darcy wrote: Approved! -Joe On 12/10/2012 02:44 PM, Jim Gish wrote: Please review and push this ridiculously trivial fix: http://cr.openjdk.java.net/~jgish/Bug4819681-Day1-typo-in-LogManager/ http://cr.openjdk.java.net/%7Ejgish/Bug4819681-Day1-typo-in-LogManager/ I'd love to know how many people have stumbled on this ridiculous one-letter capitalization bug which has been around since day-1 of the introduction of logging and kept pushing it under the rug for others to stumble on instead of just fixing the thing and getting it out of the way. Why do we continue to carry this kind of nonsense around? I can just imagine the number of reports that have been generated over the years for every single release since 1.4, where someone has just said Ah that one. It's trivial. Just forget it. Jeez! (rant over :-) ) Thanks, Jim
hg: jdk8/tl/jdk: 4819681: Typo in http://java.sun.com/j2se/1.4.1/docs/api/java/util/logging/LogManager.html
Changeset: cac1bfaceaaa Author:mchung Date: 2012-12-10 15:15 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cac1bfaceaaa 4819681: Typo in http://java.sun.com/j2se/1.4.1/docs/api/java/util/logging/LogManager.html Summary: Simple capitalization typo in LogManager() description Reviewed-by: darcy, mchung ! src/share/classes/java/util/logging/LogManager.java
Re: RFR (ultra-trivial review): Typo in http://java.sun.com/j2se/1.4.1/docs/api/java/util/logging/LogManager.html
Jim, Pushed [1]. I'm sorry that I missed to set the user when creating the changeset and it was too late when I noticed that and attempted to kill the hg push comment. Hope you don't mind. Mandy [1] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cac1bfaceaaa On 12/10/2012 2:44 PM, Jim Gish wrote: Please review and push this ridiculously trivial fix: http://cr.openjdk.java.net/~jgish/Bug4819681-Day1-typo-in-LogManager/ http://cr.openjdk.java.net/%7Ejgish/Bug4819681-Day1-typo-in-LogManager/ I'd love to know how many people have stumbled on this ridiculous one-letter capitalization bug which has been around since day-1 of the introduction of logging and kept pushing it under the rug for others to stumble on instead of just fixing the thing and getting it out of the way. Why do we continue to carry this kind of nonsense around? I can just imagine the number of reports that have been generated over the years for every single release since 1.4, where someone has just said Ah that one. It's trivial. Just forget it. Jeez! (rant over :-) ) Thanks, Jim
Re: Review Request: 8004201: add reducers to primitive type wrappers
http://cr.openjdk.java.net/~akhil/8004201.2/webrev/
- replaced Suitable for conversion as a method reference to functional
interfaces such as ... with @see java.util.function.BinaryOperator
- javadoc - replaced 'a type argument'/'another type argument' with
'the first operand'/'the second operand'
- did not widen return types - widening the return type makes these
methods unusable as reducers, since BinaryOperator is declared
T operate(T left, T right)
Thanks
On 12/05/2012 03:44 PM, Joseph Darcy wrote:
Akhil,
In javadoc like this
298 * as {@code BinaryOperatorlt;Booleangt;}.
you don't have to use lt; and the like inside {@code}; please change to
298 * as {@code BinaryOperatorBoolean}.
As a general comment, if the operations for primitive type Foo are put
into java.lang.Foo, then the type of the operation needs to be
explicitly represented in the expression calling the method (unless
static imports are used, etc.). Therefore, I suggest putting these sort
of static methods all into one class to allow overloading to do its
thing (java.lang.Operators?). Also, for methods like sum, I think it is
worth considering returning a larger type than the operands to avoid
problems from integer or floating-point overflow. For example, sum on
byte and short would return int, sun on int would return long, etc.
As an aside, to get a numerically robust result, the summation logic
over a set of doubles needs to be more than just a set of raw double
additions, but that can be tackled later.
Cheers,
-Joe
On 12/5/2012 1:27 PM, Akhil Arora wrote:
Updated - http://cr.openjdk.java.net/~akhil/8004201.1/webrev/
- delegate to Math.min/max for int/long/float/double
- rename Boolean.and/or/xor to logicalAnd/logicalOr/logicalXor
- removed Character variants of min/max/sum
On 12/02/2012 05:50 PM, David Holmes wrote:
Hi Akhil,
Is it really necessary/desirable to flag all of these as Suitable for
conversion as a method reference to functional interfaces such as ... ?
Not necessary, but it does provide a hint as to their intended use to a
casual browser of these docs.
This style:
+ * @param a a boolean argument.
+ * @param b another boolean argument.
is at odds with the style used elsewhere for new Functional APIs, and
with the style of other methods in these classes. Can we just use first
operand and second operand for all of these?
It is consistent with Math.min/max, which use the a/b style. Since these
methods are not in one of the functional package, is'nt it better to
stick to the local style?
Character.sum does not make sense to me. Who adds together characters?
I'm not even sure min and max are worth supporting for Character.
Good point - removed these methods for Character.
I disagree with other suggestions to use the Math functions for
float/double. I think all these methods should use the underlying
primitive operator regardless of type.
Are you disagreeing only for float/double or for int/long also? Can you
provide more information as to why you disagree?
Thanks
Thanks,
David
-
On 1/12/2012 4:44 AM, Akhil Arora wrote:
Hi
Requesting review for some basic functionality related to lambdas -
Add min, max, sum methods to the primitive wrapper classes - Byte,
Short, Integer, Long, Float, Double and Character so as to be able to
use them as reducers in lambda expressions. Add and, or, xor methods to
Boolean.
http://cr.openjdk.java.net/~akhil/8004201.0/webrev/
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8004201
Thanks
Re: RFR: JDK-8003890: modify test scripts to pass VMOPTIONS
On 12/10/12 3:25 AM, Alan Bateman wrote: On 10/12/2012 11:13, Chris Hegarty wrote: Thank you Mark, this is a really useful contribution. I noticed that a few tests (below) specify either -client or -server. If the vmoptions contain either of these arguments, and this seems to be the way our test/Makefile is passing them, then it may result in the test using the wrong vm. I think we should remove TESTVMOPTS from these tests, but then we lose other potential opts, like heap size, etc. java/rmi/reliability/benchmark/runRmiBench.sh java/rmi/reliability/benchmark/runSerialBench.sh sun/management/jmxremote/startstop/JMXStartStopTest.sh You're right, this will cause conflict. In the case of JMXStartStopTest.sh and runSerialBench.sh then it might be better to remove the -server. Stuart might have an opinion on runRmiBench.sh but one idea is to change is so that when run interactively (TESTJAVA not set) then it works as it does now and runs the benchmark twice. If TESTJAVA is not set then it runs the benchmark once, with whatever VM options are in use. For runRmiBench.sh the benchmark actually runs two JVMs, one as the RMI server and the other as the RMI client. Note that -client and -server are passed as arguments to the bench.rmi.Main start-class, which causes them to behave differently. I'm wondering if whoever wrote the script originally confused the arguments to Main with the arguments to the JVMs. Or, perhaps some tenuous line of reasoning was used, such as the RMI server is probably running on a server and so should be running the server VM, and same for the RMI client. So, I think that the JVM -server and -client args should be replaced by $TESTVMOPTS. But of course please leave -server and -client args to the Main class unchanged. (Note, the runRmiBench.sh test is currently on the problem list so it's not run right now anyway.) For runSerialBench.sh, yes, I think removing -server and adding $TESTVMOPTS is the right thing. * * * I've looked over the test/java/io/Serializable and the test/java/rmi changes and they look fine. I haven't looked at the other changes though. I can look at more files too if you need to spread out the reviewing load. s'marks
Re: Proposal: Fully Concurrent ClassLoading
On 10/12/2012 11:53 PM, David M. Lloyd wrote: On 12/09/2012 10:03 PM, David Holmes wrote: That sounds promising. Are you in a position to try out this proposal on a testbed with your app server? Sure, I'd love to. What tree should I build? Please apply the patches from the webrevs here: http://cr.openjdk.java.net/~dholmes/concurrent-loaders/webrev.hotspot/ http://cr.openjdk.java.net/~dholmes/concurrent-loaders/webrev.jdk/ They should apply to a jdk8 or tl forest. (And I hope I didn't mess something up generating the webrev ;-) ) Thanks, David
Re: bottleneck by java.lang.Class.getAnnotations() - rebased patch
Hi Peter,
On 12/10/2012 02:23 PM, Peter Levart wrote:
Hi David, Joe,
I unpacked the src.zip of the first release of JDK 1.5.0. Here's the
relevant part:
private transient MapClass, Annotation annotations;
private transient MapClass, Annotation declaredAnnotations;
private synchronized void initAnnotationsIfNecessary() {
if (annotations != null)
return;
declaredAnnotations = AnnotationParser.parseAnnotations(
getRawAnnotations(), getConstantPool(), this);
Class? superClass = getSuperclass();
if (superClass == null) {
annotations = declaredAnnotations;
} else {
annotations = new HashMapClass, Annotation();
superClass.initAnnotationsIfNecessary();
for (Map.EntryClass, Annotation e :
superClass.annotations.entrySet()) {
Class annotationClass = e.getKey();
if
(AnnotationType.getInstance(annotationClass).isInherited())
annotations.put(annotationClass, e.getValue());
}
annotations.putAll(declaredAnnotations);
}
}
...there's no sign of invalidation logic here yet. Neither for
annotations nor for fields/methods/constructors. This appears to have
been added later, presumably to accommodate class redefinition
changing annotations.
I would also like to see the source of AnnotationType to see if it
used the same logic and synchronization, but that's not part of
src.zip sources...
In JDK 5 GA, the annotations feature did not attempt to support class
file definition. From some quick bug archaeology, that omission was
addressed circa 5.0u8 (and JDK 6) via bugs including:
5002251 potential bug with annotations and class file evolution
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5002251
6412391 fix for annotation cache and RedefineClasses() conflict
needs HotSpot changes
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6412391
6422541 fix for {Constructor,Field,Method} annotation cache and
RedefineClasses() conflict needs HS changes
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6422541
-Joe
Regards, Peter
On 12/10/2012 09:52 PM, Joe Darcy wrote:
Yes; the OpenJDK sources only go back to circa 2007, which was
part-way into the JDK 7 release. The exact changesets of how
annotations were added back in JDK 5 are not available publicly.
However, the final results of that process may be mostly visible in
the src.zip from JDK 5.
HTH,
-Joe
On 12/10/2012 8:13 AM, Peter Levart wrote:
Hi David,
It would be informative to look at how java.lang.Class evolved
during history. The oldest revision I can access is from 1st of Dec.
2007, which already contains Java 1.5 code (annotations) and is more
or less unchanged until now. The most interesting changesets would
be those that introduced annotations.
Regards, Peter
On 12/10/2012 03:59 PM, Peter Levart wrote:
On 12/10/2012 07:18 AM, David Holmes wrote:
Hi Peter,
Sorry for the delay on this.
Thank you for taking it into consideration. I can only imagine how
busy you are with other things.
Generally your VolatileData and my ReflectionHelper are doing a
similar job. But I agree with your reasoning that all of the
cached SoftReferences are likely to be cleared at once, and so a
SoftReference to a helper object with direct references, is more
effective than a direct reference to a helper object with
SoftReferences. My initial stance with this was very conservative
as the more change that is introduced the more uncertainty there
is about the impact.
I say the above primarily because I think, if I am to proceed with
this, I will need to separate out the general reflection caching
changes from the annotation changes. There are a number of reasons
for this:
First, I'm not at all familiar with the implementation of
annotations at the VM or Java level, and the recent changes in
this area just exacerbate my ignorance of the mechanics. So I
don't feel qualified to evaluate that aspect.
Second, the bulk of the reflection caching code is simplified by
the fact that due to current constraints on class redefinition the
caching is effectively idempotent for fields/methods/constructors.
But that is not the case for annotations.
I think that on the Class-level these two aspects can be separated.
But not on the Field/Method/Constructor level, because annotations
are referenced by Field/Method/Constructor objects and there is no
invalidation logic - like on the Class-level - that would just
invalidate the sets of annotations on Field/Method/Constructor,
leaving Field/Method/Constructor objects still valid. So these two
aspects are connected - it may be - I haven't looked at history yet
- that invalidation of Field/Method/Constructor was introduced just
because of annotations.
Also, the following bug (currently not accessible):
hg: jdk8/tl/jdk: 6512101: Incorrect encoding in NetworkInterface.getDisplayName()
Changeset: 883feced1cdd Author:dingxmin Date: 2012-12-11 10:42 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/883feced1cdd 6512101: Incorrect encoding in NetworkInterface.getDisplayName() Reviewed-by: chegar, dsamersoff ! src/windows/native/java/net/NetworkInterface.c
Re: Proposal: Fully Concurrent ClassLoading
On 12/10/2012 06:36 PM, David Holmes wrote:
On 10/12/2012 11:53 PM, David M. Lloyd wrote:
On 12/09/2012 10:03 PM, David Holmes wrote:
That sounds promising. Are you in a position to try out this proposal on
a testbed with your app server?
Sure, I'd love to. What tree should I build?
Please apply the patches from the webrevs here:
http://cr.openjdk.java.net/~dholmes/concurrent-loaders/webrev.hotspot/
http://cr.openjdk.java.net/~dholmes/concurrent-loaders/webrev.jdk/
They should apply to a jdk8 or tl forest. (And I hope I didn't mess
something up generating the webrev ;-) )
Well, I've just gotten everything working and done some cursory testing
on a startup of an empty JBoss AS 7 instance, and then reviewing the
metrics reported by our class loader. Our timing metrics are not really
great for general-purpose benchmarking (for various uninteresting
reasons), but I can at least get enough information to know everything
is working more or less as expected:
On JDK 6 with our unsafe lockless modification, we're typically
loading 4707 classes, and I'm seeing anywhere from 0 to 5 define races
that were automatically resolved.
On JDK 7 using the standard registerAsParallelCapable() mechanism, we
are loading 4711 classes and I'm seeing 35-50 define races that were
automatically resolved - the overhead of locking starts to come to the
fore I think.
On JDK 8 with your patches, we are loading around 4750 classes and there
are, as expected, 0 define races (I believe, however, that we're getting
a false count though whenever defineClass() returns an already-defined
class - it would be nice if there were *some* way to detect that this
happened).
On our class loader implementations, I'm initializing this way:
static {
try {
ClassLoader.registerAsFullyConcurrent();
} catch (Throwable ignored) {
try {
ClassLoader.registerAsParallelCapable();
} catch (Throwable ignored2) {
}
}
}
The debugging message confirms that our class loaders are successfully
registered as fully concurrent, and the fact that it doesn't hang or
crash on JDK 7 indicates that they are still properly being registered
as parallel-capable on that platform.
--
- DML
Re: Proposal: Fully Concurrent ClassLoading
FYI I've made some small updates to the blog entry. Just to quantify the inefficiencies here is the instrumented output of a simple test that tries to load all classes in rt.jar and prints out the size of the lock maps: 18521 classes loaded. sun.misc.Launcher$AppClassLoader@f2f585 - lockMap size = 19050 sun.misc.Launcher$ExtClassLoader@d5163a - lockMap size = 19050 You may be wondering about the 500+ difference between the loaded classes and the map size? These represent classes in the list to load that were not actually present in rt.jar. So as you can see you not only get a lock object for every class successfully loaded, you get a lock object for every class that is attempted to be loaded! David - On 5/12/2012 9:59 PM, David Holmes wrote: Java 7 introduced support for parallel classloading by adding to each class loader a ConcurrentHashMap, referenced through a new field, parallelLockMap. This contains a mapping from class names to Objects to use as a classloading lock for that class name. This scheme has a number of inefficiencies. To address this we propose for Java 8 the notion of a fully concurrent classloader ... This is a fairly simple proposal that I've written up as a blog entry: https://blogs.oracle.com/dholmes/entry/parallel_classloading_revisited_fully_concurrent Please discuss this proposal here. Thanks, David Holmes
hg: jdk8/tl/jdk: 8004488: wrong permissions checked in krb5
Changeset: d206e52bf8a6 Author:weijun Date: 2012-12-11 13:14 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d206e52bf8a6 8004488: wrong permissions checked in krb5 Reviewed-by: xuelei ! src/share/classes/com/sun/security/auth/module/Krb5LoginModule.java ! src/share/classes/sun/security/jgss/krb5/Krb5Util.java + test/sun/security/krb5/auto/KeyPermissions.java ! test/sun/security/krb5/auto/KeyTabCompat.java
Re: Proposal: Fully Concurrent ClassLoading
David,
Many thanks for trialling this so promptly!
Do you have any suggestions for what instrumentation you would like to
see accompany this?
David
On 11/12/2012 1:41 PM, David M. Lloyd wrote:
On 12/10/2012 06:36 PM, David Holmes wrote:
On 10/12/2012 11:53 PM, David M. Lloyd wrote:
On 12/09/2012 10:03 PM, David Holmes wrote:
That sounds promising. Are you in a position to try out this
proposal on
a testbed with your app server?
Sure, I'd love to. What tree should I build?
Please apply the patches from the webrevs here:
http://cr.openjdk.java.net/~dholmes/concurrent-loaders/webrev.hotspot/
http://cr.openjdk.java.net/~dholmes/concurrent-loaders/webrev.jdk/
They should apply to a jdk8 or tl forest. (And I hope I didn't mess
something up generating the webrev ;-) )
Well, I've just gotten everything working and done some cursory testing
on a startup of an empty JBoss AS 7 instance, and then reviewing the
metrics reported by our class loader. Our timing metrics are not really
great for general-purpose benchmarking (for various uninteresting
reasons), but I can at least get enough information to know everything
is working more or less as expected:
On JDK 6 with our unsafe lockless modification, we're typically
loading 4707 classes, and I'm seeing anywhere from 0 to 5 define races
that were automatically resolved.
On JDK 7 using the standard registerAsParallelCapable() mechanism, we
are loading 4711 classes and I'm seeing 35-50 define races that were
automatically resolved - the overhead of locking starts to come to the
fore I think.
On JDK 8 with your patches, we are loading around 4750 classes and there
are, as expected, 0 define races (I believe, however, that we're getting
a false count though whenever defineClass() returns an already-defined
class - it would be nice if there were *some* way to detect that this
happened).
On our class loader implementations, I'm initializing this way:
static {
try {
ClassLoader.registerAsFullyConcurrent();
} catch (Throwable ignored) {
try {
ClassLoader.registerAsParallelCapable();
} catch (Throwable ignored2) {
}
}
}
The debugging message confirms that our class loaders are successfully
registered as fully concurrent, and the fact that it doesn't hang or
crash on JDK 7 indicates that they are still properly being registered
as parallel-capable on that platform.
Re: RFR: 8001647: In-place methods on Collection/List
http://cr.openjdk.java.net/~akhil/8001647.3/webrev/
- now with synchronized and unmodifiable wrappers in Collections.java
for the default methods being added
On 12/10/2012 01:48 PM, Akhil Arora wrote:
Updated with yours and Alan's comments -
http://cr.openjdk.java.net/~akhil/8001647.2/webrev/
- removed null check for removeSet
- cache this.size in removeAll, replaceAll
(for ArrayList, Vector and CopyOnWriteArrayList)
- calculate removeCount instead of BitCount.cardinality()
- removed unnecessary @library from test support classes
Catching IndexOOB and throwing CME in forEach/removeAll/replaceAll seems
unecessary... did you have a use-case in mind where an IndexOOB can
occur with these methods?
Thanks
On 12/08/2012 05:11 AM, Arne Siegel wrote:
ArrayList.java, line 1171:
final boolean anyToRemove = (removeSet != null)
!removeSet.isEmpty();
As removeSet will never be null, this line can be simplified. This is a
left-over from the
preceding implementation (see b67).
ArrayList.java, method forEach optimizes the loop by reducing the number of
heap accesses:
final int size = this.size;
for (int i=0; modCount == expectedModCount i size; i++) {
...
This trick might also be introduced to methods removeAll and replaceAll.
In the ListIterator implementation of ArrayList, there are various appearances
of the idiom:
try {
...
} catch (IndexOutOfBoundsException ex) {
throw new ConcurrentModificationException();
}
This technique might also be used in forEach, removeAll, and replaceAll (though
not likely to
be of any importance).
Regards
Arne Siegel
