RE: JDK10 RFR of JDK-8190258: (tz) Support tzdata2017c and 8190259: test tck.java.time.zone.TCKZoneRules is broken by tzdata2017c

Thank you Martin for your review.   For the java file updates, currently I don’t use any script but just simple “find and replace” whenever possible and rest is manual. I run all the TZ related tests locally on Ubuntu and use JPRT for all other platforms.     Regards, Ramanand.   From:

Re: RFR: jsr166 jdk10 integration wave 5

On Mon, Nov 6, 2017 at 5:33 PM, David Holmes wrote: > On 7/11/2017 8:17 AM, Martin Buchholz wrote: > >> Thanks for the review! >> >> On Mon, Nov 6, 2017 at 1:36 PM, David Holmes > > wrote: >> >> >>

Re: [NEW BUG][JDK10] TimeUnit.timedWait(Object obj, long timeout) doc should emphasize possibility of IllegalMonitorStateException

Thanks - you're right - I filed https://bugs.openjdk.java.net/browse/JDK-8190889 On Tue, Nov 7, 2017 at 2:25 PM, Christoph Dreis wrote: > Hi, > > I don't know if this is considered a real bug or just a lack of > documentation, but I found that although the example

Re: JDK-8067661: transferTo proposal for Appendable

Hi Patrick, On Nov 7, 2017, at 3:01 PM, Patrick Reinhart wrote: > I integrated your changes into my current version which I will look into > tomorrow with Alan at Devoxx. I think adding some implementation note on the > default method implementation will be needed to review

Re: RFR 8190884: java/lang/Runtime/exec/LotsOfOutput fails intermittently

+1 Mandy On 11/7/17 2:55 PM, Roger Riggs wrote: Please review a test update to run LotsOfOutput with 'othervm'; running under the default batch testing mode of agentvm may cause instability. --- old/test/jdk/java/lang/Runtime/exec/LotsOfOutput.java 2017-11-07 17:48:38.398907014 -0500 +++

Re: JDK-8067661: transferTo proposal for Appendable

Hi Brian, I integrated your changes into my current version which I will look into tomorrow with Alan at Devoxx. I think adding some implementation note on the default method implementation will be needed to review later on… I also made an override on the CharBuffer, where we need to add a

Re: [Patch][JDK10] Use Class.getPackageName() where possible

On 11/7/17 6:48 AM, Christoph Dreis wrote: === PATCH === diff -r 67aa34b019e1 src/java.base/share/classes/java/io/ObjectInputFilter.java --- a/src/java.base/share/classes/java/io/ObjectInputFilter.javaMon Nov 06 17:48:00 2017 -0800 +++

Re: RFR 8190884: java/lang/Runtime/exec/LotsOfOutput fails intermittently

+1 > On Nov 7, 2017, at 5:55 PM, Roger Riggs wrote: > > Please review a test update to run LotsOfOutput with 'othervm'; > running under the default batch testing mode of agentvm may cause instability. > > --- old/test/jdk/java/lang/Runtime/exec/LotsOfOutput.java

RFR 8190884: java/lang/Runtime/exec/LotsOfOutput fails intermittently

Please review a test update to run LotsOfOutput with 'othervm'; running under the default batch testing mode of agentvm may cause instability. --- old/test/jdk/java/lang/Runtime/exec/LotsOfOutput.java 2017-11-07 17:48:38.398907014 -0500 +++

Re: [10] RFR 8186046 Minimal ConstantDynamic support

> On 7 Nov 2017, at 12:04, Karen Kinnear wrote: > > Paul, > > Thank you for the explanations. Asking for your help in some test case > construction at the end here: > >>> 3. java/lang/invoke/package-info.java 128-134 >>> Error handling could be clearer. >>> My

Re: JDK-8067661: transferTo proposal for Appendable

I am a little late to this thread, but some alternative verbiage to consider is included below. This wording however diverges from that of [1] so if we want them to remain similar then it’s probably not worth it to change from what has been agreed upon already. Thanks, Brian [1]

Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants

> On 7 Nov 2017, at 14:18, fo...@univ-mlv.fr wrote: >> >> Try passing Class.class to it. To be honest this is somewhat motivated by >> testing when called explicitly. > > see my answer to John. > Fair point, i’ll go with Class. It’s the less smelly option. Updated along with other changes:

[NEW BUG][JDK10] TimeUnit.timedWait(Object obj, long timeout) doc should emphasize possibility of IllegalMonitorStateException

Hi, I don't know if this is considered a real bug or just a lack of documentation, but I found that although the example in the documentation of TimeUnit.timedWait(Object obj, long timeout) shows the correct usage of the method, I think at least the documentation should emphasize the possibility

Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants

> De: "John Rose" > À: "Rémi Forax" > Cc: "Paul Sandoz" , "hotspot-dev" > , "core-libs-dev" > > Envoyé: Mardi 7 Novembre 2017 23:20:01 > Objet: Re: RFR 8187742

Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants

