Re: RFR: javax.xml.stream: Using ServiceLoader to load JAXP stream factories (7169894: JAXP Plugability Layer: using service loader)

[Mandy Chung]

Hi Daniel,

There are currently several different FactoryFinder class in the JAXP 
implementation and there might be slight difference in each of them.  Do 
you know if it has been looked into whether it's feasible to merge them 
into a single utility class?  I suspect someone might have brought this 
up in the past.


Mandy

On 12/17/2012 9:43 AM, Daniel Fuchs wrote:

Hi,

Here is a new webrev in the series that addresses using ServiceLoader in
JAXP for JDK 8.

7169894: JAXP Plugability Layer: using service loader

This changeset addresses modification in the javax.xml.stream
package.
It is similar to changes proposed for the javax.xml.parsers
package [1], with a few differences due to the specificity of
javax.xml.stream.

Namely:

1. The XMLXxxxFactory.newInstance methods that takes parameter takes
   a property name, rather than a class name, and thus calls
   FactoryFinder.find.

2. One of the deprecated XMLOutputFactory.newInstance method had a
   bug - it used to return an XMLInputFactory - and was deprecated
   because of that - so I did preserve the bug.

3. The noarg newFactory() methods were leaking instances of the
   private FactoryFinder$ConfigurationError - my patch corrects
   that since it removes FactoryFinder$ConfigurationError.

4. In FactoryFinder.find() the ClassLoader parameter is often ignored,
   which makes the factories in the stream package behave differently
   from theirs cousins in the other packages. I believe this was an
   oversight due to (1).
   My patch will not fix that - I will instead log this as a separate
   issue.


 



best regards,

-- daniel

previous webrevs in the series:

[1] javax.xml.parsers:
 



[2] javax.xml.datatype:
 

Re: Review request: 8003562: Provide a command-line tool to find static dependencies

[Mandy Chung]

On 12/17/12 8:13 AM, Ulf Zibis wrote:

Am 17.12.2012 16:23, schrieb Alan Bateman:
I mostly agree with your argument that this is a new tool and that we 
should be consistent with GNU-style options. So what was the final 
bid is, is it --classpath or --class-path?


I propose --class-path so that it makes it easier to determine 
-classpath is not supported
I guess it wouldn't do any harm to silently supporting -cp and 
-classpath as folks used to java* tools will probably use them 
without thinking.


How important to carry the same option -cp and -classpath on j* tools?  
I do think developers can well adapt to a slight different option when 
using a new tool.  I'm inclined not to mix these styles.  In addition, 
I'd like to see how jdeps will be used after the initial push and people 
finds --class-path hard to move to.


Thanks for supporting my arguments about "silent familarity" to old 
style, Alan.
As --class(-)path is a frequent and important option, I additionally 
think, we should have an official short form. If '-cp' doesn't conform 
with GNU-style, what about '-C' ?


I considered '-C' and it might be confused with 'jar -C' and 'tar -C' 
option to change the directory.  I think '-c' may be okay and it can 
avoid such confusion and closer to '-cp'.   Are you okay with that?


I'd like to send a new webrev tomorrow and finalize the review this week.

Mandy

Re: RFR: 8004699 : Add type annotation storage to Constructor, Field and Method

[David Holmes]

On 18/12/2012 3:06 AM, Joel Borggrén-Franck wrote:


On Dec 17, 2012, at 5:00 PM, Peter Levart  wrote:


Hi Joel,

  82 // This is set by the vm at Method creation
  83 private byte[]  type_annotations;


Wouldn't it be better to initialize this field in the constructor? You could 
create an overloaded constructor if you wanted to be compatible with previous 
versions of the VM.



Fields aren't created using a constructor as far as I can see.


Correct, all the reflection objects are allocated directly by the VM and 
initialized field by field as needed. No Java level constructor involved.


This change is also fine with me, on the proviso that the follow up 
change to deal with efficiency and the field name is not too far away. 
Though really I don't think the change of name will be that big a deal 
anyway.


Cheers,
David


cheers
/Joel

Re: EnumData space optimization in j.l.Class (JEP-146)

[David Holmes]

Hi Peter,

BTW JEP-149 not 146!

Sorry I didn't get a chance to respond last night before you continued 
on this path. I have to say no to this too. First I am just running out 
of time to get this finalized by M6 - particularly with the Xmas break 
looming.


Second the trade-off here is far less clear. Not only may the 
performance aspect be more significant (as per Remi's discussion) but 
the memory saving may not even eventuate depending on alignment.


It may be worth doing a new JEP for continued enhancements in this area 
post JDK 8, or maybe just have a RFE filed. But for now I have to put 
the brakes on and just run with what we have with the reflection caching 
changes.


Your efforts are very much appreciated - I just wish the timing could 
have been different.


Thanks,
David

On 18/12/2012 1:36 AM, Peter Levart wrote:

Hi David and others,

Here's a patch that eliminates one of two fields in java.lang.Class,
related to caching enum constants:

http://dl.dropbox.com/u/101777488/jdk8-tl/JEP-149.enum/webrev.01/index.html

It does it by moving one field to a subclass of HashMap, which is
referenced by a remaining field that serves two different
purposes/stages of caching.

These are the results of a micro-benchmark that exercises public API
that uses the internal j.l.Class API regarding enum constants:

enum MyEnum { ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, TEN }
EnumSet.noneOf(MyEnum.class): 300_000_000 loops
MyEnum.valueOf(String): 30_000_000 loops * 10 calls for different names

** Original JDK8 code

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp
../out/production/test test.EnumTest reference

EnumSet.noneOf(Class): 351610312 340302968 33989 339774384 339750612
339558414 339547022 339621595
MyEnum.valueOf(String): 935153830 897188742 887541353 960839820
886119463 885818334 885827093 885752461
EnumSet.noneOf(Class): 339552678 339469528 339513757 339451341 339512154
339511634 339664326 339793144

** patched java.lang.Class

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp
../out/production/test -Xbootclasspath/p:../out/production/jdk test.EnumTest

EnumSet.noneOf(Class): 351724931 339286591 305082929 305042885 305058303
305044144 305073463 305049604
MyEnum.valueOf(String): 955032718 908534137 891406394 891506147
891414312 893652469 891412757 891409294
EnumSet.noneOf(Class): 414044087 406904161 406788898 406839824 406765274
406815728 407002576 406779162

The slow-down of about 20% (last line) is presumably a consequence of
another in-direction to obtain shared enum constants array when there is
already a Map in place. It is still fast though (300M EnumSet instances
/ 0.4 s).

Here's the source of the micro-benchmark:

https://raw.github.com/plevart/jdk8-tl/JEP-149.enum/test/src/test/EnumTest.java

I don't know what's more important in this occasion. A small space gain
(8 or 4 bytes per j.l.Class instance) or a small performance gain (20%).

Regards, Peter

hg: jdk8/tl/langtools: 8005137: Rename DocLint.call to DocLint.init which overrides Plugin.init

[mandy . chung]

Changeset: ef537bcc825a
Author:mchung
Date:  2012-12-17 15:19 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/ef537bcc825a

8005137: Rename DocLint.call to DocLint.init which overrides Plugin.init
Reviewed-by: darcy, jjh

! src/share/classes/com/sun/tools/doclint/DocLint.java

Re: EnumData space optimization in j.l.Class (JEP-146)

[Remi Forax]

On 12/17/2012 11:14 PM, Peter Levart wrote:

On 12/17/2012 10:26 PM, Mandy Chung wrote:

On 12/17/12 7:36 AM, Peter Levart wrote:

Hi David and others,

Here's a patch that eliminates one of two fields in java.lang.Class, 
related to caching enum constants:


http://dl.dropbox.com/u/101777488/jdk8-tl/JEP-149.enum/webrev.01/index.html 



It does it by moving one field to a subclass of HashMap, which is 
referenced by a remaining field that serves two different 
purposes/stages of caching.




Your observation of merging the enumConstants and 
enumConstantDirectory is a good one.   I see that caching of 
enumConstantDirectory is important as it's used by EnumMap and 
EnumSet whose performance is critical (specified with constant time 
operations).  I'm unsure about Class.getEnumConstants whether it's 
performance critical and worths the complexity of your proposed fix 
(the enumData field of two types).  If a class has cached an 
enumConstantDirectory, Class.getEnumConstants can return a clone of 
its values().


Anyone knows how Class.getEnumConstants is commonly used and needs to 
be performant?  I suspect it's more typical to obtain the list of 
enum constants statistically (calling Enum.values()) than reflectively.

Hi Mandy,

public Class.getEnumConstants() is a reflection mirror of 
SomeEnum.values(). It returns a defensive copy of the constants array. 
The primary place for Enum constants is in a private static final 
$VALUES field, generated by compiler in each Enum subclass. But that I 
think is not part of specification, so for internal usage (as far as I 
have managed to find out only in the constructors of EnumSet and 
EnumMap), the package-private Class.getEnumConstantsShared() is used 
which obtains a copy of the array by calling SomeEnum.values() and 
than caches is.


The Class.enumConstantDirectory() on the other hand is an internal 
package-private method that returns a shared/cached Map, 
which is used internally to implement SomeEnum.valueOf(String) and 
Enum.valueOf(Class, String) static methods.


Both package-private methods must be fast.

Regards, Peter


for what it worth, I'm the guy behind the patch of bug 6276988 (it was 
before OpenJDK was setup BTW),

  http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6276988
