Extending java.base module
Hi Jigsaw experts, as you might or might not know, we have an own JDK implementation with some extension code that is quite interwoven with the jdk. Now I'm looking into how this coding can be spread into a good module structure for jdk9. And I'm not a crack yet on using the module system though I've read quite a bit into the spec documentation available so far;-) The first point for me is that we have to place some of our coding in the java.base module as we used to add private fields and methods to basic classes such as java.lang.Thread or java.lang.Exception. However, I don't want to have so much of our stuff in java.base and rather think that it should live in its own module. So the question here is if it is possible to call code of other modules from java.base, e.g. via the Service Provider interface? I see that I can define a service in java.base and specify some "uses" statement in module-info. But will my implementation of such a service from other modules be available to java.base? Also I'm contemplating about this requirement: I have a class which I would need somewhere in java.base but I'd also like to export it in the public API of my own extension module. So, if I create the class in java.base, I'm not allowed to export this class publicly, unqualified, right? But when I have it living in my extension module, then java.base would not see it. What can I do? Probably create some inherited class in my extension module that extends from the java.base impl and export this?? I'm hoping that those are easy questions for you and you can give me some helpful answers. Thanks a lot in advance!! Best regards Christoph
Re: Extending java.base module
Disclaimer: I am not a jigsaw expert. The provides/uses mechanism is certainly more formal, but you can also do http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d282c1a8d20b. --Max On 02/15/2017 04:36 PM, Langer, Christoph wrote: Hi Jigsaw experts, as you might or might not know, we have an own JDK implementation with some extension code that is quite interwoven with the jdk. Now I'm looking into how this coding can be spread into a good module structure for jdk9. And I'm not a crack yet on using the module system though I've read quite a bit into the spec documentation available so far;-) The first point for me is that we have to place some of our coding in the java.base module as we used to add private fields and methods to basic classes such as java.lang.Thread or java.lang.Exception. However, I don't want to have so much of our stuff in java.base and rather think that it should live in its own module. So the question here is if it is possible to call code of other modules from java.base, e.g. via the Service Provider interface? I see that I can define a service in java.base and specify some "uses" statement in module-info. But will my implementation of such a service from other modules be available to java.base? Also I'm contemplating about this requirement: I have a class which I would need somewhere in java.base but I'd also like to export it in the public API of my own extension module. So, if I create the class in java.base, I'm not allowed to export this class publicly, unqualified, right? But when I have it living in my extension module, then java.base would not see it. What can I do? Probably create some inherited class in my extension module that extends from the java.base impl and export this?? I'm hoping that those are easy questions for you and you can give me some helpful answers. Thanks a lot in advance!! Best regards Christoph
Re: Extending java.base module
> On 15 Feb 2017, at 08:51, Weijun Wang wrote: > > Disclaimer: I am not a jigsaw expert. > > The provides/uses mechanism is certainly more formal, but you can also do > http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d282c1a8d20b. This is, at best, a hack. The use of Services is a better approach, where possible. -Chris.
RE: Extending java.base module
Hi Chris, Max, thanks for your quick answers. So the service approach seems to fit quite well. But can I assume that my service implementation will be available already at "bootstrap time" of the JDK? E.g. if I need to register/reach my service already at the early stages of JVM initialization, e.g. when a class java.lang.Thread gets initialized, can I assume a service from my extension module would be available? Thanks, Christoph > -Original Message- > From: Chris Hegarty [mailto:chris.hega...@oracle.com] > Sent: Mittwoch, 15. Februar 2017 10:04 > To: Weijun Wang > Cc: Langer, Christoph ; jigsaw- > d...@openjdk.java.net > Subject: Re: Extending java.base module > > > > On 15 Feb 2017, at 08:51, Weijun Wang wrote: > > > > Disclaimer: I am not a jigsaw expert. > > > > The provides/uses mechanism is certainly more formal, but you can also do > http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d282c1a8d20b. > > This is, at best, a hack. The use of Services is a better approach, where > possible. > > -Chris.
Re: Extending java.base module
On 15/02/2017 08:36, Langer, Christoph wrote: Hi Jigsaw experts, as you might or might not know, we have an own JDK implementation with some extension code that is quite interwoven with the jdk. Now I'm looking into how this coding can be spread into a good module structure for jdk9. And I'm not a crack yet on using the module system though I've read quite a bit into the spec documentation available so far;-) The first point for me is that we have to place some of our coding in the java.base module as we used to add private fields and methods to basic classes such as java.lang.Thread or java.lang.Exception. However, I don't want to have so much of our stuff in java.base and rather think that it should live in its own module. So the question here is if it is possible to call code of other modules from java.base, e.g. via the Service Provider interface? I see that I can define a service in java.base and specify some "uses" statement in module-info. But will my implementation of such a service from other modules be available to java.base? Also I'm contemplating about this requirement: I have a class which I would need somewhere in java.base but I'd also like to export it in the public API of my own extension module. So, if I create the class in java.base, I'm not allowed to export this class publicly, unqualified, right? But when I have it living in my extension module, then java.base would not see it. What can I do? Probably create some inherited class in my extension module that extends from the java.base impl and export this?? I'm hoping that those are easy questions for you and you can give me some helpful answers. Yes, services is the way to do this. The jdk.net module is one example, there are several others. The other thing to be aware of is the module-info.java.extra files to augment the module declarations during the build, I suspect you'll end up using that. -Alan
Re: Java SE JSR 250 annotations module renamed to java.xml.ws.annotation?
I agree. Remi On February 15, 2017 12:52:53 AM GMT+01:00, mark.reinh...@oracle.com wrote: >2017/2/13 9:17:47 -0800, Guillaume Smet : >> On Mon, Feb 13, 2017 at 6:10 PM, alan.bate...@oracle.com wrote: >>> On 13/02/2017 16:58, Guillaume Smet wrote: On Mon, Feb 13, 2017 at 5:12 PM, alan.bate...@oracle.com wrote: > > I agree that @Generated is awkward but I haven't suggested >removing it. What do you suggest then? As far as I understood you, you were >suggesting removing the module in Java 10 so the @Generated annotation would >also be gone? Or did I misunderstand? >>> >>> That is the proposal. If it goes ahead then it means that tools that >rely >>> on these annotations in the JDK would need to deploy the standalone >version >>> on the class path or as a module on the module path. >> >> Yeah, so basically, it would end up with >> http://hg.openjdk.java.net/code-tools/jmh/rev/d74b2861222c . >> >> I don't think it's the best possible outcome for this useful >annotation. > >I agree. > >The `@Generated` annotation falls outside the original charter of the >`java.lang.annotation` package, which was meant for annotations that >directly support the language's annotation facility, but we already >added `@Native` in SE 8, so let's add `@Generated` in SE 9 as David >suggests and encourage people to use it when running on this and later >releases. > >The fact that `@Generated` is so widely used is new information to some >of us, so thanks for bringing it up. > >- Mark -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: Extending java.base module
On 15/02/2017 8:03 PM, Langer, Christoph wrote: Hi Chris, Max, thanks for your quick answers. So the service approach seems to fit quite well. But can I assume that my service implementation will be available already at "bootstrap time" of the JDK? E.g. if I need to register/reach my service already at the early stages of JVM initialization, e.g. when a class java.lang.Thread gets initialized, can I assume a service from my extension module would be available? I'm pretty sure the answer to that will be No! Thread is one of the earlier classes to be initialized, the module system is initialized much later. David - Thanks, Christoph -Original Message- From: Chris Hegarty [mailto:chris.hega...@oracle.com] Sent: Mittwoch, 15. Februar 2017 10:04 To: Weijun Wang Cc: Langer, Christoph ; jigsaw- d...@openjdk.java.net Subject: Re: Extending java.base module On 15 Feb 2017, at 08:51, Weijun Wang wrote: Disclaimer: I am not a jigsaw expert. The provides/uses mechanism is certainly more formal, but you can also do http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d282c1a8d20b. This is, at best, a hack. The use of Services is a better approach, where possible. -Chris.
Re: Extending java.base module
On 15/02/2017 12:10, David Holmes wrote: On 15/02/2017 8:03 PM, Langer, Christoph wrote: Hi Chris, Max, thanks for your quick answers. So the service approach seems to fit quite well. But can I assume that my service implementation will be available already at "bootstrap time" of the JDK? E.g. if I need to register/reach my service already at the early stages of JVM initialization, e.g. when a class java.lang.Thread gets initialized, can I assume a service from my extension module would be available? I'm pretty sure the answer to that will be No! Thread is one of the earlier classes to be initialized, the module system is initialized much later. That's right as only classes in java.base can be loaded during startup. I don't know what services that Christoph is thinking of but hopefully they can be deferred until the VM is initialized. -Alan
Re: Extending java.base module
> > E.g. if I need to register/reach my service already at the early stages of > JVM initialization, e.g. when a class java.lang.Thread gets initialized, > can I assume a service from my extension module would be available? > No. At that time only java.base classes can be loaded. If you look at the comments in the initPhase# methods in System, it gives some good info about when things are initialized. http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/tip/src/java.base/share/classes/java/lang/System.java#l1850 /Michael
RE: Extending java.base module
Hi, thanks all for your very helpful comments. So I see that I cannot rely on my Service implementation at an early stage. But I can imagine that I query for the service provider and if it's not there I have to be able to live without it. The question that remains is, when I query the ServiceLoader for my impl at a later time, then I would find it registered, right? I assume that if my module specifies "provides with ", the provider will automatically be registered once the module is loaded. Can you confirm that? Thanks Christoph > -Original Message- > From: Alan Bateman [mailto:alan.bate...@oracle.com] > Sent: Mittwoch, 15. Februar 2017 13:22 > To: David Holmes ; Langer, Christoph > ; Chris Hegarty ; > Weijun Wang > Cc: jigsaw-dev@openjdk.java.net > Subject: Re: Extending java.base module > > > > On 15/02/2017 12:10, David Holmes wrote: > > On 15/02/2017 8:03 PM, Langer, Christoph wrote: > >> Hi Chris, Max, > >> > >> thanks for your quick answers. So the service approach seems to fit > >> quite well. > >> > >> But can I assume that my service implementation will be available > >> already at "bootstrap time" of the JDK? E.g. if I need to > >> register/reach my service already at the early stages of JVM > >> initialization, e.g. when a class java.lang.Thread gets initialized, > >> can I assume a service from my extension module would be available? > > > > I'm pretty sure the answer to that will be No! Thread is one of the > > earlier classes to be initialized, the module system is initialized > > much later. > That's right as only classes in java.base can be loaded during startup. > I don't know what services that Christoph is thinking of but hopefully > they can be deferred until the VM is initialized. > > -Alan
RFR: 8175010: ImageReader is not thread-safe
Hi, a few intermittent but rare test failures[1] that has appeared since the latest jake integration, and since one of the changes in there was to make initialization of the system ImageReader lazy there appears to be cases where ImageReaders are not safely published: - Ensure ImageReader::open is called only once per Path in ImageReaderFactory by using CHM.computeIfAbsent - Ensure ImageReader.reader is safely published to a final field and signal close using a volatile boolean instead webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/ bug: https://bugs.openjdk.java.net/browse/JDK-8175010 Testing shows no issues (which admittedly doesn't mean we're actually solving the root cause for JDK-8174817), and performance numbers from adding a volatile read indicate that any overhead is lost in the noise on ImageReader-heavy workloads. Thanks! /Claes [1] https://bugs.openjdk.java.net/browse/JDK-8174817
Re: Extending java.base module
On 15/02/2017 13:21, Langer, Christoph wrote: Hi, thanks all for your very helpful comments. So I see that I cannot rely on my Service implementation at an early stage. But I can imagine that I query for the service provider and if it's not there I have to be able to live without it. The question that remains is, when I query the ServiceLoader for my impl at a later time, then I would find it registered, right? I assume that if my module specifies "provides with ", the provider will automatically be registered once the module is loaded. Can you confirm that? Yes. If you look in areas like JMX or networking then you'll see lots of examples where "extensions" are deployed as service providers. These should help you get started. -Alan
Re: RFR: 8175010: ImageReader is not thread-safe
+1 > On Feb 15, 2017, at 9:22 AM, Claes Redestad wrote: > > Hi, > > a few intermittent but rare test failures[1] that has appeared > since the latest jake integration, and since one of the changes > in there was to make initialization of the system ImageReader > lazy there appears to be cases where ImageReaders are not > safely published: > > - Ensure ImageReader::open is called only once per Path in > ImageReaderFactory by using CHM.computeIfAbsent > - Ensure ImageReader.reader is safely published to a > final field and signal close using a volatile boolean instead > > webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/ > bug: https://bugs.openjdk.java.net/browse/JDK-8175010 > > Testing shows no issues (which admittedly doesn't mean we're > actually solving the root cause for JDK-8174817), and performance > numbers from adding a volatile read indicate that any overhead > is lost in the noise on ImageReader-heavy workloads. > > Thanks! > > /Claes > > [1] https://bugs.openjdk.java.net/browse/JDK-8174817
Re: RFR: 8175010: ImageReader is not thread-safe
> On 15 Feb 2017, at 13:22, Claes Redestad wrote: > > Hi, > > a few intermittent but rare test failures[1] that has appeared > since the latest jake integration, and since one of the changes > in there was to make initialization of the system ImageReader > lazy there appears to be cases where ImageReaders are not > safely published: > > - Ensure ImageReader::open is called only once per Path in > ImageReaderFactory by using CHM.computeIfAbsent > - Ensure ImageReader.reader is safely published to a > final field and signal close using a volatile boolean instead > > webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/ Looks good Claes. -Chris. > bug: https://bugs.openjdk.java.net/browse/JDK-8175010 > > Testing shows no issues (which admittedly doesn't mean we're > actually solving the root cause for JDK-8174817), and performance > numbers from adding a volatile read indicate that any overhead > is lost in the noise on ImageReader-heavy workloads. > > Thanks! > > /Claes > > [1] https://bugs.openjdk.java.net/browse/JDK-8174817
Re: Java SE JSR 250 annotations module renamed to java.xml.ws.annotation?
On 02/14/2017 05:52 PM, mark.reinh...@oracle.com wrote: 2017/2/13 9:17:47 -0800, Guillaume Smet : On Mon, Feb 13, 2017 at 6:10 PM, alan.bate...@oracle.com wrote: On 13/02/2017 16:58, Guillaume Smet wrote: On Mon, Feb 13, 2017 at 5:12 PM, alan.bate...@oracle.com wrote: I agree that @Generated is awkward but I haven't suggested removing it. What do you suggest then? As far as I understood you, you were suggesting removing the module in Java 10 so the @Generated annotation would also be gone? Or did I misunderstand? That is the proposal. If it goes ahead then it means that tools that rely on these annotations in the JDK would need to deploy the standalone version on the class path or as a module on the module path. Yeah, so basically, it would end up with http://hg.openjdk.java.net/code-tools/jmh/rev/d74b2861222c . I don't think it's the best possible outcome for this useful annotation. I agree. The `@Generated` annotation falls outside the original charter of the `java.lang.annotation` package, which was meant for annotations that directly support the language's annotation facility, but we already added `@Native` in SE 8, so let's add `@Generated` in SE 9 as David suggests and encourage people to use it when running on this and later releases. The fact that `@Generated` is so widely used is new information to some of us, so thanks for bringing it up. I'm glad to hear it... thanks! -- - DML
Re: RFR: 8175010: ImageReader is not thread-safe
On 15/02/2017 13:22, Claes Redestad wrote: Hi, a few intermittent but rare test failures[1] that has appeared since the latest jake integration, and since one of the changes in there was to make initialization of the system ImageReader lazy there appears to be cases where ImageReaders are not safely published: - Ensure ImageReader::open is called only once per Path in ImageReaderFactory by using CHM.computeIfAbsent - Ensure ImageReader.reader is safely published to a final field and signal close using a volatile boolean instead webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/ bug: https://bugs.openjdk.java.net/browse/JDK-8175010 ImageReaderFactory looks good. The changes to ImageReader are okay too, always a bit odd that this code throw NPE when the reader was closed. There is still an issue with async close of course in that someone could close at the same time as an access. However that is a high-level issue for jrtfs, at run-time then the image file is opened once and is never closed. -Alan
Re: RFR: 8175010: ImageReader is not thread-safe
Jim, Chris, Alan, thanks for reviewing! On 02/15/2017 02:56 PM, Alan Bateman wrote: On 15/02/2017 13:22, Claes Redestad wrote: Hi, a few intermittent but rare test failures[1] that has appeared since the latest jake integration, and since one of the changes in there was to make initialization of the system ImageReader lazy there appears to be cases where ImageReaders are not safely published: - Ensure ImageReader::open is called only once per Path in ImageReaderFactory by using CHM.computeIfAbsent - Ensure ImageReader.reader is safely published to a final field and signal close using a volatile boolean instead webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/ bug: https://bugs.openjdk.java.net/browse/JDK-8175010 ImageReaderFactory looks good. The changes to ImageReader are okay too, always a bit odd that this code throw NPE when the reader was closed. There is still an issue with async close of course in that someone could close at the same time as an access. However that is a high-level issue for jrtfs, at run-time then the image file is opened once and is never closed. Yes, this patch doesn't attempt to improve on how races are handled when closing, but should help to avoid potential publication issues when opening. It seems it could be worthwhile to have a special implementation class for the system module, since we could make it perfectly shareable and non-closeable, thus avoid delegation and closed-checking overheads, but that'd be a larger endeavor, perhaps suitable as an RFE for 10. /Claes -Alan
Re: Extending java.base module
Hi Max, I'm not an jigsaw either, but wouldn't your solution break a tool like jlink? In other words, if an application uses your code and the developer uses jlink to create a run-time image, wouldn't that image fail to execute his application because jlink fails to see that java.base depends on java.security.jgss in that special case? Thanks, Volker On Wed, Feb 15, 2017 at 9:51 AM, Weijun Wang wrote: > Disclaimer: I am not a jigsaw expert. > > The provides/uses mechanism is certainly more formal, but you can also do > http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d282c1a8d20b. > > --Max > > > On 02/15/2017 04:36 PM, Langer, Christoph wrote: >> >> Hi Jigsaw experts, >> >> as you might or might not know, we have an own JDK implementation with >> some extension code that is quite interwoven with the jdk. >> >> Now I'm looking into how this coding can be spread into a good module >> structure for jdk9. And I'm not a crack yet on using the module system >> though I've read quite a bit into the spec documentation available so far;-) >> >> The first point for me is that we have to place some of our coding in the >> java.base module as we used to add private fields and methods to basic >> classes such as java.lang.Thread or java.lang.Exception. However, I don't >> want to have so much of our stuff in java.base and rather think that it >> should live in its own module. So the question here is if it is possible to >> call code of other modules from java.base, e.g. via the Service Provider >> interface? I see that I can define a service in java.base and specify some >> "uses" statement in module-info. But will my implementation of such a >> service from other modules be available to java.base? >> >> Also I'm contemplating about this requirement: I have a class which I >> would need somewhere in java.base but I'd also like to export it in the >> public API of my own extension module. So, if I create the class in >> java.base, I'm not allowed to export this class publicly, unqualified, >> right? But when I have it living in my extension module, then java.base >> would not see it. What can I do? Probably create some inherited class in my >> extension module that extends from the java.base impl and export this?? >> >> I'm hoping that those are easy questions for you and you can give me some >> helpful answers. >> >> Thanks a lot in advance!! >> >> Best regards >> Christoph >> >
Re: Java SE JSR 250 annotations module renamed to java.xml.ws.annotation?
Hi Mark, On Wed, Feb 15, 2017 at 12:52 AM, wrote: > The `@Generated` annotation falls outside the original charter of the > `java.lang.annotation` package, which was meant for annotations that > directly support the language's annotation facility, but we already > added `@Native` in SE 8, so let's add `@Generated` in SE 9 as David > suggests and encourage people to use it when running on this and later > releases. > Thanks for taking it into consideration. -- Guillaume
Re: Extending java.base module
Hi Volker, On 15/02/17 15:52, Volker Simonis wrote: Hi Max, I'm not an jigsaw either, but wouldn't your solution break a tool like jlink? In other words, if an application uses your code and the developer uses jlink to create a run-time image, wouldn't that image fail to execute his application because jlink fails to see that java.base depends on java.security.jgss in that special case? In that specific case it's not java.base that depends on java.security.jgss, but the application itself. So I would expect the application code to either require java.security.jgss, or some higher level module for that itself requires java.security.jgss, or jlink to be run with command line options that explicitly add java.security.jgss to the image. best regards, -- daniel Thanks, Volker
Re: RFR: 8175010: ImageReader is not thread-safe
> On Feb 15, 2017, at 5:22 AM, Claes Redestad wrote: > > Hi, > > a few intermittent but rare test failures[1] that has appeared > since the latest jake integration, and since one of the changes > in there was to make initialization of the system ImageReader > lazy there appears to be cases where ImageReaders are not > safely published: > > - Ensure ImageReader::open is called only once per Path in > ImageReaderFactory by using CHM.computeIfAbsent > - Ensure ImageReader.reader is safely published to a > final field and signal close using a volatile boolean instead > > webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/ Looks good. Mandy
Re: Extending java.base module
On 15/02/2017 16:01, Daniel Fuchs wrote: In that specific case it's not java.base that depends on java.security.jgss, but the application itself. So I would expect the application code to either require java.security.jgss, or some higher level module for that itself requires java.security.jgss, or jlink to be run with command line options that explicitly add java.security.jgss to the image. java.security.jgss exports an API so it will be resolved by default when the initial class is loaded from the class path. In addition, it provides a SecurityProvider implementation and so will be resolved because java.base `uses java.security.Provider`. For the jlink case then you are right, it needs someone to know that the application might need to do SPNEGO authentication. In any case, it's an example of how not to do things, and hopefully it will be cleaned up at some point. -Alan
Re: Extending java.base module
On Wed, Feb 15, 2017 at 5:16 PM, Alan Bateman wrote: > On 15/02/2017 16:01, Daniel Fuchs wrote: > >> >> In that specific case it's not java.base that depends >> on java.security.jgss, but the application itself. >> >> So I would expect the application code to either require >> java.security.jgss, or some higher level module for that >> itself requires java.security.jgss, or jlink to be run with >> command line options that explicitly add java.security.jgss >> to the image. > > java.security.jgss exports an API so it will be resolved by default when the > initial class is loaded from the class path. In addition, it provides a > SecurityProvider implementation and so will be resolved because java.base > `uses java.security.Provider`. For the jlink case then you are right, it > needs someone to know that the application might need to do SPNEGO > authentication. > > In any case, it's an example of how not to do things, and hopefully it will > be cleaned up at some point. > Daniel, Alan, thanks for the clarification. I didn't wanted to blame anybody - just looking for good arguments to prevent such code in our version of the JDK :) > -Alan > >
RFR: 8175026: Capture build-time parameters to --generate-jli-classes
Hi, currently the file we generate at build time as input to --generate-jli-classes is lost when linking custom images, which means user generate images may perform worse in certain ways, mostly generating more classes during startup. Additionally, there's a strong assumption in --generate-jli-classes that the VM running jlink is going to generate compatible classes with the image being linked, which we can only really guarantee if the java.base in the linked image is of the same version as the java.base in the VM running jlink. This patch tightens these checks to ensure we have freedom to evolve and re-evaluate the implementation in future releases. JDK: http://cr.openjdk.java.net/~redestad/8175026/jdk.01/ Top: http://cr.openjdk.java.net/~redestad/8175026/top.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8175026 Thanks! /Claes
Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges
Hi Mandy, Some early comments: GenGraphs.java -- 58 dir = Paths.get(args[++i]); may produced ArrayOutOfBoundsException - should we have better error reporting? Or should it check && i < args.length - 1 so that it falls back to having dir == null below? 93 .resolve(ModuleFinder.ofSystem(), could that be: .resolve(finder, Graph.java -- 119 return builder.build().reduce(); 277 this.nodes.addAll(nodes); These were bugs, which you're taking this opportunity to fix - right? JdepsTask.java: --- 1027 // print module descriptor Is this comment accurate? DotFileTest.java Missing @bug tag? best regards, -- daniel On 15/02/17 00:28, Mandy Chung wrote: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.00/ This is the first step to enable generating dot graph to be included in module summary javadoc, if desired. jdeps already supports generating the dot graph for modules. This patch converts GenGraphs build tool to use jdeps implementation as well as fixes jdeps to work with -apionly to generate a dot graph containing `requires transitive` edges only. Mandy [1] https://bugs.openjdk.java.net/browse/JDK-8173303
Re: RFR: 8175026: Capture build-time parameters to --generate-jli-classes
> On Feb 15, 2017, at 9:12 AM, Claes Redestad wrote: > > Hi, > > currently the file we generate at build time as input to > --generate-jli-classes is lost when linking custom images, which means > user generate images may perform worse in certain ways, mostly > generating more classes during startup. > > Additionally, there's a strong assumption in --generate-jli-classes that > the VM running jlink is going to generate compatible classes with the > image being linked, which we can only really guarantee if the java.base > in the linked image is of the same version as the java.base in the VM > running jlink. This patch tightens these checks to ensure we have > freedom to evolve and re-evaluate the implementation in future > releases. > > JDK: http://cr.openjdk.java.net/~redestad/8175026/jdk.01/ > Top: http://cr.openjdk.java.net/~redestad/8175026/top.01/ This restriction sounds reasonable and we can enhance this in a future release. I think the plugin can record the configuration in its own format to be independent of the trace output format. Instead of throwing ISA, you can have a test method to return a boolean to indicate if the default trace file should be read. Instead of running java.base from the runtime, you can use Runtime.version() instead and I think comparing Version::major should be adequate. This change disables this default setting entirely if the image being created is not the same version as this plugin (defaultSpecies and defaultInvokers, etc). Is it intended? In addition, if the main argument is specified but the version does not match, it will ignore the given argument. Should it be an error instead? We are the one who will generate a trace file and specify it in the jlink plugin option. It’s okay to ignore the default trace output if no plugin option is specified and I think no warning should be printed in this case. It’s just like this plugin is disabled. You may want to add a suboption to turn on verbose that will trace what is generated and what is ignored. Mandy
Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges
> On Feb 15, 2017, at 10:29 AM, Daniel Fuchs wrote: > > Hi Mandy, > > Some early comments: > > GenGraphs.java > -- > > 58 dir = Paths.get(args[++i]); > > may produced ArrayOutOfBoundsException - should we have better > error reporting? > Or should it check && i < args.length - 1 so that it falls back > to having dir == null below? > Good catch. Fixed to: i++; dir = i < args.length ? Paths.get(args[i]) : null; > 93 .resolve(ModuleFinder.ofSystem(), > > could that be: .resolve(finder, > Fixed. > > Graph.java > -- > > 119 return builder.build().reduce(); > 277 this.nodes.addAll(nodes); > > > These were bugs, which you're taking this opportunity to fix - right? > Yes. 119 is caught by this change. 277 is caught by code inspection. > > JdepsTask.java: > --- > > 1027 // print module descriptor > > Is this comment accurate? > I updated the comment: // generate dot graph from the resolved graph from module // resolution. No class dependency analysis is performed. > DotFileTest.java > > > Missing @bug tag? > Fixed. Updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/ Mandy
Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges
Hi Mandy, > Updated webrev: > http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/ Looks good. I haven't reviewed the build changes. I assume they're OK if you managed to build ;-) best regards, -- daniel On 15/02/17 19:32, Mandy Chung wrote: On Feb 15, 2017, at 10:29 AM, Daniel Fuchs wrote: Hi Mandy, Some early comments: GenGraphs.java -- 58 dir = Paths.get(args[++i]); may produced ArrayOutOfBoundsException - should we have better error reporting? Or should it check && i < args.length - 1 so that it falls back to having dir == null below? Good catch. Fixed to: i++; dir = i < args.length ? Paths.get(args[i]) : null; 93 .resolve(ModuleFinder.ofSystem(), could that be: .resolve(finder, Fixed. Graph.java -- 119 return builder.build().reduce(); 277 this.nodes.addAll(nodes); These were bugs, which you're taking this opportunity to fix - right? Yes. 119 is caught by this change. 277 is caught by code inspection. JdepsTask.java: --- 1027 // print module descriptor Is this comment accurate? I updated the comment: // generate dot graph from the resolved graph from module // resolution. No class dependency analysis is performed. DotFileTest.java Missing @bug tag? Fixed. Updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/ Mandy
Re: RFR: 8175010: ImageReader is not thread-safe
Hi Claes, Reading the https://bugs.openjdk.java.net/browse/JDK-8174817 and then the change that was just pushed, I can't seem to figure out what was the problem with original code. I can't find evidence for claims in https://bugs.openjdk.java.net/browse/JDK-8175010 . Is the problem publication of ImageReader via ImageReaderFactory? That can't be it, since ImageReaderFactory is using ConcurrentHashMap which ensures happens before relationships. Is there any place else where ImageReader.open(Path) is called and then the instance unsafely published to other threads? The only place I could find is in jdk.internal.jrtfs.SystemImage.open(): static SystemImage open() throws IOException { if (modulesImageExists) { // open a .jimage and build directory structure final ImageReader image = ImageReader.open(moduleImageFile); image.getRootDirectory(); return new SystemImage() { @Override Node findNode(String path) throws IOException { return image.findNode(path); } @Override byte[] getResource(Node node) throws IOException { return image.getResource(node); } @Override void close() throws IOException { image.close(); } }; } ...here the final 'image' local variable is captured by anonymous inner SystemImage subclass into a synthetic final field, so this final field ensures that ImageReader is published safely as a delegate of SystemImage. ImageReader.open(Path) factory method delegates to ImageReader.SharedImageReader.open(Path, ByteOrder) which creates a new instance of ImageReader encapsulating a SharedImageReader object: public static ImageReader open(Path imagePath, ByteOrder byteOrder) throws IOException { Objects.requireNonNull(imagePath); Objects.requireNonNull(byteOrder); synchronized (OPEN_FILES) { SharedImageReader reader = OPEN_FILES.get(imagePath); if (reader == null) { // Will fail with an IOException if wrong byteOrder. reader = new SharedImageReader(imagePath, byteOrder); OPEN_FILES.put(imagePath, reader); } else if (reader.getByteOrder() != byteOrder) { throw new IOException("\"" + reader.getName() + "\" is not an image file"); } ImageReader image = new ImageReader(reader); // <<-- HERE reader.openers.add(image); return image; } } ...the ImageReader returned from this method is not safe to publish via data race, but as far as I can see, there is no such publishing going on. So am I right in that all this patch solves is it eliminates a possibility of NPE when ImageReader is close()-d concurrently with being used from some other thread. If such NPE was observed, it means that close() is being called concurrently with access and there are still races possible which can cause undesired effects. For example: calling ImageReader.close() while using it concurrently may close underlying SharedImageReader and then after closing, access it. So is there a concurrent ImageReader.close() possible? The only place I see ImageReader.close() being invoked is from SystemImage.close() in the anonymous inner class implementation which wraps ImageReader. SystemImage.close() is only being invoked from JrtFileSystem.cleanup(), which is called from JrtFileSystem.close() and JrtFileSystem.finalize(). The following is theoretically possible: FileSystem fs = FileSystems.newFileSystem(URI.create("jrt:/"), ...); Path p = fs.getPath(...); FileSystemProvider provider = fs.provider(); InputStream is = provider.newInputStream(p, ...); // 'fs' and 'p' (which has a reference to 'fs') may be found finalizable and be finalized while the call to obtain content of input stream is in progress For this to be prevented, the following method in JrtFileSystem: // returns the content of the file resource specified by the path byte[] getFileContent(JrtPath path) throws IOException { Node node = checkNode(path); if (node.isDirectory()) { throw new FileSystemException(path + " is a directory"); } //assert node.isResource() : "resource node expected here"; return image.getResource(node); } ...would have to be changed to: byte[] getFileContent(JrtPath path) throws IOException { Node node = checkNode(path); if (node.isDirectory()) { throw new FileSystemException(path + " is a directory"); } //assert node.isResource() : "resource node expected here"; byte[] content = image.getResource(node); Reference.reachabilityFence(this); return
Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges
> On Feb 15, 2017, at 12:27 PM, Daniel Fuchs wrote: > > Hi Mandy, > > > Updated webrev: > > http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/ > > Looks good. I haven't reviewed the build changes. > I assume they're OK if you managed to build ;-) Thanks. The build change is trivial adding —-add-exports for compiling and running GenGraphs build tool. Also add a new target to generate spec-only module graphs. Mandy
Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges
> On Feb 15, 2017, at 1:08 PM, Mandy Chung wrote: >> >>> Updated webrev: >>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/ You may be interested in the module graphs generated from this change. http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/spec-dotfiles/ http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/gengraphs/ Open jdk.graph.html if you want to view all .png files. Mandy
Re: RFR: 8175010: ImageReader is not thread-safe
Hi Peter, are you suggesting that if I have class Foo { Bar b; }, then creating and putting Foo b in a CHM before returning it to a consumer which is then read from another thread is enough to force b to be safely published even when the other thread does *not* get the object via a call to the same CHM (which would go via the same volatile read and add the necessary happens before relationship)? I recalled having seen examples to the effect that a non-volatile b isn't safely published in this case. The (very shaky) hypothesis is thus that this could be what's happening in any of the places which load and locally cache the system ImageReader for anyone to use, e.g., SystemModuleFinder.SystemImage or JavaRuntimeURLConnection (which is implicitly called when there's a security manager installed). I might be (in fact likely am) wrong, but we discussed this offline and came to the conclusion that there was no harm in implementing these improvements regardless of whether they actually resolve 8174817 or not. I think prior to this patch a concurrent ImageReader.close() could happen if there was a race between 3 or more threads to resolve the same Path from ImageReaderFactory.get (especially since there might be a longish time window there since we might block to load a library etc), so I don't think we lose anything from plugging that hole by using computeIfAbsent here. I think your observations about potential issues in JRTFS is correct, but there was nothing to suggest JRTFS code was involved in JDK-8174817 (as it's not code that's used by the BuiltinClassLoader). Thanks! /Claes On 2017-02-15 21:52, Peter Levart wrote: Hi Claes, Reading the https://bugs.openjdk.java.net/browse/JDK-8174817 and then the change that was just pushed, I can't seem to figure out what was the problem with original code. I can't find evidence for claims in https://bugs.openjdk.java.net/browse/JDK-8175010 . Is the problem publication of ImageReader via ImageReaderFactory? That can't be it, since ImageReaderFactory is using ConcurrentHashMap which ensures happens before relationships. Is there any place else where ImageReader.open(Path) is called and then the instance unsafely published to other threads? The only place I could find is in jdk.internal.jrtfs.SystemImage.open(): static SystemImage open() throws IOException { if (modulesImageExists) { // open a .jimage and build directory structure final ImageReader image = ImageReader.open(moduleImageFile); image.getRootDirectory(); return new SystemImage() { @Override Node findNode(String path) throws IOException { return image.findNode(path); } @Override byte[] getResource(Node node) throws IOException { return image.getResource(node); } @Override void close() throws IOException { image.close(); } }; } ...here the final 'image' local variable is captured by anonymous inner SystemImage subclass into a synthetic final field, so this final field ensures that ImageReader is published safely as a delegate of SystemImage. ImageReader.open(Path) factory method delegates to ImageReader.SharedImageReader.open(Path, ByteOrder) which creates a new instance of ImageReader encapsulating a SharedImageReader object: public static ImageReader open(Path imagePath, ByteOrder byteOrder) throws IOException { Objects.requireNonNull(imagePath); Objects.requireNonNull(byteOrder); synchronized (OPEN_FILES) { SharedImageReader reader = OPEN_FILES.get(imagePath); if (reader == null) { // Will fail with an IOException if wrong byteOrder. reader = new SharedImageReader(imagePath, byteOrder); OPEN_FILES.put(imagePath, reader); } else if (reader.getByteOrder() != byteOrder) { throw new IOException("\"" + reader.getName() + "\" is not an image file"); } ImageReader image = new ImageReader(reader); // <<-- HERE reader.openers.add(image); return image; } } ...the ImageReader returned from this method is not safe to publish via data race, but as far as I can see, there is no such publishing going on. So am I right in that all this patch solves is it eliminates a possibility of NPE when ImageReader is close()-d concurrently with being used from some other thread. If such NPE was observed, it means that close() is being called concurrently with access and there are still races possible which can cause undesired effects. For example: calling ImageReader.close() while using it concurrently may close underlying SharedImageReader and then after closing, access it. So is th
Re: Running jaotc with an external Graal
I don’t know how one patches a module in the middle of the module graph. That is, we use --patch-module and --upgrade-module-path to override the jdk.vm.compiler module in the JDK. I don’t know what that means for modules such as jdk.aot that depend on jdk.vm.compiler. Maybe someone from the AOT or jigsaw team can help. -Doug > On 14 Feb 2017, at 12:26, Andrew Haley wrote: > > Is this possible? Seems that no matter what I do, aotc always prefers to > use the internal version of org.graalvm.compiler which is in HotSpot. > > I don't quite get why this is: I can run an external Graal using JVMCI > with no problems. I saw "8145337: [JVMCI] JVMCI initialization with > SecurityManager installed fails:" which might be related, but perhaps > it's not. > > So, why is it possible to use an external Graal with JVMCI, but > apparently not with jaotc? And is there anything I can do to make > progress? > > Thanks, > > Andrew.
Re: Running jaotc with an external Graal
> On Feb 14, 2017, at 4:38 AM, Andrew Haley wrote: > > On 14/02/17 14:37, Alan Bateman wrote: >> --patch-module can be used to patch any module in the boot layer. So if >> you are looking to override or add classes then --patch-module should work. > > Aha! Thanks, Does it?
Re: Running jaotc with an external Graal
> On Feb 15, 2017, at 1:56 PM, Christian Thalinger > wrote: > > >> On Feb 14, 2017, at 4:38 AM, Andrew Haley wrote: >> >> On 14/02/17 14:37, Alan Bateman wrote: >>> --patch-module can be used to patch any module in the boot layer. So if >>> you are looking to override or add classes then --patch-module should work. >> >> Aha! Thanks, > > Does it? Yes it does except that module-info.class can’t be patched. You will get a warning if the patched path contains module-info.class. Are you seeing the otherwise? Mandy
Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges
On 2017-02-15 22:08, Mandy Chung wrote: On Feb 15, 2017, at 12:27 PM, Daniel Fuchs wrote: Hi Mandy, Updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/ Looks good. I haven't reviewed the build changes. I assume they're OK if you managed to build ;-) Thanks. The build change is trivial adding —-add-exports for compiling and running GenGraphs build tool. Also add a new target to generate spec-only module graphs. +1 /Claes Mandy
Re: Running jaotc with an external Graal
Can i say again that when using --patch-module, module-info.class should be merged instead of having a warning ? Rémi - Mail original - > De: "Mandy Chung" > À: "Christian Thalinger" > Cc: jigsaw-dev@openjdk.java.net, "hotspot compiler" > , graal-...@openjdk.java.net > Envoyé: Mercredi 15 Février 2017 23:44:19 > Objet: Re: Running jaotc with an external Graal >> On Feb 15, 2017, at 1:56 PM, Christian Thalinger >> wrote: >> >> >>> On Feb 14, 2017, at 4:38 AM, Andrew Haley wrote: >>> >>> On 14/02/17 14:37, Alan Bateman wrote: --patch-module can be used to patch any module in the boot layer. So if you are looking to override or add classes then --patch-module should work. >>> >>> Aha! Thanks, >> >> Does it? > > Yes it does except that module-info.class can’t be patched. You will get a > warning if the patched path contains module-info.class. Are you seeing the > otherwise? > > Mandy