On Nov 7, 2017, at 2:18 PM, fo...@univ-mlv.fr wrote: > >>> >>> Also, why it's not called in invoke ? >>> >> >> What “it” are you referring to? > > validateClassAccess. Because having a MH means you have already been granted access to invoke it. Having a Class does *not* mean you have been

Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants

- Mail original - > De: "Paul Sandoz" > À: "Remi Forax" > Cc: "core-libs-dev" , "hotspot-dev" > > Envoyé: Mardi 7 Novembre 2017 21:54:06 > Objet: Re: RFR 8187742 Minimal set of

Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants

> On 7 Nov 2017, at 13:54, Paul Sandoz wrote: >> >>> If it's not used by an indy, why do we need to test that ? Also, why it's >>> not called in invoke ? >> >> …Enum.valueOf doesn't do a security check; that is its choice. >> This means that if you pass it an enum type

Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants

> On 7 Nov 2017, at 12:59, John Rose wrote: > > Good comments! I can handle a couple of them… > > On Nov 7, 2017, at 11:33 AM, Remi Forax wrote: >> >> I fail to see the point of using validateClassAccess(), if the BSM is used >> through an indy,

Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants

- Mail original - > De: "John Rose" > À: "Rémi Forax" > Cc: "Paul Sandoz" , "hotspot-dev" > , "core-libs-dev" > > Envoyé: Mardi 7 Novembre 2017 21:59:41 >

Re: RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

On Tue, Nov 7, 2017 at 3:45 PM Remi Forax wrote: > > > On November 7, 2017 8:48:43 PM GMT+01:00, Vitaly Davidovich < > vita...@gmail.com> wrote: > >On Tue, Nov 7, 2017 at 1:25 PM Remi Forax wrote: > > > >> My bad, > >> I've calculated that the header was 8

Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants

Good comments! I can handle a couple of them… On Nov 7, 2017, at 11:33 AM, Remi Forax wrote: > > I fail to see the point of using validateClassAccess(), if the BSM is used > through an indy, the type is created only if the caller class can create it, > so the test seems

Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants

Hi Remi, You are asking a lot of the same questions we went through a number of times before landing where we are :-) > On 7 Nov 2017, at 11:33, Remi Forax wrote: > > Hi Paul, > > You have an import static requireNonNull but it is only used once in > nullConstant, all

Re: RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

On November 7, 2017 8:48:43 PM GMT+01:00, Vitaly Davidovich wrote: >On Tue, Nov 7, 2017 at 1:25 PM Remi Forax wrote: > >> My bad, >> I've calculated that the header was 8 bytes instead of 12, so I've >> supposed that the boolean was not stored in a byte.

Re: RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

On 11/7/17 12:15 PM, Brent Christian wrote: Updated webrev here: http://cr.openjdk.java.net/~bchristi/8185925/webrev.04/ Nit: FLAGS should be lower-case (not a constant) Otherwise looks good.   No need for a new webrev. Mandy

Re: RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

On 11/07/2017 09:45 AM, mandy chung wrote: StackFrameInfo.java 38 // Footprint improvement: MemberName::clazz can replace 39 // StackFrameInfo::declaringClass. The above comment can be removed. Whoops - thanks. 41 private final boolean retainClassRef; JVMS [1] has a note

Re: RFR (JDK10/JAXP) 8181151: Fix lint warnings in JAXP repo: cast

Thanks Lance, Roger!  After this, will do a final check, a few JDK 9 deprecations, we'll be completely free from warnings! -Joe On 11/7/2017 12:04 PM, Roger Riggs wrote: +1 On 11/7/2017 2:47 PM, Lance Andersen wrote: Looks OK Joe On Nov 7, 2017, at 2:12 PM, Joe Wang

Review request: JDK-8157246 MHs.arrayLength, arrayElementGetter/Setter, arrayConstructor need to specify invocation-time behavior

http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8157246/webrev.00/index.html This fixes the spec of MethodHandles::arrayLength, arrayConstructor, arrayElementGetter/Setter to specify the behavior if the returned method handle is invoked with null array or invalid index; same runtime

Re: [10] RFR 8186046 Minimal ConstantDynamic support

Paul, Thank you for the explanations. Asking for your help in some test case construction at the end here: >> 3. java/lang/invoke/package-info.java 128-134 >> Error handling could be clearer. >> My understanding is that if a LinkageError or subclass is thrown, this will >> be rethrown >> for

Re: RFR (JDK10/JAXP) 8181151: Fix lint warnings in JAXP repo: cast

+1 On 11/7/2017 2:47 PM, Lance Andersen wrote: Looks OK Joe On Nov 7, 2017, at 2:12 PM, Joe Wang wrote: Hi, This change fixes about 300 [cast] warnings in the JAXP sources. Changes are basically removing the redundant cast, a bit more than that only in one case: in