and for the little story, I need that patch because I was developing an 
Eclipse plugin that uses EnumSet to represent the possible completion 
values.
So to answer to Mandy, this application needs really fast EnumSet 
creation thus really fast getEnumConstantShared() because the EnumSets 
was created as user types code.


Also, Peter, in your getEnumConstantShared(), while the first instanceof 
is a cheap one, the second is not.
I think I prefer either the status quo or to group all exotic fields in 
a specific object and pay the indirection to that object but not the 
instanceof checks.


cheers,
Rémi





Mandy

These are the results of a micro-benchmark that exercises public API 
that uses the internal j.l.Class API regarding enum constants:


enum MyEnum { ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, 
TEN }

EnumSet.noneOf(MyEnum.class): 300_000_000 loops
MyEnum.valueOf(String): 30_000_000 loops * 10 calls for different names

** Original JDK8 code

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
../out/production/test test.EnumTest reference


  EnumSet.noneOf(Class): 351610312 340302968 33989 339774384 
339750612 339558414 339547022 339621595
 MyEnum.valueOf(String): 935153830 897188742 887541353 960839820 
886119463 885818334 885827093 885752461
  EnumSet.noneOf(Class): 339552678 339469528 339513757 339451341 
339512154 339511634 339664326 339793144


** patched java.lang.Class

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
../out/production/test -Xbootclasspath/p:../out/production/jdk 
test.EnumTest


  EnumSet.noneOf(Class): 351724931 339286591 305082929 305042885 
305058303 305044144 305073463 305049604
 MyEnum.valueOf(String): 955032718 908534137 891406394 891506147 
891414312 893652469 891412757 891409294
  EnumSet.noneOf(Class): 414044087 406904161 406788898 406839824 
406765274 406815728 407002576 406779162


The slow-down of about 20% (last line) is presumably a consequence 
of another in-direction to obtain shared enum constants array when 
there is already a Map in place. It is still fast though (300M 
EnumSet instances / 0.4 s).


Here's the source of the micro-benchmark:

https://raw.github.com/plevart/jdk8-tl/JEP-149.enum/test/src/test/EnumTest.java 



I don't know what's more important in this occasion. A small space 
gain (8 or 4 bytes per j.l.Class instance) or a small performance 
gain (20%).


Regards, Peter

Re: EnumData space optimization in j.l.Class (JEP-146)

[Peter Levart]

On 12/17/2012 10:26 PM, Mandy Chung wrote:

On 12/17/12 7:36 AM, Peter Levart wrote:

Hi David and others,

Here's a patch that eliminates one of two fields in java.lang.Class, 
related to caching enum constants:


http://dl.dropbox.com/u/101777488/jdk8-tl/JEP-149.enum/webrev.01/index.html 



It does it by moving one field to a subclass of HashMap, which is 
referenced by a remaining field that serves two different 
purposes/stages of caching.




Your observation of merging the enumConstants and 
enumConstantDirectory is a good one.   I see that caching of 
enumConstantDirectory is important as it's used by EnumMap and EnumSet 
whose performance is critical (specified with constant time 
operations).  I'm unsure about Class.getEnumConstants whether it's 
performance critical and worths the complexity of your proposed fix 
(the enumData field of two types).  If a class has cached an 
enumConstantDirectory, Class.getEnumConstants can return a clone of 
its values().

Hi again,

I think that the two-part caching (array-Map) was meant to save the 
creation of a HashMap if it was not needed and also to provide a shared 
array of constants for internal usage in order to spare from creating 
array clones.


Peter


Anyone knows how Class.getEnumConstants is commonly used and needs to 
be performant?  I suspect it's more typical to obtain the list of enum 
constants statistically (calling Enum.values()) than reflectively.


Mandy

These are the results of a micro-benchmark that exercises public API 
that uses the internal j.l.Class API regarding enum constants:


enum MyEnum { ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, 
TEN }

EnumSet.noneOf(MyEnum.class): 300_000_000 loops
MyEnum.valueOf(String): 30_000_000 loops * 10 calls for different names

** Original JDK8 code

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
../out/production/test test.EnumTest reference


  EnumSet.noneOf(Class): 351610312 340302968 33989 339774384 
339750612 339558414 339547022 339621595
 MyEnum.valueOf(String): 935153830 897188742 887541353 960839820 
886119463 885818334 885827093 885752461
  EnumSet.noneOf(Class): 339552678 339469528 339513757 339451341 
339512154 339511634 339664326 339793144


