RE: RFR (T): 8243117: Cleanups in Java code of module jdk.jlink
Hi Alan,
> >> Looks okay to me too, except @SuppressWarnings("unused") looks
> strange,
> >> is that an Eclipse compiler warning name?
> > Yes, it comes from Eclipse, probably only used by its compiler. The private
> field EXIT_SYSERR doesn't seem to be used. Alternatively, I could comment it
> out? Or remove it?
> >
> The jimage tool went through a few iterations and I think it's just a
> left over and should be okay to remove.
OK, I'll remove it altogether then:
http://cr.openjdk.java.net/~clanger/webrevs/8243117.1/
I'll push it tomorrow, after a final round of testing.
Cheers
Christoph
RE: RFR (T): 8243117: Cleanups in Java code of module jdk.jlink
Hi Alan,
> Looks okay to me too, except @SuppressWarnings("unused") looks strange,
> is that an Eclipse compiler warning name?
Yes, it comes from Eclipse, probably only used by its compiler. The private
field EXIT_SYSERR doesn't seem to be used. Alternatively, I could comment it
out? Or remove it?
/Christoph
RE: RFR (T): 8243117: Cleanups in Java code of module jdk.jlink
Hi Claes, thanks for your quick review. I configured my IDE to go to wildcard at 10 imports from the same package. But if there's a common style guide for OpenJDK, I can certainly reconfigure this. Best regards Christoph > -Original Message- > From: Claes Redestad > Sent: Sonntag, 19. April 2020 18:32 > To: Langer, Christoph ; jigsaw- > d...@openjdk.java.net > Cc: core-libs-...@openjdk.java.net > Subject: Re: RFR (T): 8243117: Cleanups in Java code of module jdk.jlink > > Looks good to me, Christoph, > > JmodTask.java: Not sure what the consensus is about wildcard imports, > but I'm fine with it here. > > /Claes > > On 2020-04-19 17:32, Langer, Christoph wrote: > > Hi, > > > > please help review this cleanup patch for the Java code in module jdk.jlink. > It's mostly automatic IDE cleanups of unused imports plus a few removals for > unused private methods, fields and annotations. I've also added some > @Override annotations where such were missing. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8243117 > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8243117.0/ > > > > Thanks > > Christoph > >
RFR (T): 8243117: Cleanups in Java code of module jdk.jlink
Hi, please help review this cleanup patch for the Java code in module jdk.jlink. It's mostly automatic IDE cleanups of unused imports plus a few removals for unused private methods, fields and annotations. I've also added some @Override annotations where such were missing. Bug: https://bugs.openjdk.java.net/browse/JDK-8243117 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8243117.0/ Thanks Christoph
RE: RFR: 8242039: Improve jlink VersionPropsPlugin
Hi Claes and Mark, thanks again for your inputs. I'll push this with Claes' review then. Best regards Christoph > -Original Message- > From: core-libs-dev On Behalf > Of Claes Redestad > Sent: Mittwoch, 8. April 2020 00:12 > To: core-libs-...@openjdk.java.net > Subject: Re: RFR: 8242039: Improve jlink VersionPropsPlugin > > > > On 2020-04-03 15:36, Langer, Christoph wrote: > > Eventually I came up with this result and then I also asked myself the > question whether the new complexity was worth the benefit. I answered > myself with a yes (though definitely not a clear one ), and that's why I > proposed the change. After all, the new complexity isn't huge... > > I don't mind the cleaned up patch[1]. > > It also gets rid of the constants being replaced, which I assume will > otherwise be loaded and kept on the heap and in the string table > forever. While unlikely to cause confusion, I'd argue that not finding > the value replaced in heap dumps might be of some value. > > /Claes > > [1] http://cr.openjdk.java.net/~clanger/webrevs/8242039.1/
RE: RFR: 8242039: Improve jlink VersionPropsPlugin
Hi Mark, > -Original Message- > From: mark.reinh...@oracle.com > Sent: Donnerstag, 2. April 2020 23:13 > To: Langer, Christoph > Cc: jigsaw-dev@openjdk.java.net; core-libs-...@openjdk.java.net > Subject: Re: RFR: 8242039: Improve jlink VersionPropsPlugin > > 2020/4/2 8:01:28 -0700, christoph.lan...@sap.com: > > please review a small improvement for the jlink > > VersionPropsPlugin. The Plugin modifies the bytecode of > > java/lang/VersionProps.class to replace the initializion of certain > > vendor specific system properties with custom values. This > > modification currently adds two bytecodes per constant, a pop and > > another ldc. I overhauled it to simply replace the original ldc of the > > old value with another ldc, loading the custom value. > > I thought about doing this when I originally wrote that plugin, but it’s > so awkward to achieve with ASM -- as demonstrated by your patch -- that > I concluded it wasn’t worth it. Who will notice an extra pop in a basic > block that’s only ever executed once? Is the complexity of this new > code worth the benefit? Well, first I started playing with this and got a bit obsessed to find optimizations in that area. (I learned quite a bit about java asm.) It would be of higher (micro-)benefit for common VM startup if the fields to be modified could be final but that's even more awkward to do and requires intricate knowledge and assumptions about how VersionProps.java is structured. So I decided against messing with that. Eventually I came up with this result and then I also asked myself the question whether the new complexity was worth the benefit. I answered myself with a yes (though definitely not a clear one ), and that's why I proposed the change. After all, the new complexity isn't huge... So, would that be your terminal veto or could you imagine accepting the change? Best regards Christoph
RE: RFR: 8242039: Improve jlink VersionPropsPlugin
Thanks, Claes. > -Original Message- > From: Claes Redestad > Sent: Donnerstag, 2. April 2020 23:05 > To: Langer, Christoph ; jigsaw- > d...@openjdk.java.net; core-libs-...@openjdk.java.net > Subject: Re: RFR: 8242039: Improve jlink VersionPropsPlugin > > > > On 2020-04-02 20:21, Langer, Christoph wrote: > > OK, I created an edition which doesn't change the formatting so that the > added code becomes more obvious: > > http://cr.openjdk.java.net/~clanger/webrevs/8242039.1/ > > > > Does it look better? > > I think so, yes. > > /Claes
RE: RFR: 8242039: Improve jlink VersionPropsPlugin
Hi Claes,
> fun little micro-optimization. :-)
Yep, I got a bit obsessed here. But thanks for the review.
> My only nit is that I'd have preferred if you kept the indentation
> style, since the patch now shifts code around a bit (which makes it
> harder to review and see the history)
OK, I created an edition which doesn't change the formatting so that the added
code becomes more obvious:
http://cr.openjdk.java.net/~clanger/webrevs/8242039.1/
Does it look better?
Cheers
Christoph
>
> /Claes
>
>
> On 2020-04-02 17:01, Langer, Christoph wrote:
> > Hi,
> >
> > please review a small improvement for the jlink VersionPropsPlugin. The
> Plugin modifies the bytecode of java/lang/VersionProps.class to replace the
> initializion of certain vendor specific system properties with custom values.
> This modification currently adds two bytecodes per constant, a pop and
> another ldc. I overhauled it to simply replace the original ldc of the old
> value
> with another ldc, loading the custom value.
> >
> > I was playing a bit with the plugin and tried to familiarize with the code
> > – so
> that’s the outcome
> >
> > I also added an @SuppressWarnings("unused") in
> VersionProps.java.template to quiesce an IDE warning.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8242039
> > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8242039.0/
> >
> > Thanks
> > Christoph
> >
RFR: 8242039: Improve jlink VersionPropsPlugin
Hi,
please review a small improvement for the jlink VersionPropsPlugin. The Plugin
modifies the bytecode of java/lang/VersionProps.class to replace the
initializion of certain vendor specific system properties with custom values.
This modification currently adds two bytecodes per constant, a pop and another
ldc. I overhauled it to simply replace the original ldc of the old value with
another ldc, loading the custom value.
I was playing a bit with the plugin and tried to familiarize with the code – so
that’s the outcome
I also added an @SuppressWarnings("unused") in VersionProps.java.template to
quiesce an IDE warning.
Bug: https://bugs.openjdk.java.net/browse/JDK-8242039
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8242039.0/
Thanks
Christoph
RE: RFR (XS): 8220409: [TESTBUG] jdk jtreg test jdk/modules/scenarios/overlappingpackages/OverlappingPackagesTest.java - testOverlapWithBaseModule tests the wrong thing
Thanks for the review, Alan. I pushed it. > -Original Message- > From: Alan Bateman > Sent: Montag, 11. März 2019 10:04 > To: Langer, Christoph ; Java Core Libs d...@openjdk.java.net>; jigsaw-dev > Subject: Re: RFR (XS): 8220409: [TESTBUG] jdk jtreg test > jdk/modules/scenarios/overlappingpackages/OverlappingPackagesTest.java > - testOverlapWithBaseModule tests the wrong thing > > On 11/03/2019 08:50, Langer, Christoph wrote: > > Hi, > > > > please review this small test fix. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8220409 > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8220409.0/ > > > > The test uses a wrong option "-add-modules" instead of "--add-modules" > in one place. > > > The change to testOverlapWithBaseModule looks okay, the changes to the > imports are not needed but okay too. > > -Alan >
RFR (XS): 8220409: [TESTBUG] jdk jtreg test jdk/modules/scenarios/overlappingpackages/OverlappingPackagesTest.java - testOverlapWithBaseModule tests the wrong thing
Hi, please review this small test fix. Bug: https://bugs.openjdk.java.net/browse/JDK-8220409 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8220409.0/ The test uses a wrong option "-add-modules" instead of "--add-modules" in one place. Thanks Christoph
RE: Use classes in unnamed module that are also contained in a JDK platform/runtime module
Hi Alex, > On 7/6/2017 11:37 PM, Langer, Christoph wrote: > >> On 7/4/2017 12:02 AM, Langer, Christoph wrote: > >>> I have some piece of software that we ship as a jar file and > >>> which will hence run on a JDK 9 in the unnamed module. However, > >>> this jar file contains a package that is also contained in our > >>> JDK image in a module that is always part of the runtime. > >> > >> It would be remiss of me if I didn't ask for some details about why > >> this scenario has arisen. From "always part of the runtime", I > >> would have guessed the package is in java.base, but you also > >> mention "our JDK image" so perhaps the package is in a module that > >> SAP always jlinks in? > > > > Thanks for your interest in the details. Here it is: We have a > > separate module that exposes an augmenting SAP specific API which is > > publicly exported. This module is part of our JDK image. > > The default set of root modules for the unnamed module is specified by > JEP 261 rather than a JSR, but I guess the SAP JDK implementation uses > the same default set as the OpenJDK implementation. Then, since your > separate module exports a package without qualification, the separate > module is in the default set. Exactly, this is how it is designed. > You could mark the separate module as DoNotResolveByDefault so that it > is observable *but not readable* by the unnamed module which holds your > JAR file's tool classes + copies of SAP-specific API/impl classes. > > Or, given that the JAR file is meant to be cross-JDK and so should never > be aware of the separate module, your tool's launch script could make > the separate module unobservable (--limit-modules). OK, this maybe is a good idea to try. When I'm running my JDK-independent tool on top of our VM, then this public API is not used and probably neither of its dependents. I'll test that. Thanks a lot for your consulting. Best regards Christoph
RE: Use classes in unnamed module that are also contained in a JDK platform/runtime module
Hi Alex, > On 7/4/2017 12:02 AM, Langer, Christoph wrote: > > I have some piece of software that we ship as a jar file and which > > will hence run on a JDK 9 in the unnamed module. However, this jar > > file contains a package that is also contained in our JDK image in a > > module that is always part of the runtime. > > It would be remiss of me if I didn't ask for some details about why this > scenario has arisen. From "always part of the runtime", I would have > guessed the package is in java.base, but you also mention "our JDK > image" so perhaps the package is in a module that SAP always jlinks in? Thanks for your interest in the details. Here it is: We have a separate module that exposes an augmenting SAP specific API which is publicly exported. This module is part of our JDK image. And it in turn pulls a few other modules with the implementing classes and packages. And then we ship a few tools and services apart from the JDK. Some of these are Eclipse based, some of these are standalone. And parts of these tools use the basic classes which are also observable in the JDK. But they need to be there as our tooling should run on any JDK, not only ours. I think Eclipse-wise we are good, at least when Eclipse runs on Java 8. With the Eclipse classloading, we get the classes out of the plugins. However, since Eclipse is not really mature yet for Java 9, I didn't test there so far. And for the standalone apps, I probably need to look into implementing some custom classloader. I'll try with Alan's suggestions (thanks Alan) but didn't find the time yet. Thanks & Best regards Christoph
Module in JDK image as platform module or loaded by application class loader?
Hi there, for our JDK 9 image the jtreg test jdk/modules/etc/VerifyModuleDelegation has unveiled a problem. The issue is that we introduced a few modules that are defined as a platform module. One of these modules requires jdk.attach which is neither boot nor platform. The code within the module that requires jdk.attach seems to work since the platform class loader is able to load jdk.attach classes because it delegates to the application class loader. However, VerifyModuleDelegation is not satisfied with the fact that the loader of our custom module isn't the loader of jdk.attach or one of its ancestors. Now, for me the question is how to solve this correctly? I currently see 3 options: 1. move jdk.attach (and hence jdk.internal.jvmstat ) to the platform modules 2. remove our custom modules from the list of platform modules in order to get them under the application class loader 3. leave the class loaders of the modules alone, remove the requires statement from module-info and implement a Service (Loader) that can be implemented in our jdk.attach version To get on the right way here I probably understand too little of the implications of a module being "Platform". I understand that platform modules don't have all permissions by default such as modules loaded by the boot loader. However, there are some differences and there were reasons why we elevated our modules to "platform", I think it had to do something with caller sensitive calls or other authentication checking mechanisms. So, probably solution 1.) would be the easiest but I don't know if it is a good idea and we lose some compatibility with OpenJDK. 2.) I have to check again where the problems were and 3.) Seems a bit complicated and would unnecessarily break the coupling of modules via module-info. So, I'm hoping to get some advice here :) Thanks in advance and best regards Christoph
Use classes in unnamed module that are also contained in a JDK platform/runtime module
Hi experts, probably this was already asked or discussed here but I don't find an exact answer for my type of issue, so I'm asking again. I have some piece of software that we ship as a jar file and which will hence run on a JDK 9 in the unnamed module. However, this jar file contains a package that is also contained in our JDK image in a module that is always part of the runtime. So, when my app wants to use one of these classes, I get a java.lang.IllegalAccessError because the class is not exported from the runtime module to my app. And, also if I have a class in that package which is not part of the runtime module, I get a java.lang.NoClassDefFoundError because probably the package is loaded from the runtime module. As I don't want to maintain 2 copies of the same code just to have different package names, I'm wondering if I could solve this somehow by implementing my own class loader for the app that would first try to load these classes from the classpath before delegating to the default application class loader. Would that work? I also did some googling on implementing a non-delegating class loader but to me that seems not so easy and has pitfalls. Is there a good reference or even something in the JDK that I could subclass? Thanks Christoph
SecurityManager.checkPackageAccess for qualified exports
Hi all,
while playing with the security manager (using -Djava.security.manager) in Java
9 and testing platform modules that we have added specifically in our build, I
came across the following thing:
As we are using some stuff from jdk.internal, I get the AccessControlException:
"exception access denied ("java.lang.RuntimePermission"
"accessClassInPackage.jdk.internal.misc")" in several places, even if my code
runs priviledged. I figured that I need to grant permission "permission
java.lang.RuntimePermission "accessClassInPackage.jdk.internal.misc"" to my
module. I was looking around where this restriction comes from and learned the
following in the documentation of SecurityManager.checkPackageAccess:
Implementation Note:
This implementation also restricts all non-exported packages of modules loaded
by the platform class
loader
or its ancestors. A "non-exported package" refers to a package that is not
exported to all modules. Specifically, it refers to a package that either is
not exported at all by its containing module or is exported in a qualified
fashion by its containing module.
Reading this, I'm wondering whether the implementation should implicitly grant
package access for modules that a package in question was exported to in a
qualified fashion? Now one ends up having to additionally add specific
permissions which can easily be forgot.
Any comments? Shouldn't that be improved?
Best regards
Christoph
RE: Another module system question
Thanks, Alan. That sounds promising. I'll try it. > -Original Message- > From: Alan Bateman [mailto:alan.bate...@oracle.com] > Sent: Montag, 20. Februar 2017 20:49 > To: Langer, Christoph <christoph.lan...@sap.com>; jigsaw- > d...@openjdk.java.net > Subject: Re: Another module system question > > On 20/02/2017 19:15, Langer, Christoph wrote: > > > : > > > > However, the main issue I have with this approach is: When is the time that > my service from the second module 'b' will be available and registered? I > assume, module 'a' will load first, then 'b'. So, when I use some static class > initialization as the place in 'a' where I try to load the service from 'b', > I guess > the service couldn't be available yet. Is there some module system > initialization hook, where I could plug in and then assume the service from > 'b' > is ready? Or should I maybe add a static class initializer in 'a' which would > try > to resolve the implementing service from 'b' in a loop until it is ready? > > > Declaring the `uses` and `provides` and deploy the modules on the module > path should be enough. When the consumer module (the module with > `uses`) > is resolved then it will trigger any providers to be resolved. The > resolved modules are loaded eagerly (meaning they are defined to the VM) > but there is nothing actually loaded. When you invoke ServiceLoader to > get a stream or to iterate over the providers then it will load the > provider from `b`. It's probably best to just try it out as it's easy to > get something working. Often the hard bit is deciding on the service > interface and choosing the methods to expose to allow a consumer select. > > -Alan
Another module system question
Hi, here is another question which generates out of my attempts to refactor our code into modules. My design idea currently is to have a module for the API and another module which implements it. The API has a package where service interfaces are defined and the implementation will implement these service interfaces. Let's say it looks like this: a) module my.api exports the.api.package.services b) module my.impl requires my.api provides the.api.package.services.Service1 with the.impl.package.providers.Service1Impl This seems obvious to me as the impl needs to know the service interfaces. Anyway, maybe you could comment on this idea if this sounds like a reasonable pattern. However, the main issue I have with this approach is: When is the time that my service from the second module 'b' will be available and registered? I assume, module 'a' will load first, then 'b'. So, when I use some static class initialization as the place in 'a' where I try to load the service from 'b', I guess the service couldn't be available yet. Is there some module system initialization hook, where I could plug in and then assume the service from 'b' is ready? Or should I maybe add a static class initializer in 'a' which would try to resolve the implementing service from 'b' in a loop until it is ready? Looking forward to some suggestions here. Thanks and best regards Christoph
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 <weijun.w...@oracle.com> > Cc: Langer, Christoph <christoph.lan...@sap.com>; jigsaw- > d...@openjdk.java.net > Subject: Re: Extending java.base module > > > > On 15 Feb 2017, at 08:51, Weijun Wang <weijun.w...@oracle.com> 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.
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