Re: RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

On Tue, Nov 7, 2017 at 1:25 PM Remi Forax wrote: > My bad, > I've calculated that the header was 8 bytes instead of 12, so I've > supposed that the boolean was not stored in a byte. > > For my culture, why the header is 12 bytes, the pointer to the vtable is > on 64bits and

Re: RFR (JDK10/JAXP) 8181151: Fix lint warnings in JAXP repo: cast

Looks OK Joe > On Nov 7, 2017, at 2:12 PM, Joe Wang wrote: > > Hi, > > This change fixes about 300 [cast] warnings in the JAXP sources. Changes are > basically removing the redundant cast, a bit more than that only in one case: > in XSDUniqueOrKeyTraverser at line 92,

Re: RFR 8187742 Minimal set of bootstrap methods for dynamic constants

Hi Paul, You have an import static requireNonNull but it is only used once in nullConstant, all other methods use Objects.requireNonNull. The test that checks that lookup, name and type are null or not is different in each method, so by example in primitiveClass, if type equals null, you get

RFR (JDK10/JAXP) 8181151: Fix lint warnings in JAXP repo: cast

Hi, This change fixes about 300 [cast] warnings in the JAXP sources. Changes are basically removing the redundant cast, a bit more than that only in one case: in XSDUniqueOrKeyTraverser at line 92, uniqueOrKey was assigned to itself when it meant to be "idc". The change didn't affect the

Re: JDK10 RFR of JDK-8190258: (tz) Support tzdata2017c and 8190259: test tck.java.time.zone.TCKZoneRules is broken by tzdata2017c

Looks good to me too. I've always wondered about how the corresponding java source files are maintained. Is it all manual or do y'all have some magic script to help keep IANA data and java data aligned? Do we need more testing that mistakes don't creep in? On Tue, Nov 7, 2017 at 3:41 AM,

Re: JDK10 RFR of JDK-8190258: (tz) Support tzdata2017c and 8190259: test tck.java.time.zone.TCKZoneRules is broken by tzdata2017c

Looks good. Naoto On 11/7/17 3:41 AM, Ramanand Patil wrote: Hi Naoto, Thank you for pointing that. I have modified both the affected files(ZoneName.java from src and test). Here is the updated Webrev: http://cr.openjdk.java.net/~rpatil/8190258+8190259/webrev.01/ Regards, Ramanand.

RFR 8187742 Minimal set of bootstrap methods for dynamic constants

Hi, Please review the patch that adds a minimal set of bootstrap methods can be be used for producing dynamic constants: http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webrev/ The aim

Re: RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

My bad, I've calculated that the header was 8 bytes instead of 12, so I've supposed that the boolean was not stored in a byte. For my culture, why the header is 12 bytes, the pointer to the vtable is on 64bits and can not be compressed like the other oops ? Rémi On November 7, 2017 5:42:33

Re: RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

On 11/6/17 4:23 PM, Brent Christian wrote: Please review my code change for this.  The webrev is here: http://cr.openjdk.java.net/~bchristi/8185925/webrev.03/ It's a good footprint improvement.  Thanks for doing this. StackFrameInfo.java 38 // Footprint improvement:

Re: RFR: 8185925 & 8153682 (footprint reduction of java.lang.StackFrameInfo)

Hi, Remi Thanks for the idea. From my reading of the jol ouput: http://cr.openjdk.java.net/~bchristi/8185925/StackFrameInfo.jol.rmDeclClass.txt I believe retainClassRef is already being stored in a single byte (i.e. SIZE of 1): OFFSET SIZE TYPE DESCRIPTION ... 10 1

Re: [10] RFR 8186046 Minimal ConstantDynamic support

> On 5 Nov 2017, at 16:55, David Holmes wrote: > > On 4/11/2017 7:28 AM, Paul Sandoz wrote: >>> On 3 Nov 2017, at 11:14, Karen Kinnear wrote: >>> 6. SD::find_java_mirror_for_type >>> You have resolve_or_null/fail doing CHECK_(empty) which

RE: [Patch][JDK10] Use Class.getPackageName() where possible

Hi, > On 11/3/17 6:52 PM, Bernd Eckenfels wrote: >> The private static helper in ObjectStreamClass became a oneliner and can be >> removed and the callsites can transform from getPackageName(c) to >> c.getPackageName(). > Good catch. we should clean that up. > Christoph - can you send a

RE: JDK10 RFR of JDK-8190258: (tz) Support tzdata2017c and 8190259: test tck.java.time.zone.TCKZoneRules is broken by tzdata2017c

Hi Naoto, Thank you for pointing that. I have modified both the affected files(ZoneName.java from src and test). Here is the updated Webrev: http://cr.openjdk.java.net/~rpatil/8190258+8190259/webrev.01/ Regards, Ramanand. > -Original Message- > From: Naoto Sato > Sent: Tuesday,