** patched java.lang.Class

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
../out/production/test -Xbootclasspath/p:../out/production/jdk 
test.EnumTest


  EnumSet.noneOf(Class): 351724931 339286591 305082929 305042885 
305058303 305044144 305073463 305049604
 MyEnum.valueOf(String): 955032718 908534137 891406394 891506147 
891414312 893652469 891412757 891409294
  EnumSet.noneOf(Class): 414044087 406904161 406788898 406839824 
406765274 406815728 407002576 406779162


The slow-down of about 20% (last line) is presumably a consequence of 
another in-direction to obtain shared enum constants array when there 
is already a Map in place. It is still fast though (300M EnumSet 
instances / 0.4 s).


Here's the source of the micro-benchmark:

https://raw.github.com/plevart/jdk8-tl/JEP-149.enum/test/src/test/EnumTest.java 



I don't know what's more important in this occasion. A small space 
gain (8 or 4 bytes per j.l.Class instance) or a small performance 
gain (20%).


Regards, Peter

Re: EnumData space optimization in j.l.Class (JEP-146)

[Peter Levart]

On 12/17/2012 10:26 PM, Mandy Chung wrote:

On 12/17/12 7:36 AM, Peter Levart wrote:

Hi David and others,

Here's a patch that eliminates one of two fields in java.lang.Class, 
related to caching enum constants:


http://dl.dropbox.com/u/101777488/jdk8-tl/JEP-149.enum/webrev.01/index.html 



It does it by moving one field to a subclass of HashMap, which is 
referenced by a remaining field that serves two different 
purposes/stages of caching.




Your observation of merging the enumConstants and 
enumConstantDirectory is a good one.   I see that caching of 
enumConstantDirectory is important as it's used by EnumMap and EnumSet 
whose performance is critical (specified with constant time 
operations).  I'm unsure about Class.getEnumConstants whether it's 
performance critical and worths the complexity of your proposed fix 
(the enumData field of two types).  If a class has cached an 
enumConstantDirectory, Class.getEnumConstants can return a clone of 
its values().


Anyone knows how Class.getEnumConstants is commonly used and needs to 
be performant?  I suspect it's more typical to obtain the list of enum 
constants statistically (calling Enum.values()) than reflectively.

Hi Mandy,

public Class.getEnumConstants() is a reflection mirror of 
SomeEnum.values(). It returns a defensive copy of the constants array. 
The primary place for Enum constants is in a private static final 
$VALUES field, generated by compiler in each Enum subclass. But that I 
think is not part of specification, so for internal usage (as far as I 
have managed to find out only in the constructors of EnumSet and 
EnumMap), the package-private Class.getEnumConstantsShared() is used 
which obtains a copy of the array by calling SomeEnum.values() and than 
caches is.


The Class.enumConstantDirectory() on the other hand is an internal 
package-private method that returns a shared/cached Map, 
which is used internally to implement SomeEnum.valueOf(String) and 
Enum.valueOf(Class, String) static methods.


Both package-private methods must be fast.

Regards, Peter



Mandy

These are the results of a micro-benchmark that exercises public API 
that uses the internal j.l.Class API regarding enum constants:


enum MyEnum { ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, 
TEN }

EnumSet.noneOf(MyEnum.class): 300_000_000 loops
MyEnum.valueOf(String): 30_000_000 loops * 10 calls for different names

** Original JDK8 code

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
../out/production/test test.EnumTest reference


  EnumSet.noneOf(Class): 351610312 340302968 33989 339774384 
339750612 339558414 339547022 339621595
 MyEnum.valueOf(String): 935153830 897188742 887541353 960839820 
886119463 885818334 885827093 885752461
  EnumSet.noneOf(Class): 339552678 339469528 339513757 339451341 
339512154 339511634 339664326 339793144


** patched java.lang.Class

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
../out/production/test -Xbootclasspath/p:../out/production/jdk 
test.EnumTest


  EnumSet.noneOf(Class): 351724931 339286591 305082929 305042885 
305058303 305044144 305073463 305049604
 MyEnum.valueOf(String): 955032718 908534137 891406394 891506147 
891414312 893652469 891412757 891409294
  EnumSet.noneOf(Class): 414044087 406904161 406788898 406839824 
406765274 406815728 407002576 406779162


The slow-down of about 20% (last line) is presumably a consequence of 
another in-direction to obtain shared enum constants array when there 
is already a Map in place. It is still fast though (300M EnumSet 
instances / 0.4 s).


Here's the source of the micro-benchmark:

https://raw.github.com/plevart/jdk8-tl/JEP-149.enum/test/src/test/EnumTest.java 



I don't know what's more important in this occasion. A small space 
gain (8 or 4 bytes per j.l.Class instance) or a small performance 
gain (20%).


Regards, Peter

Re: EnumData space optimization in j.l.Class (JEP-146)

[Mandy Chung]

On 12/17/12 7:36 AM, Peter Levart wrote:

Hi David and others,

Here's a patch that eliminates one of two fields in java.lang.Class, 
related to caching enum constants:


http://dl.dropbox.com/u/101777488/jdk8-tl/JEP-149.enum/webrev.01/index.html 



It does it by moving one field to a subclass of HashMap, which is 
referenced by a remaining field that serves two different 
purposes/stages of caching.




Your observation of merging the enumConstants and enumConstantDirectory 
is a good one.   I see that caching of enumConstantDirectory is 
important as it's used by EnumMap and EnumSet whose performance is 
critical (specified with constant time operations).  I'm unsure about 
Class.getEnumConstants whether it's performance critical and worths the 
complexity of your proposed fix (the enumData field of two types).  If a 
class has cached an enumConstantDirectory, Class.getEnumConstants can 
return a clone of its values().


Anyone knows how Class.getEnumConstants is commonly used and needs to be 
performant?  I suspect it's more typical to obtain the list of enum 
constants statistically (calling Enum.values()) than reflectively.


Mandy

These are the results of a micro-benchmark that exercises public API 
that uses the internal j.l.Class API regarding enum constants:


enum MyEnum { ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, TEN }
EnumSet.noneOf(MyEnum.class): 300_000_000 loops
MyEnum.valueOf(String): 30_000_000 loops * 10 calls for different names

** Original JDK8 code

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
../out/production/test test.EnumTest reference


  EnumSet.noneOf(Class): 351610312 340302968 33989 339774384 
339750612 339558414 339547022 339621595
 MyEnum.valueOf(String): 935153830 897188742 887541353 960839820 
886119463 885818334 885827093 885752461
  EnumSet.noneOf(Class): 339552678 339469528 339513757 339451341 
339512154 339511634 339664326 339793144


** patched java.lang.Class

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
../out/production/test -Xbootclasspath/p:../out/production/jdk 
test.EnumTest


  EnumSet.noneOf(Class): 351724931 339286591 305082929 305042885 
305058303 305044144 305073463 305049604
 MyEnum.valueOf(String): 955032718 908534137 891406394 891506147 
