hg: jigsaw/jake/jdk: 2 new changesets

[mandy . chung]

Changeset: b27959538721
Author:sdrach
Date:  2016-10-28 13:00 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/b27959538721

Port changeset for 8164805: Fail to create a MR modular JAR with a versioned 
entry of a concealed package
Reviewed-by: mchung

! src/jdk.jartool/share/classes/sun/tools/jar/Main.java
! src/jdk.jartool/share/classes/sun/tools/jar/Validator.java
! src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties
+ test/tools/jar/mmrjar/ConcealedPackage.java
+ test/tools/jar/mmrjar/src/classes/p/Hi.java
+ test/tools/jar/mmrjar/src/mr10/p/internal/bar/Gee.java
+ test/tools/jar/mmrjar/src/mr9/module-info.java
+ test/tools/jar/mmrjar/src/mr9/p/Hi.java
+ test/tools/jar/mmrjar/src/mr9/p/internal/Bar.java

Changeset: 93fe3e95251c
Author:mchung
Date:  2016-10-28 21:07 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/93fe3e95251c

Update jar tool warning message

! src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties

Re: Request Review: JDK-6479237 (cl) Add support for classloader names

[Brent Christian]

On 10/28/16 1:44 PM, Mandy Chung wrote:



On Oct 28, 2016, at 11:11 AM, Brent Christian  
wrote:

Should something be done for STEs returned from 
StackFrameInfo.toStackTraceElement() ?


Good catch - I missed it.  I added package-private static methods in 
StackTraceElement class for both Throwable and StackFrameInfo to get 
StackTraceElement(s).

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/webrev.02/



Looks good.

-Brent

Re: New proposal for #ReflectiveAccessToNonExportedTypes: Open modules & open packages

[John Rose]

On Oct 27, 2016, at 9:07 AM, mark.reinh...@oracle.com wrote:
> 
> Further comments most welcome, as usual!


+100 for *qualified* opens; this puts an important limit on deep reflection.

My main concern with reflection is to avoid "falling off the encapsulation 
cliff"
the first time a user module wishes to open itself up for deep reflection.
By such a "cliff" I mean that when deep reflection is allowed, potentially
any name in the module can be inspected.  Even of only 0.01% of names
are actually inspected, tools for reorganizing code must assume that 100%
of the names *might* be inspected, at some point in the future, by the
allowed party.

So qualified opens is a partial solution.  And, I think it is exactly right for
today, because it can be extended tomorrow.  I.e., it is future-friendly.
I'll explain by sketching a concept called "moderate reflection", or "MR".

A fuller solution would allow other tools to make conclusions about more
specific limitations on the actual extent of the deep reflection.   Call such
an extent-limited reflection moderate reflection, where "moderation" is
declared statically by some sort of flexible declarative rule set.  In this way
tools could make more detailed conclusions about the encapsulation of
particular names in the module.

 For future JDK releases we can consider layering  of this qualified opens,
by creating a trusted, parameterized meta-framework ("Moderate Reflection")
to which a user module can be qualified-open.  This meta-framework can
then perform deep reflection on behalf of self-describing "moderately 
reflecting"
client frameworks.  User modules would not directly open themselves to any
module other than MR.

The advantage would come when the client frameworks explicitly declare their
self-restraint ("moderation") in reflection.  For example, the "Foo" framework
might limit itself to names annotated with @FooFrameworkHook.

