[core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control (original) (raw)

David Holmes david.holmes at oracle.com
Tue May 22 06:24:09 UTC 2018

Sorry Remi I overlooked responding to this. Thanks for confirming I don't have to do anything at this time.

David

On 17/05/2018 8:02 PM, forax at univ-mlv.fr wrote:

----- Mail original ----- De: "David Holmes" <david.holmes at oracle.com> À: "Remi Forax" <forax at univ-mlv.fr>, "Alan Bateman" <Alan.Bateman at oracle.com> Cc: "core-libs-dev" <core-libs-dev at openjdk.java.net> Envoyé: Jeudi 17 Mai 2018 10:37:46 Objet: Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control Hi Remi, Hi David

On 17/05/2018 6:16 PM, Remi Forax wrote: Hi all,

----- Mail original ----- De: "Alan Bateman" <Alan.Bateman at oracle.com> À: "David Holmes" <david.holmes at oracle.com>, "core-libs-dev" <core-libs-dev at openjdk.java.net> Envoyé: Mardi 15 Mai 2018 15:53:44 Objet: Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

On 15/05/2018 01:52, David Holmes wrote: This review is being spread across four groups: langtools, core-libs, hotspot and serviceability. This is the specific review thread for core-libs - webrev:

http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v1/ [...] Maybe a question for Kumar but are we planning to pull in any ASM updates for JDK 11? NestMembers extends Attribute looks okay, I'm less sure about the change to ClassReader as I don't know if there is somewhere else in ASM that has the list of attributes to always parse. With my ASM hat, the current master of ASM (the release of ASM 6.2 is scheduled for the next week-end) already supports nestmates (and constant dynamic and preview feature) so i suppose that at some point in the future Kumar will merge it to the JDK. Unfortunately Kumar is no longer with us. I will miss him. We have recently changed the way we implement features in ASM, instead of having features lingering in different branches, we now integrate them directly in the master under an experimental flag (ASM7EXPERIMENTAL), which means for the JDK that it is no longer necessary to wait until the release of ASM 7 because it can use the experimental support of ASM 6.2. (note that experimental doesn't mean full of bugs, or half baked or anything like this, it means that the feature is not yet integrated in a released JDK). I've taking a look to the code in this patch, i've two comments, - in Attributes, it seems that the code store the bytecode slice corresponding to the attribute only to use its length as argument of the ByteVector which is like an ArrayList of byte, it grows automatically so the initial capacity is a perf optimization. Perhaps the byte array is used somewhere else ? - patching the ClassReader.accept is really a quick hack because the method accept with 3 arguments is not patched so if this method is called somewhere in the JDK it will behave as it should. I'll take a look at this. To be honest I don't even remember who provided those changes ... I thought you had provided feedback at some point in the past :) yes, i may have, i do not remember. There's a valhalla-dev email with a link that's no longer valid: https://gitlab.ow2.org/asm/asm/tree/NESTMATES as i said above, the support has been integrated in the master of ASM, see https://gitlab.ow2.org/asm/asm/blob/master/asm/src/main/java/org/objectweb/asm/ClassVisitor.java#L156 and https://gitlab.ow2.org/asm/asm/blob/master/asm/src/main/java/org/objectweb/asm/ClassVisitor.java#L246 the current code is fine, just not optimal, but given that when the JDK will use ASM 6.2, this code will have to be removed, i do not think you should spend some time one this. Thanks, David regards, Rémi

More information about the core-libs-dev mailing list