891414312 893652469 891412757 891409294
  EnumSet.noneOf(Class): 414044087 406904161 406788898 406839824 
406765274 406815728 407002576 406779162


The slow-down of about 20% (last line) is presumably a consequence of 
another in-direction to obtain shared enum constants array when there 
is already a Map in place. It is still fast though (300M EnumSet 
instances / 0.4 s).


Here's the source of the micro-benchmark:

https://raw.github.com/plevart/jdk8-tl/JEP-149.enum/test/src/test/EnumTest.java 



I don't know what's more important in this occasion. A small space 
gain (8 or 4 bytes per j.l.Class instance) or a small performance gain 
(20%).


Regards, Peter

RFR:8005118 Javadoc styles are inconsistent

[Jim Gish]

Please review the simple fix that makes the String classes more 
consistent w.r.t. javadoc (specifically usages of , , {@code}, 
etc.).


http://cr.openjdk.java.net/~jgish/Bug8005118-String-spec-consistency/ 



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

hg: jdk8/tl/langtools: 8004961: rename Plugin.call to Plugin.init

[jonathan . gibbons]

Changeset: 064e372f273d
Author:jjg
Date:  2012-12-17 10:55 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/064e372f273d

8004961: rename Plugin.call to Plugin.init
Reviewed-by: mcimadamore

! src/share/classes/com/sun/source/util/Plugin.java
! src/share/classes/com/sun/tools/javac/main/Main.java
! test/tools/javac/plugin/showtype/ShowTypePlugin.java
! test/tools/javac/plugin/showtype/Test.java

RFR: [jdk7u-dev] - 8003898: X11 toolkit can be chosen as the default toolkit

[Rob McKenna]

Hi folks,

This review contains:

8003898: X11 toolkit can be chosen as the default toolkit
7162111: TEST_BUG: change tests run in headless mode [macosx] (open)
8004928: TEST_BUG: Reduce dependence of CoreLib tests from the AWT subsystem

Unfortunately the last two patches didn't apply cleanly, hence the 
review request. (the commit comments will be altered appropriately 
before integration)


Webrev at:

http://cr.openjdk.java.net/~robm/7162111/webrev.01/ 



I've just submitted a test job so I'll integrate once I get a review and 
a clean bill of health from that.


-Rob

hg: jdk8/tl/jdk: 8004832: Add new doclint package

[jonathan . gibbons]

Changeset: bac477d67867
Author:jjg
Date:  2012-12-17 10:31 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bac477d67867

8004832: Add new doclint package
Reviewed-by: erikj, ohair

! make/common/Release.gmk
! make/common/internal/Defs-langtools.gmk
! makefiles/CreateJars.gmk

RFR: javax.xml.stream: Using ServiceLoader to load JAXP stream factories (7169894: JAXP Plugability Layer: using service loader)

[Daniel Fuchs]

Hi,

Here is a new webrev in the series that addresses using ServiceLoader in
JAXP for JDK 8.

7169894: JAXP Plugability Layer: using service loader

This changeset addresses modification in the javax.xml.stream
package.
It is similar to changes proposed for the javax.xml.parsers
package [1], with a few differences due to the specificity of
javax.xml.stream.

Namely:

1. The XMLXxxxFactory.newInstance methods that takes parameter takes
   a property name, rather than a class name, and thus calls
   FactoryFinder.find.

2. One of the deprecated XMLOutputFactory.newInstance method had a
   bug - it used to return an XMLInputFactory - and was deprecated
   because of that - so I did preserve the bug.

3. The noarg newFactory() methods were leaking instances of the
   private FactoryFinder$ConfigurationError - my patch corrects
   that since it removes FactoryFinder$ConfigurationError.

4. In FactoryFinder.find() the ClassLoader parameter is often ignored,
   which makes the factories in the stream package behave differently
   from theirs cousins in the other packages. I believe this was an
   oversight due to (1).
   My patch will not fix that - I will instead log this as a separate
   issue.




best regards,

-- daniel

previous webrevs in the series:

[1] javax.xml.parsers:


[2] javax.xml.datatype:

Re: RFR: 8004699 : Add type annotation storage to Constructor, Field and Method

[Joel Borggrén-Franck]

On Dec 17, 2012, at 5:00 PM, Peter Levart  wrote:

> Hi Joel,
> 
>  82 // This is set by the vm at Method creation
>  83 private byte[]  type_annotations;
> 
> 
> Wouldn't it be better to initialize this field in the constructor? You could 
> create an overloaded constructor if you wanted to be compatible with previous 
> versions of the VM.
> 

Fields aren't created using a constructor as far as I can see.

cheers
/Joel

Re: RFR: 4247235: (spec str) StringBuffer.insert(int, char[]) specification is inconsistent

[Jim Gish]

I've created a new bug: https://jbs.oracle.com/bugs/browse/JDK-8005118 - 
I'll address the consistency issues there, and treat the NPE specs 
separately.


Jim

On 12/17/2012 11:29 AM, Jim Gish wrote:


On 12/15/2012 08:58 AM, Alan Bateman wrote:

On 14/12/2012 22:49, Jim Gish wrote:
Please review 
http://cr.openjdk.java.net/~jgish/Bug4247235-Add-Blanket-Null-Stmt/ 



This minor spec change (which will require CCC approval), adds 
blanket null-handling statements to both StringBuffer and 
StringBuilder, equivalent to the one already in String.
It looks like most (all?) of the methods defined by StringBuffer and 
StringBuilder that can throw NPE already specify it. Are you planning 
on removing the @throws from the methods?
I left it as is to raise this very point.  In a previous proposed 
change, adding a method somewhere that was explicit about throwing 
NPE, someone brought up the issue that we should rely on a blanket 
statement and not clutter the javadoc with @throws NPE.


Should we, in fact, be removing these method-specific @throws specs?


I see that String uses null, the proposed update to 
StringBuilder uses null, and the proposed update to 
StringBuffer uses {@code null}. We should try to be consistent.
I aimed for self-consistency here -- keeping what was already in 
place.  This was intentional to raise the point of whether everything 
should be brought up to date.


Thanks,
   Jim



-Alan.




--
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: 8004699 : Add type annotation storage to Constructor, Field and Method

[Joel Borggrén-Franck]

On Dec 17, 2012, at 4:47 PM, Alan Bateman  wrote:

> On 17/12/2012 15:41, Joel Borggrén-Franck wrote:
>> Here is the first in a series of changes to add type annotation support to 
>> reflection.
>> 
>> http://cr.openjdk.java.net/~jfranck/8004699/webrev.00/
>> 
> I assume this should be typeAnnotations rather than type_annotations so that 
> it's consistent with the existing fields.

Doh. However since these fields are not the permanent solution and the overhead 
to changing naming is large, I would (vert strongly) prefer to keep the (bad) 
HotSpot naming convention for now.

> Have the Hotspot changes been pushed already?
> 

Out for review here: 
http://mail.openjdk.java.net/pipermail/hotspot-dev/2012-December/007637.html