An AOT compiler (or other jlink-time reorganization tool) would (1) note that
some user module bar is qualified-open only to Moderate Reflection, and
has an export (maybe qualified) to a Foo framework.  At runtime, (2) the
Foo framework would ask Moderate Reflection to reflect into bar.
Moderate Reflection would (3) check the self-limitation on Foo, and also
ensure that bar exports (though doesn't open) to Foo.  MR would then
gain the requested access and delegate it back to Foo.  Finally, (4) the
AOT compiler would free itself to omit metadata for unexported names
not visible to moderate reflection.

(Method handles are a good way to grant access, since they can
be delegated from MR to Foo without re-authorizing Foo.  Core
Reflection re-authorizes on every access, which awkwardly
requires deep reflection on usage, as well as initial lookup.)

Moderate Reflection might also look at metadata on the bar module
(besides its module exports) to impose further limits on reflection.
Thus, MR-specific metadata might prevent MR from granting bar
access to normally-trusted frameworks that bar doesn't trust, if
bar can blacklist them for itself in a place that MR checks.

And so on.  None of this needs to be built into the module system.
To gain the benefit of it, basic parametric policies need to be implemented,
adopted by client frameworks, and used by tools to perform optimizations.

A typical optimization is "tree shaking", where a class or method that is
never used is dropped from the application.  Or, if it is used in a way that
can be inlined locally, the metadata can still be dropped.  In either case,
making code go dead is an important ingredient to many optimizations.

— John

Re: Request Review: JDK-6479237 (cl) Add support for classloader names

[Mandy Chung]

> On Oct 28, 2016, at 12:06 AM, serguei.spit...@oracle.com wrote:
> 
>   Can we give the bootstrap classloader the name "boot" or "bootstrap”?

BootClassLoader is not the boostrap class loader but instead it's 
implementation details.  The bootstrap ClassLoader instance is null and so you 
can’t invoke ClassLoader::getName.

>   
> Also, the lines 415 and 422 can be simplified: 415 s += m.getName(); 422 s += 
> version;

OK.  At one point, that was how it was coded.  

> Also, if the loader has a name but (m == null || !m.isNamed())  then it looks 
> like the sign "/" will be added twice (see L410 and L428). It can be fixed 
> and simplified with: Add line before 425: s += "/"; 428 return s + 
> cls.getName();

“//” is correct.

> 
>  Also, it is not clear why the loader name is not included for an instance of 
> theBuiltinClassLoader?

Make the output compact when it can, for example, the class loader name “app” 
and “platform” from classes from the JDK can be implied.

>  Would it make sense to add a comment explaining it?

Maybe not much to add that.

Mandy

Re: Size difference of Java.base image between OSX and Linux

[David Holmes]

On 29/10/2016 3:32 AM, Waldek Kozaczuk wrote:


I used jlink to create minimal custom JRE image on OSX and Linux and I noticed 
there is a significant difference in size. The Mac version takes 21M where the 
Linux one takes 30M which means Linux version is 50% bigger than Mac.

Is it as you would have expected? Why is size difference so big?

I noticed that lib/modules is similar in size (1M difference) but the biggest 
difference is between ./lib/server/libjvm.dylib (12 M) and 
./lib/amd64/server/libjvm.so (21 M). So the linux version of the shared library 
object is almost twice as big on linux (both 64 bit).


Nothing jlink related. This is a question for the build-dev and/or 
hotspot-dev folk. It seems that in 8u libjvm is <1MB larger on Linux 
than OSX. However in 9 the Linux version has steadily grown to ~21M 
while the OSX version has shrunk to ~12M. It may be related to the 
debugging/symbol info still present in the library.


David


Here is the command I used to create the image:
jdk-9/bin/jlink --module-path jdk-9/jmods --add-modules java.base --output 
image_target --strip-debug --compress=2

Here is a detailed listing of files on Mac:
68K ./bin/java 68K  ./bin/keytool 8.0K  ./conf/net.properties 4.0K  
./conf/security/java.policy 40K ./conf/security/java.security 4.0K  
./conf/security/policy/limited/default_local.policy 4.0K
./conf/security/policy/limited/default_US_export.policy 4.0K
./conf/security/policy/limited/exempt_local.policy 4.0K 
./conf/security/policy/README.txt 4.0K  
./conf/security/policy/unlimited/default_local.policy 4.0K  
./conf/security/policy/unlimited/default_US_export.policy 20K   
./include/classfile_constants.h 4.0K./include/darwin/jni_md.h 76K   
./include/jni.h 80K ./include/jvmti.h 4.0K  ./include/jvmticmlr.h 68K   
./lib/jli/libjli.dylib 16K  ./lib/jspawnhelper 4.0K ./lib/jvm.cfg 204K  
./lib/libjava.dylib 32K ./lib/libjimage.dylib 12K   ./lib/libjsig.dylib 92K 
./lib/libnet.dylib 68K  ./lib/libnio.dylib 44K  ./lib/libosxsecurity.dylib 48K  
./lib/libverify.dylib 32K   ./lib/libzip.dylib 7.2M ./lib/modules 4.0K  
./lib/security/blacklist 4.0K   ./lib/security/blacklisted.certs 112K   
./lib/security/cacerts 8.0K ./lib/security/default.policy 0B
./lib/security/trusted.libraries 12K./lib/server/libjsig.dylib 12M  
./lib/server/libjvm.dylib 4.0K  ./lib/server/Xusage.txt 104K./lib/tzdb.dat 
4.0K ./release

Here is a detailed listing of files on Linux:

4.0K./lib/security/blacklisted.certs
8.0K./lib/security/default.policy
4.0K./lib/security/blacklist
0   ./lib/security/trusted.libraries
104K./lib/tzdb.dat
12K ./lib/jexec
4.0K./lib/amd64/jvm.cfg
108K./lib/amd64/libnet.so
72K ./lib/amd64/jli/libjli.so
132K./lib/amd64/libjimage.so
220K./lib/amd64/libjava.so
36K ./lib/amd64/libzip.so
21M ./lib/amd64/server/libjvm.so
4.0K./lib/amd64/server/Xusage.txt
12K ./lib/amd64/server/libjsig.so
88K ./lib/amd64/libnio.so
12K ./lib/amd64/libjsig.so
64K ./lib/amd64/libverify.so
8.2M./lib/modules
4.0K./release
4.0K./include/jvmticmlr.h
4.0K./include/linux/jni_md.h
20K ./include/classfile_constants.h
80K ./include/jvmti.h
76K ./include/jni.h
4.0K./conf/security/policy/limited/default_local.policy
4.0K./conf/security/policy/limited/default_US_export.policy
4.0K./conf/security/policy/limited/exempt_local.policy
4.0K./conf/security/policy/README.txt
4.0K./conf/security/policy/unlimited/default_local.policy
4.0K./conf/security/policy/unlimited/default_US_export.policy
40K ./conf/security/java.security
4.0K./conf/security/java.policy
8.0K./conf/net.properties
12K ./bin/java
12K ./bin/keytool

Regards,
Waldek

Sent from my iPhone

Re: Request Review: JDK-6479237 (cl) Add support for classloader names

[David Holmes]

Hi Mandy,

I know it's rather late in the game to notice this but I only just 
noticed this due to Serguei's comment ...


On 28/10/2016 5:06 PM, serguei.spit...@oracle.com wrote:

Hi Mandy,


I have a few comments.

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/webrev.00/jdk/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java.udiff.html


 private static class BootClassLoader extends BuiltinClassLoader {
 BootClassLoader(URLClassPath bcp) {
- super(null, bcp);
+ super(null, null, bcp);
 }
. . .

 PlatformClassLoader(BootClassLoader parent) {
- super(parent, null);
+ super("platform", parent, null);
 }

. . .

 AppClassLoader(PlatformClassLoader parent, URLClassPath ucp) {
- super(parent, ucp);
+ super("app", parent, ucp);
 this.ucp = ucp;
 }


   Can we give the bootstrap classloader the name "boot" or "bootstrap"?
   Or this will impact too many places, and so, very risky to do?


Given the BootClassLoader instance is not in fact the boot loader at all 
I think it would have been clearer and avoid potential confusion to call 
this something more representative of its purpose - perhaps 
BootResourceloader or BootLoaderHelper or ...


Thanks,
David




http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/webrev.00/jdk/src/java.base/share/classes/java/lang/StackTraceElement.java.frames.html


379 ClassLoader loader = cls.getClassLoader0(); The loader is unused.
402 private static String toLoaderModuleClassName(Class cls) {
403 ClassLoader loader = cls.getClassLoader0();
404 Module m = cls.getModule();
405
406 // First element - class loader name
407 String s = "";
408 if (loader != null && !(loader instanceof BuiltinClassLoader) &&
409 loader.getName() != null) {
410 s = loader.getName() + "/";
411 }
412
413 // Second element - module name and version
414 if (m != null && m.isNamed()) {
415 s = s.isEmpty() ? m.getName() : s + m.getName();
416 // drop version if it's JDK module tied with java.base,
417 // i.e. non-upgradeable
418 if (!HashedModules.contains(m)) {
419 Optional ov = m.getDescriptor().version();
420 if (ov.isPresent()) {
421 String version = "@" + ov.get().toString();
422 s = s.isEmpty() ? version : s + version;
423 }
424 }
425 }
426
427 // fully-qualified class name
428 return s.isEmpty() ? cls.getName() : s + "/" + cls.getName();
429 }
Also, the lines 415 and 422 can be simplified: 415 s += m.getName(); 422
s += version; Also, if the loader has a name but (m == null ||
!m.isNamed())  then it looks like the sign "/" will be added twice (see
L410 and L428). It can be fixed and simplified with: Add line before
425: s += "/"; 428 return s + cls.getName();

  Also, it is not clear why the loader name is not included for an
instance of theBuiltinClassLoader?
  Would it make sense to add a comment explaining it?

Thanks, Serguei

On 10/25/16 16:10, Mandy Chung wrote:

Webrev at:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/webrev.00/

Specdiff:

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/specdiff/overview-summary.html


This is a long-standing RFE for adding support for class
loader names.  It's #ClassLoaderNames on JSR 376 issue
list where the proposal [1] has been implemented in jake
for some time.  This patch brings this change to jdk9.

A short summary:
- New constructors are added in ClassLoader, SecureClassLoader
   and URLClassLoader to specify the class loader name.

- New ClassLoader::getName and StackTraceElement::getClassLoaderName
   method

- StackTraceElement::toString is updated to include the name
   of the class loader and module of that frame in this format:
  //(:)

The detail is in StackTraceElement::buildLoaderModuleClassName
that compress the output string for cases when the loader
has no name or the module is unnamed module.  Another thing
to mention is that VM sets the Class object when filling in
a stack trace of a Throwable object.  Then the library will
build a String from the Class object for serialization purpose.

Mandy
[1]
http://mail.openjdk.java.net/pipermail/jpms-spec-observers/2016-September/000550.html

Re: Request Review: JDK-6479237 (cl) Add support for classloader names

[Mandy Chung]

> On Oct 28, 2016, at 11:11 AM, Brent Christian  
> wrote:
> 
> Should something be done for STEs returned from 
> StackFrameInfo.toStackTraceElement() ?

Good catch - I missed it.  I added package-private static methods in 
StackTraceElement class for both Throwable and StackFrameInfo to get 
StackTraceElement(s).

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/webrev.02/

Mandy

Size difference of Java.base image between OSX and Linux

[Waldek Kozaczuk]

I used jlink to create minimal custom JRE image on OSX and Linux and I noticed 
there is a significant difference in size. The Mac version takes 21M where the 
Linux one takes 30M which means Linux version is 50% bigger than Mac.

Is it as you would have expected? Why is size difference so big?

I noticed that lib/modules is similar in size (1M difference) but the biggest 
difference is between ./lib/server/libjvm.dylib (12 M) and 
./lib/amd64/server/libjvm.so (21 M). So the linux version of the shared library 
object is almost twice as big on linux (both 64 bit).

Here is the command I used to create the image:
jdk-9/bin/jlink --module-path jdk-9/jmods --add-modules java.base --output 
image_target --strip-debug --compress=2

Here is a detailed listing of files on Mac:
68K ./bin/java 68K  ./bin/keytool 8.0K  ./conf/net.properties 4.0K  
./conf/security/java.policy 40K ./conf/security/java.security 4.0K  
./conf/security/policy/limited/default_local.policy 4.0K
./conf/security/policy/limited/default_US_export.policy 4.0K
./conf/security/policy/limited/exempt_local.policy 4.0K 
./conf/security/policy/README.txt 4.0K  
./conf/security/policy/unlimited/default_local.policy 4.0K  
./conf/security/policy/unlimited/default_US_export.policy 20K   
./include/classfile_constants.h 4.0K./include/darwin/jni_md.h 76K   
./include/jni.h 80K ./include/jvmti.h 4.0K  ./include/jvmticmlr.h 68K   
./lib/jli/libjli.dylib 16K  ./lib/jspawnhelper 4.0K ./lib/jvm.cfg 204K  
./lib/libjava.dylib 32K ./lib/libjimage.dylib 12K   ./lib/libjsig.dylib 92K 
./lib/libnet.dylib 68K  ./lib/libnio.dylib 44K  ./lib/libosxsecurity.dylib 48K  
./lib/libverify.dylib 32K   ./lib/libzip.dylib 7.2M ./lib/modules 4.0K  
./lib/security/blacklist 4.0K   ./lib/security/blacklisted.certs 112K   
./lib/security/cacerts 8.0K ./lib/security/default.policy 0B
./lib/security/trusted.libraries 12K./lib/server/libjsig.dylib 12M  
./lib/server/libjvm.dylib 4.0K  ./lib/server/Xusage.txt 104K./lib/tzdb.dat 
4.0K ./release

Here is a detailed listing of files on Linux:

4.0K./lib/security/blacklisted.certs
8.0K./lib/security/default.policy
4.0K./lib/security/blacklist
0   ./lib/security/trusted.libraries
104K./lib/tzdb.dat
12K ./lib/jexec
4.0K./lib/amd64/jvm.cfg
108K./lib/amd64/libnet.so
72K ./lib/amd64/jli/libjli.so
132K./lib/amd64/libjimage.so
220K./lib/amd64/libjava.so
36K ./lib/amd64/libzip.so
21M ./lib/amd64/server/libjvm.so
4.0K./lib/amd64/server/Xusage.txt
12K ./lib/amd64/server/libjsig.so
88K ./lib/amd64/libnio.so
12K ./lib/amd64/libjsig.so
64K ./lib/amd64/libverify.so
8.2M./lib/modules
4.0K./release
4.0K./include/jvmticmlr.h
4.0K./include/linux/jni_md.h
20K ./include/classfile_constants.h
80K ./include/jvmti.h
76K ./include/jni.h
4.0K./conf/security/policy/limited/default_local.policy
4.0K./conf/security/policy/limited/default_US_export.policy
4.0K./conf/security/policy/limited/exempt_local.policy
4.0K./conf/security/policy/README.txt
4.0K./conf/security/policy/unlimited/default_local.policy
4.0K./conf/security/policy/unlimited/default_US_export.policy
40K ./conf/security/java.security
4.0K./conf/security/java.policy
8.0K./conf/net.properties
12K ./bin/java
12K ./bin/keytool

Regards,
Waldek

Sent from my iPhone

Re: RFR: 8156499 Update jlink to support creating images with modules that are packaged as multi-release JARs

[Alan Bateman]

On 28/10/2016 01:22, Steve Drach wrote:
I’ve put out another webrev, 
http://cr.openjdk.java.net/~sdrach/8156499/webrev.05/ 
, that 
addresses Mandy’s concerns.  In particular I demonstrate that the 
resultant image is “runnable” and that a Main class in the image 
can/cannot find the java.logging module when the module-info.class is 
changed to require java.logging in one case but not in the other.
The changes to jlink look okay (same as previous round). I briefly 
looked at the updated test and it looks like it does all the right 
checking. Mandy had detailed comments on the tests so I'll leave that to 
her.


Once this is in then the next step will be have jlink find the java.base 
module and then use its version as runtime version for the module finder 
that finds the modules to link into the image. The changes to jlink 
should be straight-forward but testing will be complicated. I only 
mention now in case it you are thinking of re-using the current test for 
the second phase.


-Alan

Re: Request Review: JDK-6479237 (cl) Add support for classloader names

[David M. Lloyd]

On 10/27/2016 10:02 PM, Mandy Chung wrote:



On Oct 27, 2016, at 8:04 AM, David M. Lloyd  wrote:

OK Thanks.  I was looking at the StackTraceElement#toString JavaDoc and it was 
not very clear if this was the expected output; maybe it's worth pointing out 
explicitly.


http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/javadoc/java/lang/StackTraceElement.html

Is this verion better?


Perfect, thanks.
--
- DML

Re: New proposal for #ReflectiveAccessToNonExportedTypes: Open modules & open packages

[Sander Mak]

No specific comments (yet), but wanted to express a sincere thanks for taking 
into account the community feedback around this issue. Things are looking good.


Sander

> On 27 Oct 2016, at 18:07, mark.reinh...@oracle.com wrote:
> 
> Thanks to everyone who commented on the weak-modules proposal.  I've
> just posted a third proposal for #ReflectiveAccessToNonExportedTypes,
> along with an analysis of the proposals presented thus far:
> 
>  
> http://mail.openjdk.java.net/pipermail/jpms-spec-experts/2016-October/000430.html
>  
> http://mail.openjdk.java.net/pipermail/jpms-spec-experts/2016-October/000431.html
> 
> Further comments most welcome, as usual!
> 
> - Mark

Re: New proposal for #ReflectiveAccessToNonExportedTypes: Open modules & open packages

[Alan Bateman]

On 27/10/2016 19:10, Alan Snyder wrote:


:
I’m wondering if it would be possible to open the package(s) used by Swing Look 
and Feels, to better support custom look and feels.

The most obvious candidates are the packages containing the Portable Look and 
Feel. The PLAF design is somewhat schizophrenic. It is clearly designed for 
reuse, yet it has encapsulations that sometimes get in the way of reuse. (You 
can see examples in the Aqua LAF, originally coded by Apple, where there are 
workarounds with comments like “bad Sun”.)

Other candidates are AWT support packages, which I have found useful in 
implementing a custom platform-specific look and feel.

The basic issue for my look and feel is whether I will need to include 
instructions for developers on special command line arguments to allow the code 
to run in JDK 9. Obviously, I would rather not have to do this.

These are really topics for swing-dev and awt-dev. All the supported 
APIs in the java.desktop module are already exported unconditionally. 
The packages for those APIs are not proposed to be open. AFAIK, the name 
of the LAFs are a supported interface (as the Swing API requires the 
class name) but directly using or extending is not. JEP 272 [1] defines 
several new APIs for desktop applications that would have traditionally 
needed to use unsupported APIs. In addition, Phil Race has proposed a 
jdk.desktop module [2] to provide a solution for yet more cases where 
applications have been hacking into JDK internals.


-Alan

[1] http://openjdk.java.net/jeps/272
[2] http://mail.openjdk.java.net/pipermail/awt-dev/2016-October/012089.html

Re: Request Review: JDK-6479237 (cl) Add support for classloader names

[serguei.spit...@oracle.com]

Hi Mandy,


I have a few comments.

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/webrev.00/jdk/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java.udiff.html

 private static class BootClassLoader extends BuiltinClassLoader {
 BootClassLoader(URLClassPath bcp) {
- super(null, bcp);
+ super(null, null, bcp);
 }
. . .

 PlatformClassLoader(BootClassLoader parent) {
- super(parent, null);
+ super("platform", parent, null);
 }

. . .

 AppClassLoader(PlatformClassLoader parent, URLClassPath ucp) {
- super(parent, ucp);
+ super("app", parent, ucp);
 this.ucp = ucp;
 }


   Can we give the bootstrap classloader the name "boot" or "bootstrap"?
   Or this will impact too many places, and so, very risky to do?



http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/webrev.00/jdk/src/java.base/share/classes/java/lang/StackTraceElement.java.frames.html

379 ClassLoader loader = cls.getClassLoader0(); The loader is unused.
402 private static String toLoaderModuleClassName(Class cls) {
403 ClassLoader loader = cls.getClassLoader0();
404 Module m = cls.getModule();
405
406 // First element - class loader name
407 String s = "";
408 if (loader != null && !(loader instanceof BuiltinClassLoader) &&
409 loader.getName() != null) {
410 s = loader.getName() + "/";
411 }
412
413 // Second element - module name and version
414 if (m != null && m.isNamed()) {
415 s = s.isEmpty() ? m.getName() : s + m.getName();
416 // drop version if it's JDK module tied with java.base,
417 // i.e. non-upgradeable
418 if (!HashedModules.contains(m)) {
419 Optional ov = m.getDescriptor().version();
420 if (ov.isPresent()) {
421 String version = "@" + ov.get().toString();
422 s = s.isEmpty() ? version : s + version;
423 }
424 }
425 }
426
427 // fully-qualified class name
428 return s.isEmpty() ? cls.getName() : s + "/" + cls.getName();
429 }
Also, the lines 415 and 422 can be simplified: 415 s += m.getName(); 422 
s += version; Also, if the loader has a name but (m == null || 
!m.isNamed())  then it looks like the sign "/" will be added twice (see 
L410 and L428). It can be fixed and simplified with: Add line before 
425: s += "/"; 428 return s + cls.getName();


  Also, it is not clear why the loader name is not included for an instance of 
theBuiltinClassLoader?
  Would it make sense to add a comment explaining it?

Thanks, Serguei

On 10/25/16 16:10, Mandy Chung wrote:

Webrev at:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/webrev.00/

Specdiff:

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/specdiff/overview-summary.html

This is a long-standing RFE for adding support for class
loader names.  It's #ClassLoaderNames on JSR 376 issue
list where the proposal [1] has been implemented in jake
for some time.  This patch brings this change to jdk9.

A short summary:
- New constructors are added in ClassLoader, SecureClassLoader
   and URLClassLoader to specify the class loader name.

- New ClassLoader::getName and StackTraceElement::getClassLoaderName
   method

- StackTraceElement::toString is updated to include the name
   of the class loader and module of that frame in this format:
  //(:)

The detail is in StackTraceElement::buildLoaderModuleClassName
that compress the output string for cases when the loader
has no name or the module is unnamed module.  Another thing
to mention is that VM sets the Class object when filling in
a stack trace of a Throwable object.  Then the library will
build a String from the Class object for serialization purpose.

Mandy
[1] 
http://mail.openjdk.java.net/pipermail/jpms-spec-observers/2016-September/000550.html