Hi, thanks for your time. Comments inline.
----- Original Message ----- > From: "Alan Bateman" <alan.bate...@oracle.com> > To: "Matej Novotny" <manov...@redhat.com> > Cc: jigsaw-dev@openjdk.java.net, "Martin Kouba" <mko...@redhat.com>, "Tomas > Remes" <tre...@redhat.com> > Sent: Thursday, April 20, 2017 10:52:04 AM > Subject: Re: setAccessible() alternative with Jigsaw - feedback on Lookup > > On 20/04/2017 07:43, Matej Novotny wrote: > > > So I did some hacking and tried to make Weld use MethodHandles.Lookup and > > here is a bit of a feedback for you. > > But first of all, thanks for your quick advice on how to approach this. > Thank you for writing down your experiences. > > > > > > > So, how did Lookup work for us? > > > > 1) privateLookupIn + drop private mode > > This was the way to go, as the bean classes can be anywhere (in classes not > > openly accessible to us, especially if we consider modules). > > BTW I am not sure about the purpose of the private mode as you always need > > to drop it to be able to use lookup. > This should work fine for existing bean classes as presumably they are > in the class path or at least not in modules that are strongly > encapsulated. In the future then if the bean classes are compiled and > deployed as modules then the owner of the module will have to open the > packages to at least the library that wants to reflect deeply. > > The reason you have to drop the PRIVATE mode is because it is expected > to be used in the future. JDK-8171335 [1] and JEP 181 [2] provide some > context as how this could evolve. Thanks for explanation, it makes some sense now. > > > > > 2) Lookup approach carries along the need to pass the reference to the base > > lookup class at all times. > > This is kind of weird because in some (not-so-rare) cases, we need to > > create "artificial packages" in which we place proxy classes. For instance > > when we create a proxy for Interger, Map, Principal,... > > Ofc we cannot place in into java.* packages, so we create our own. For this > > to work with Lookup, we need to have that package created ahead of time > > and have a reference to some "dummy" useless class inside to be able to do > > the lookup. > > Or is there a way to define a whole new (open by default) package where we > > could place it using Lookup? Having the "dummy" package/class just to use > > Lookup is lame and very error prone (I can think of several ways to break > > the proxy creation even now). > What is the defining loader of the proxy class that you spin for types > such as java.lang.Integer? If it's your class loader implementation then > you can invoke its defineClass method to define the class, your don't > need a Lookup to an existing class for that scenario. Not really sure, but it is certainly not one under our control. I suppose these "basic" classes will be loaded by, say, Wildfly as soon as you bootstrap it, before any CDI-enabled deployment takes place. > > If you are looking to define a class in a new runtime package and the > class loader that is not your implementation then you are out of luck. > Lookup is designed for doing computations in the context of an existing > class. There is a note in the archives here from John Rose about > extending the lookup API for cases like this but it would come with a > lot of complexity. Also there isn't any notion of adding a package to a > named module at this time. When java.lang.reflect.Proxy spins proxy > classes then it doesn't add packages to named modules, it instead spins > the proxy classes into its own module (something you could do too, > assuming the classes that you are proxying are accessible). I think it's more or less what we did/are doing - we spin the proxies either into the same package the came from or to our own package/module. But in order to avoid proxy name clash in the same package, we just took the original package and replaced the prefix 'java' with something like 'org.jboss.weld.proxies'. This effectively means that 'java.util.Map' proxy will end up as something like 'org.jboss.weld.proxies.util.Map$Proxy'. Obviously, 'org.jboss.weld.proxies.util' did not exist beforehand and putting everything into one existing package using Lookup is bound to blow up very quickly. So there is basically no way to achieve this with JDK 9? > > > > > 3) All in all the changes carried along a lot of complexity compared to > > plain old ClassLoader.defineClass > > We need to create A LOT of Lookups (might present performance overhead, > > haven't tested yet). > > Extra care needs to be given to the packages for Lookup not to blow up > > while we in fact don't really care where the proxy lands as long as it is > > usable. > Can you expand on the "don't really care" comment? Do you mean that you > don't care about either the defining loader or runtime package? I was referring to the runtime package. What we need is to ensure that there won't be a clash - there can be two classes with same name and different packages, which, for some reason, will have to land in a package created by us. If this happened and we were using a fixed existing package to place them in, it would blow up. So what I meant was that with JDK 8 I don't need to care for this, I just use prefix and I am done with it. In JDK 9, this seems like quite an obstacle. > > > > > > > Another nasty thing is that the code of course needs to work with both, JDK > > 9 and 8. > > While it isn't impossible, it will add a not-so-nice reflection magic layer > > to the mix. > > > Multi-release JARs (JEP 238 [3]) and the ability to compile to older > releases (JEP 247 [4]) might be useful here (you might know about these > features already). Haven't really explored this yet, it might be a way. Gotta see how Maven deals with this feature to allow us to create MRJARs in a reasonable manner. Matej > > -Alan. > > [1] https://bugs.openjdk.java.net/browse/JDK-8171335 > [2] http://openjdk.java.net/jeps/181 > [3] http://openjdk.java.net/jeps/238 > [4] http://openjdk.java.net/jeps/247 >