cheers
/Joel

hg: jdk8/tl/jdk: 8005090: Include com.sun.source.doctree in Tree API docs

[jonathan . gibbons]

Changeset: 9f1b516cd9cb
Author:jjg
Date:  2012-12-17 08:34 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/9f1b516cd9cb

8005090: Include com.sun.source.doctree in Tree API docs
Reviewed-by: erikj

! make/docs/NON_CORE_PKGS.gmk

hg: jdk8/tl: 8005090: Include com.sun.source.doctree in Tree API docs

[jonathan . gibbons]

Changeset: a0779b1e9a4d
Author:jjg
Date:  2012-12-17 08:34 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/a0779b1e9a4d

8005090: Include com.sun.source.doctree in Tree API docs
Reviewed-by: erikj

! common/makefiles/javadoc/NON_CORE_PKGS.gmk

hg: jdk8/tl/jdk: 8005081: java/util/prefs/PrefsSpi.sh fails on macos-x

[chris . hegarty]

Changeset: bcf79e6f52a0
Author:chegar
Date:  2012-12-17 16:27 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bcf79e6f52a0

8005081: java/util/prefs/PrefsSpi.sh fails on macos-x
Reviewed-by: alanb

! test/java/util/prefs/PrefsSpi.sh

Re: RFR: 4247235: (spec str) StringBuffer.insert(int, char[]) specification is inconsistent

[Jim Gish]

On 12/15/2012 08:58 AM, Alan Bateman wrote:

On 14/12/2012 22:49, Jim Gish wrote:
Please review 
http://cr.openjdk.java.net/~jgish/Bug4247235-Add-Blanket-Null-Stmt/ 



This minor spec change (which will require CCC approval), adds 
blanket null-handling statements to both StringBuffer and 
StringBuilder, equivalent to the one already in String.
It looks like most (all?) of the methods defined by StringBuffer and 
StringBuilder that can throw NPE already specify it. Are you planning 
on removing the @throws from the methods?
I left it as is to raise this very point.  In a previous proposed 
change, adding a method somewhere that was explicit about throwing NPE, 
someone brought up the issue that we should rely on a blanket statement 
and not clutter the javadoc with @throws NPE.


Should we, in fact, be removing these method-specific @throws specs?


I see that String uses null, the proposed update to 
StringBuilder uses null, and the proposed update to 
StringBuffer uses {@code null}. We should try to be consistent.
I aimed for self-consistency here -- keeping what was already in place.  
This was intentional to raise the point of whether everything should be 
brought up to date.


Thanks,
   Jim



-Alan.


--
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

hg: jdk8/tl/langtools: 8004099: Bad compiler diagnostic generated when poly expression is passed to non-existent method

[maurizio . cimadamore]

Changeset: f20568328a57
Author:mcimadamore
Date:  2012-12-17 16:13 +
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/f20568328a57

8004099: Bad compiler diagnostic generated when poly expression is passed to 
non-existent method
Summary: Some code paths in resolve do not use methodArguments to correctly 
format actuals
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/comp/Attr.java
! src/share/classes/com/sun/tools/javac/comp/Resolve.java
+ test/tools/javac/lambda/BadMethodCall2.java
+ test/tools/javac/lambda/BadMethodCall2.out

Re: Review request: 8003562: Provide a command-line tool to find static dependencies

[Ulf Zibis]

Am 17.12.2012 16:23, schrieb Alan Bateman:
I mostly agree with your argument that this is a new tool and that we should be consistent with 
GNU-style options. So what was the final bid is, is it --classpath or --class-path? I guess it 
wouldn't do any harm to silently supporting -cp and -classpath as folks used to java* tools will 
probably use them without thinking.


Thanks for supporting my arguments about "silent familarity" to old style, Alan.
As --class(-)path is a frequent and important option, I additionally think, we should have an 
official short form. If '-cp' doesn't conform with GNU-style, what about '-C' ?


-Ulf

Re: RFR: javax.xml.datatype: Using ServiceLoader to load JAXP datatype factories (7169894: JAXP Plugability Layer: using service loader)

[Alan Bateman]

On 17/12/2012 15:39, Daniel Fuchs wrote:

Hi,

As pointed out by Paul & Alan, there's no reason to force
Class.forName to initialize the class when attempting to
load it in FactoryFinder.

FactoryFinder should behave like ServiceLoader in this respect.
So here is an updated webrev for javax.xml.datatype.
 



I also reported the changes to javax.xml.parsers:
 




-- daniel

I've looked at both webrevs and this looks very good (thank you for 
being so careful).


-Alan.

Re: RRFR 8002356: Add ForkJoin common pool and CountedCompleted

[Doug Lea]

On 12/13/12 15:16, Mike Duigou wrote:

Some notes:


Thanks!



- The Object padding (pad10 - pad1d) in WorkQueue and ForkJoinPool is
sensitive to reference size and compressed OOPS. I was surprised to see
Object used rather than long or int.


Until we get @Contended, this is black art. Because of how layouts
work, you need trailing padding to be refs for anything with at
least one ref.



- how is getCommonPoolParallelism() different from
commonPool().getParallelism() ? Seems redundant.


The method exists because of static-init bootstrapping constraints,
and is public just because it is handy in other contexts as well;
for example, JDK or user code might want to adopt sizings commensurate
commonPoolParallelism. As a user of this myself (in ConcurrentHashMap
spliterators), I found that I wanted this method, so figured others
will too.




- I would document the effects of shutdown, shutdownNow, awaitTermination,
isTerminating specific to the common pool on getCommonPool() rather than on
the individual methods. The discussion seems out of place on the individual
methods. Everything about the common pool should be consolidated (or
replicated) on getCommonPool() for one-stop-shopping to understand the
characteristics of the common pool.


Notes about commonPool are also now top-level javadocs as well
as on per-method shutdown etc (where they must be). But given that
at least one reader expected them elsewhere, I'll revise:


/**
 * Returns the common pool instance. This pool is statically
 * constructed; its run state is unaffected by attempts to
 * {@link @shutdown} or {@link #shutdownNow}.
 *
 * @return the common pool instance
 */
public static ForkJoinPool commonPool() ...





- ForkJoinTask - "If the current thread is operating in a ForkJoinPool," and
similar phrases. It doesn't say what happens if it isn't.



I think that all wording along these lines  is now gone?



- CountedCompleter : "Asuuming" -> "Assuming"


Thanks. Already fixed since Chris's last grab.

-Doug

Re: RFR: 8004699 : Add type annotation storage to Constructor, Field and Method

[Peter Levart]

Hi Joel,

  82 // This is set by the vm at Method creation
  83 private byte[]  type_annotations;


Wouldn't it be better to initialize this field in the constructor? You 
could create an overloaded constructor if you wanted to be compatible 
with previous versions of the VM.


Regards, Peter

On 12/17/2012 04:41 PM, Joel Borggrén-Franck wrote:

Here is the first in a series of changes to add type annotation support to 
reflection.

http://cr.openjdk.java.net/~jfranck/8004699/webrev.00/

While this adds overhead all instances of Field, Constructor, and Method we 
need this change in order to coordinate with the changes going in to HotSpot 
There is a plan to remove the additional overhead by consolidating all 
annotation fields, though this will have to wait a bit.

cheers
/Joel

Re: RFR: 8004699 : Add type annotation storage to Constructor, Field and Method

[Alan Bateman]

On 17/12/2012 15:41, Joel Borggrén-Franck wrote:

Here is the first in a series of changes to add type annotation support to 
reflection.

http://cr.openjdk.java.net/~jfranck/8004699/webrev.00/

While this adds overhead all instances of Field, Constructor, and Method we 
need this change in order to coordinate with the changes going in to HotSpot 
There is a plan to remove the additional overhead by consolidating all 
annotation fields, though this will have to wait a bit.

cheers
/Joel
I assume this should be typeAnnotations rather than type_annotations so 
that it's consistent with the existing fields. Have the Hotspot changes 
been pushed already?


-Alan

hg: jdk8/tl/langtools: 8004832: Add new doclint package

[jonathan . gibbons]

Changeset: 75ab654b5cd5
Author:jjg
Date:  2012-12-17 07:47 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/75ab654b5cd5

8004832: Add new doclint package
Reviewed-by: mcimadamore

! make/build.properties
! src/share/classes/com/sun/source/util/DocTrees.java
! src/share/classes/com/sun/source/util/JavacTask.java
! src/share/classes/com/sun/source/util/TreePath.java
+ src/share/classes/com/sun/tools/doclint/Checker.java
+ src/share/classes/com/sun/tools/doclint/DocLint.java
+ src/share/classes/com/sun/tools/doclint/Entity.java
+ src/share/classes/com/sun/tools/doclint/Env.java
+ src/share/classes/com/sun/tools/doclint/HtmlTag.java
+ src/share/classes/com/sun/tools/doclint/Messages.java
+ src/share/classes/com/sun/tools/doclint/resources/doclint.properties
! src/share/classes/com/sun/tools/javac/api/BasicJavacTask.java
! src/share/classes/com/sun/tools/javac/api/JavacTaskImpl.java
! src/share/classes/com/sun/tools/javac/api/JavacTrees.java
! src/share/classes/com/sun/tools/javac/main/JavaCompiler.java
! src/share/classes/com/sun/tools/javac/model/JavacTypes.java
! src/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
! src/share/classes/com/sun/tools/javac/resources/compiler.properties
! src/share/classes/com/sun/tools/javac/tree/DCTree.java
! src/share/classes/com/sun/tools/javac/tree/DocPretty.java
! src/share/classes/com/sun/tools/javac/tree/TreeInfo.java
+ test/tools/doclint/AccessTest.java
+ test/tools/doclint/AccessTest.package.out
+ test/tools/doclint/AccessTest.private.out
+ test/tools/doclint/AccessTest.protected.out
+ test/tools/doclint/AccessTest.public.out
+ test/tools/doclint/AccessibilityTest.java
+ test/tools/doclint/AccessibilityTest.out
+ test/tools/doclint/DocLintTester.java
+ test/tools/doclint/EmptyAuthorTest.java
+ test/tools/doclint/EmptyAuthorTest.out
+ test/tools/doclint/EmptyExceptionTest.java
+ test/tools/doclint/EmptyExceptionTest.out
+ test/tools/doclint/EmptyParamTest.java
+ test/tools/doclint/EmptyParamTest.out
+ test/tools/doclint/EmptyReturnTest.java
+ test/tools/doclint/EmptyReturnTest.out
+ test/tools/doclint/EmptySerialDataTest.java
+ test/tools/doclint/EmptySerialDataTest.out
+ test/tools/doclint/EmptySerialFieldTest.java
+ test/tools/doclint/EmptySerialFieldTest.out
+ test/tools/doclint/EmptySinceTest.java
+ test/tools/doclint/EmptySinceTest.out
+ test/tools/doclint/EmptyVersionTest.java
+ test/tools/doclint/EmptyVersionTest.out
+ test/tools/doclint/HtmlAttrsTest.java
+ test/tools/doclint/HtmlAttrsTest.out
+ test/tools/doclint/HtmlTagsTest.java
+ test/tools/doclint/HtmlTagsTest.out
+ test/tools/doclint/MissingCommentTest.java
+ test/tools/doclint/MissingCommentTest.out
+ test/tools/doclint/MissingParamsTest.java
+ test/tools/doclint/MissingParamsTest.out
+ test/tools/doclint/MissingReturnTest.java
+ test/tools/doclint/MissingReturnTest.out
+ test/tools/doclint/MissingThrowsTest.java
+ test/tools/doclint/MissingThrowsTest.out
+ test/tools/doclint/OptionTest.java
+ test/tools/doclint/OverridesTest.java
+ test/tools/doclint/ReferenceTest.java
+ test/tools/doclint/ReferenceTest.out
+ test/tools/doclint/RunTest.java
+ test/tools/doclint/SyntaxTest.java
+ test/tools/doclint/SyntaxTest.out
+ test/tools/doclint/SyntheticTest.java
+ test/tools/doclint/ValidTest.java
+ test/tools/doclint/tidy/AnchorAlreadyDefined.java
+ test/tools/doclint/tidy/AnchorAlreadyDefined.out
+ test/tools/doclint/tidy/BadEnd.java
+ test/tools/doclint/tidy/BadEnd.out
+ test/tools/doclint/tidy/InsertImplicit.java
+ test/tools/doclint/tidy/InsertImplicit.out
+ test/tools/doclint/tidy/InvalidEntity.java
+ test/tools/doclint/tidy/InvalidEntity.out
+ test/tools/doclint/tidy/InvalidName.java
+ test/tools/doclint/tidy/InvalidName.out
+ test/tools/doclint/tidy/InvalidTag.java
+ test/tools/doclint/tidy/InvalidTag.out
+ test/tools/doclint/tidy/InvalidURI.java
+ test/tools/doclint/tidy/InvalidURI.out
+ test/tools/doclint/tidy/MissingGT.java
+ test/tools/doclint/tidy/MissingGT.out
+ test/tools/doclint/tidy/MissingTag.java
+ test/tools/doclint/tidy/MissingTag.out
+ test/tools/doclint/tidy/NestedTag.java
+ test/tools/doclint/tidy/NestedTag.out
+ test/tools/doclint/tidy/ParaInPre.java
+ test/tools/doclint/tidy/ParaInPre.out
+ test/tools/doclint/tidy/README.txt
+ test/tools/doclint/tidy/RepeatedAttr.java
+ test/tools/doclint/tidy/RepeatedAttr.out
+ test/tools/doclint/tidy/TextNotAllowed.java
+ test/tools/doclint/tidy/TextNotAllowed.out
+ test/tools/doclint/tidy/TrimmingEmptyTag.java
+ test/tools/doclint/tidy/TrimmingEmptyTag.out
+ test/tools/doclint/tidy/UnescapedOrUnknownEntity.java
+ test/tools/doclint/tidy/UnescapedOrUnknownEntity.out
+ test/tools/doclint/tidy/util/Main.java
+ test/tools/doclint/tidy/util/tidy.sh
+ test/tools/javac/diags/examples/NoContent.java

RFR: 8004699 : Add type annotation storage to Constructor, Field and Method

[Joel Borggrén-Franck]

Here is the first in a series of changes to add type annotation support to 
reflection.

http://cr.openjdk.java.net/~jfranck/8004699/webrev.00/

While this adds overhead all instances of Field, Constructor, and Method we 
need this change in order to coordinate with the changes going in to HotSpot 
There is a plan to remove the additional overhead by consolidating all 
annotation fields, though this will have to wait a bit.

cheers
/Joel

Re: RFR: javax.xml.datatype: Using ServiceLoader to load JAXP datatype factories (7169894: JAXP Plugability Layer: using service loader)

[Daniel Fuchs]

Hi,

As pointed out by Paul & Alan, there's no reason to force
Class.forName to initialize the class when attempting to
load it in FactoryFinder.

FactoryFinder should behave like ServiceLoader in this respect.
So here is an updated webrev for javax.xml.datatype.
 



I also reported the changes to javax.xml.parsers:



-- daniel

On 12/12/12 2:08 PM, Daniel Fuchs wrote:

Hi,

Please find below a refreshed webrev which adds a bit of cleanup
suggested by Paul.

Instead of casting the result of newInstance() at several places,
we pass the expected base type to newInstance so that the cast
occurs only once.




-- daniel

Note: I have applied the same cleanup to the parsers package:
javax.xml.parsers:




On 12/11/12 6:47 PM, Daniel Fuchs wrote:

Hi,

Here is a new webrev in the series that addresses using ServiceLoader in
JAXP for JDK 8.

7169894: JAXP Plugability Layer: using service loader

This changeset addresses modification in the javax.xml.datatype
package.
It is similar to changes proposed for the javax.xml.parsers
package [1], with a few differences due to the specificities of
javax.xml.datatype.

Namely:

1. The documentation that describes the loading mechanism is in the
class header rather than in the method documentation - which leads
to some wording changes.

2. The DatatypeFactory is specified to throw a
DatatypeConfigurationException - which is a checked exception,
instead of an Error - as was FactoryConfigurationError

3. DatatypeConfigurationException allows to wrap
ServiceConfigurationError directly - so the additional layer
of RuntimeException is not needed here.





-- daniel

[1] javax.xml.parsers:

EnumData space optimization in j.l.Class (JEP-146)

[Peter Levart]

Hi David and others,

Here's a patch that eliminates one of two fields in java.lang.Class, 
related to caching enum constants:


http://dl.dropbox.com/u/101777488/jdk8-tl/JEP-149.enum/webrev.01/index.html

It does it by moving one field to a subclass of HashMap, which is 
referenced by a remaining field that serves two different 
purposes/stages of caching.


These are the results of a micro-benchmark that exercises public API 
that uses the internal j.l.Class API regarding enum constants:


enum MyEnum { ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, TEN }
EnumSet.noneOf(MyEnum.class): 300_000_000 loops
MyEnum.valueOf(String): 30_000_000 loops * 10 calls for different names

** Original JDK8 code

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
../out/production/test test.EnumTest reference


  EnumSet.noneOf(Class): 351610312 340302968 33989 339774384 
339750612 339558414 339547022 339621595
 MyEnum.valueOf(String): 935153830 897188742 887541353 960839820 
886119463 885818334 885827093 885752461
  EnumSet.noneOf(Class): 339552678 339469528 339513757 339451341 
339512154 339511634 339664326 339793144


** patched java.lang.Class

Executing: /home/peter/Apps64/jdk1.8.0-jdk8-tl/bin/java -Xmx4G -cp 
../out/production/test -Xbootclasspath/p:../out/production/jdk test.EnumTest


  EnumSet.noneOf(Class): 351724931 339286591 305082929 305042885 
305058303 305044144 305073463 305049604
 MyEnum.valueOf(String): 955032718 908534137 891406394 891506147 
891414312 893652469 891412757 891409294
  EnumSet.noneOf(Class): 414044087 406904161 406788898 406839824 
406765274 406815728 407002576 406779162


The slow-down of about 20% (last line) is presumably a consequence of 
another in-direction to obtain shared enum constants array when there is 
already a Map in place. It is still fast though (300M EnumSet instances 
/ 0.4 s).


Here's the source of the micro-benchmark:

https://raw.github.com/plevart/jdk8-tl/JEP-149.enum/test/src/test/EnumTest.java

I don't know what's more important in this occasion. A small space gain 
(8 or 4 bytes per j.l.Class instance) or a small performance gain (20%).


Regards, Peter

Re: Review request: 8003562: Provide a command-line tool to find static dependencies

[Alan Bateman]

On 15/12/2012 01:30, Mandy Chung wrote:


Having a second thought, it's a new tool and we need to learn its 
options anyway.  I'd opt for consistency than familiarity and not to 
have mixture of Unix-style and GNU-style options.  If no objection, I 
will change it to --classpath for the initial push.


Mandy

I looked through the webrev and I think it looks very good.

I mostly agree with your argument that this is a new tool and that we 
should be consistent with GNU-style options. So what was the final bid 
is, is it --classpath or --class-path? I guess it wouldn't do any harm 
to silently supporting -cp and -classpath as folks used to java* tools 
will probably use them without thinking.


One other thing is that jdk.properties is a bit out of date. That's not 
important of course as it will eventually go away.


-Alan

Re: RFR 8005081: java/util/prefs/PrefsSpi.sh fails on macos-x

[Alan Bateman]

On 14/12/2012 17:36, Chris Hegarty wrote:
Strangely, this test passed in my test runs, on all platforms, before 
the push with the changes that pass the TESTVMOPTS, 8003890. It has 
now been seen to fail on some mac machines. There appears to be an 
issue with the use of quotes around TESTVMOPTS. The below change 
resolves the failure on the problem machines, and also continues to 
pass on all other platforms.
I can only guess that sh is a different shell or version. In any case, 
the change looks fine to me.


-Alan

RFR: 7150256: Add back Diagnostic Command JMX API

[Frederic Parain]

Hi,

Please review the following change for CR 7150256.

This changeset is the JDK side of two parts project.

The goal of this feature is to allow remote invocations of
Diagnostic Commands via JMX.

Diagnostic Commands are actually invoked from the jcmd
tool, which is a local tool using the attachAPI. It was
not possible to remotely invoke these commands.
This changeset creates a new PlatformMBean, remotely
accessible via the Platform MBean Server, providing
access to Diagnostic Commands too.

The set of supported Diagnostic Commands varies with
the JVM version, command line options and even some
runtime operations. The new PlatformMBean, called
DiagnosticCommandsMBean is not statically defined,
but computes its content dynamically at runtime.
This is why the MBeanInfo returned by this new
MBean contains a lot of meta-data describing
each supported Diagnostic Command (name, help
message, syntax, etc.)

Because this feature allows remote invocations, it
is important to be able to control who is invoking
and what is invoked. Java Permission checks have
been introduced before a remote invocation request
is forwarded to the JVM. So, each Diagnostic Command
can require a Java Permission to be invoked remotely.
The name of the required Java Permission is provided
in the meta-data in the MBeanInfo.
The security configuration is done using a Security
Manager and the regular JMX configurations. Some
existing Diagnostic Commands have been extended to
specify which Java Permission they require.

The webrev is here:

http://cr.openjdk.java.net/~fparain/7150256/webrev.00/

The second part of this project is the support for this
feature in the HotSpot VM, and is tracked by CR 8004095
(separate request for review will be sent for it).

Thanks,

Fred

--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: frederic.par...@oracle.com

hg: jdk8/tl/jdk: 8004928: TEST_BUG: Reduce dependence of CoreLib tests from the AWT subsystem

[alexey . utkin]

Changeset: a02212de8db6
Author:uta
Date:  2012-12-17 14:34 +0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a02212de8db6

8004928: TEST_BUG: Reduce dependence of CoreLib tests from the AWT subsystem
Summary: the tests were refactored to drop AWT dependence where it was possible.
Reviewed-by: alanb, mchung

! test/java/io/Serializable/resolveProxyClass/NonPublicInterface.java
! test/java/lang/Throwable/LegacyChainedExceptionSerialization.java
! test/java/lang/management/CompilationMXBean/Basic.java
! test/java/lang/reflect/Generics/Probe.java
! test/java/lang/reflect/Proxy/ClassRestrictions.java
! test/java/util/Collections/EmptyIterator.java
! test/java/util/logging/LoggingDeadlock4.java
! test/sun/tools/jrunscript/common.sh

Re: ReflectionData space optimization in java.lang.Class (JEP-149)

[Peter Levart]

Hi David,

Fair enough. So only the basic ReflectionData patch for now. Here I 
prepared yet another revision that only contains the basic stuff from 
1st revision of the patch but fixes long lines, splits the 
reflectionData method in two and also fixes an inconsistency - in one 
method there was a check left for "if (useCaches) ..." instead of "if 
(rd != null) ..."


http://dl.dropbox.com/u/101777488/jdk8-tl/JEP-149.c/webrev.04/index.html

Regards, Peter

P.S.

There might be an opportunity for further space optimization in 
java.lang.Class. There are two fields: enumConstants and 
enumConstantDirectory that are only used in Class instances representing 
Enum classes. I think the ratio of Enum classes vs. non-Enum classes in 
typical apps is strongly in favour of non-Enum classes. These two fields 
could be replaced with a single field pointing to "EnumData". Do you 
think it's worth it?



On 12/16/2012 10:20 PM, David Holmes wrote:

Hi Peter,

We have to be careful not to disrupt the dynamics of things too much. 
Duplicate copies wastes memory but doing the replacement wastes time. 
If we could be purely memory focused then we would do anything to save 
memory, but we can't do that - we're trying to save some memory 
without being too disruptive to performance aspects. The more changes 
like this that are made the less idea we have about the impact on 
existing reflection-using applications. And we don't have any 
compelling use-case to motivate this. So I'm inclined to not take this 
additional step at this time.


Thanks,
David

On 17/12/2012 3:26 AM, Peter Levart wrote:

Hi David, Mandy, Joel and others,

I prepared the 3rd revision of the patch:

http://dl.dropbox.com/u/101777488/jdk8-tl/JEP-149.c/webrev.03/index.html

Changes from the 1st revision (disregard 2nd revision) are as follows:

- The split of reflectionData() method into short fast-path and longer
slow-path newReflectionData() as already proposed in 2nd revision of the
patch.
- The logic of privateGetDeclared[Fields|Methods|Constructors](boolean
publicOnly) methods has been rewritten to eliminate caching of duplicate
Field/Method/Constructor instances for the same field, method or
constructor.

As it turns out, the same public Member can be cached twice. One
instance in ReflectionData.declared[Fields|Methods|Constructors] array
and the other instance in ReflectionData.declaredPublic[Fields|Methods]
or publicConstructors array. These arrays pairs
(declared/declaredPublic) retain distinct instances, because they are
obtained by separate calls to VM which always constructs new instances.

The new proposed logic eliminates duplicate instances, but does not
otherwise present any new overhead. It never requests more data from VM
then needed at a particular moment (it may request less data from VM if
external calls are issued in a particular order).

Some applications may benefit from saved allocated space if they request
"declared" members and "public" members on same Class instances.

Here's the comparison of bytes allocated for the ReflectionData object
in some Class instances after invoking particular reflection methods:

***
*** Original JDK8 privateGetDeclaredXXX methods:
***

Deep size of ReflectionData in: java.lang.Object.class

before any calls: 0 bytes
after getDeclaredConstructors: 192 bytes
after getDeclaredFields: 192 bytes
after getDeclaredMethods: 1,552 bytes
after getConstructors: 1,664 bytes
after getFields: 1,696 bytes
after getMethods: 2,856 bytes

Deep size of ReflectionData in: java.lang.String.class

before any calls: 704 bytes
after getDeclaredConstructors: 2,472 bytes
after getDeclaredFields: 2,472 bytes
after getDeclaredMethods: 10,808 bytes
after getConstructors: 12,464 bytes
after getFields: 12,712 bytes
after getMethods: 21,056 bytes

Deep size of ReflectionData in: java.util.HashMap.class

before any calls: 0 bytes
after getDeclaredConstructors: 472 bytes
after getDeclaredFields: 1,328 bytes
after getDeclaredMethods: 5,856 bytes
after getConstructors: 6,360 bytes
after getFields: 6,392 bytes
after getMethods: 9,576 bytes

Deep size of ReflectionData in: javax.swing.JTable.class

before any calls: 0 bytes
after getDeclaredConstructors: 784 bytes
after getDeclaredFields: 4,224 bytes
after getDeclaredMethods: 26,072 bytes
after getConstructors: 26,792 bytes
after getFields: 28,600 bytes
after getMethods: 79,912 bytes


***
*** With modified privateGetDeclaredXXX methods:
***

Deep size of ReflectionData in: java.lang.Object.class

before any calls: 0 bytes
after getDeclaredConstructors: 192 bytes
after getDeclaredFields: 192 bytes
after getDeclaredMethods: 1,552 bytes
after getConstructors: 1,552 bytes
after getFields: 1,568 bytes
after getMethods: 1,680 bytes

Deep size of ReflectionData in: java.lang.String.class

before any calls: 0 bytes
after getDeclaredConstructors: 1,816 bytes
after getDeclaredFields: 2,368 bytes
after getDeclaredMethods: 10,704 bytes
after getConstructors: 10,784 bytes
after getFields: 10,8