Re: [15] RFR JDK-8247785: Small clarification to the javadoc about builtin class loaders

2020-06-23 Thread David Holmes
On 24/06/2020 1:17 pm, Mandy Chung wrote: On 6/23/20 7:48 PM, David Holmes wrote: Hi Mandy, The trouble with small clarifications is that they tend to draw attention to larger issues :) On 24/06/2020 7:42 am, Mandy Chung wrote: On 6/23/20 12:01 PM, Roger Riggs wrote: Hi Mandy, There

Re: [15] RFR JDK-8247785: Small clarification to the javadoc about builtin class loaders

2020-06-23 Thread Mandy Chung
On 6/23/20 7:48 PM, David Holmes wrote: Hi Mandy, The trouble with small clarifications is that they tend to draw attention to larger issues :) On 24/06/2020 7:42 am, Mandy Chung wrote: On 6/23/20 12:01 PM, Roger Riggs wrote: Hi Mandy, There may be a missing "to" in: + *

Re: [15] RFR JDK-8247785: Small clarification to the javadoc about builtin class loaders

2020-06-23 Thread David Holmes
Hi Mandy, The trouble with small clarifications is that they tend to draw attention to larger issues :) On 24/06/2020 7:42 am, Mandy Chung wrote: On 6/23/20 12:01 PM, Roger Riggs wrote: Hi Mandy, There may be a missing "to" in: + * Platform classes are visible the platform class

Re: Exceptions priority in InputStream.read(byte[], int, int)

2020-06-23 Thread Joe Darcy
Hello, As a general comment, over time we come to regard specifications like "if these three pre-conditions of the method are all violated, this exception will be thrown; if only these two pre-conditions are violated, this other exception will be thrown" as over specifications and best

Re: RFR: JDK-8242451 : ensure semantics of non-capturing lambdas are preserved independent of execution mode

2020-06-23 Thread Mandy Chung
Hi Gilles, Additional comments: 215 try { 216 return new ConstantCallSite(Lookup.IMPL_LOOKUP.findStaticGetter(innerClass, LAMBDA_INSTANCE_FIELD, invokedType.returnType())); 217 } catch (ReflectiveOperationException e) { 218 throw new LambdaConversionException("Exception finding constructor",

Re: RFR 8248184: AMPM_OF_DAY doc fix in ChronoField

2020-06-23 Thread naoto . sato
Thanks, Brian and Lance. I forgot to update the copyright year. Pushed the changeset with the year update. Naoto On 6/23/20 3:09 PM, Lance Andersen wrote: +1 Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1

Re: RFR: JDK-8246212: JPKG001-012: application icon is missing in Control Panel Add/Remove

2020-06-23 Thread Andy Herrick
looks good /Andy On 6/23/2020 1:53 PM, Alexey Semenyuk wrote: Please review fix [2] for jpackage bug [1]. Fix how icon is configured for installers on Windows. The value of ARPPRODUCTICON property should point to an entry in Icon table of msi rather to a path of icon file in install

Re: RFR: JDK-8246212: JPKG001-012: application icon is missing in Control Panel Add/Remove

2020-06-23 Thread alexander . matveev
Hi Alexey, Looks good. Thanks, Alexander On 6/23/20 10:53 AM, Alexey Semenyuk wrote: Please review fix [2] for jpackage bug [1]. Fix how icon is configured for installers on Windows. The value of ARPPRODUCTICON property should point to an entry in Icon table of msi rather to a path of icon

Re: RFR 8248184: AMPM_OF_DAY doc fix in ChronoField

2020-06-23 Thread Lance Andersen
+1 Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com > On Jun 23, 2020, at 5:44 PM, naoto.s...@oracle.com wrote: > > Hi, > > Please review this

Re: RFR 8248184: AMPM_OF_DAY doc fix in ChronoField

2020-06-23 Thread Brian Burkhalter
Hi Naoto, +1 Brian > On Jun 23, 2020, at 2:44 PM, naoto.s...@oracle.com wrote: > > Hi, > > Please review this small doc fix for the following issue: > > https://bugs.openjdk.java.net/browse/JDK-8248184 > > The proposed patch is here: > > ---

RFR 8248184: AMPM_OF_DAY doc fix in ChronoField

2020-06-23 Thread naoto . sato
Hi, Please review this small doc fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8248184 The proposed patch is here: --- old/src/java.base/share/classes/java/time/temporal/ChronoField.java 2020-06-23 14:43:43.0 -0700 +++

Re: [15] RFR JDK-8247785: Small clarification to the javadoc about builtin class loaders

2020-06-23 Thread Mandy Chung
On 6/23/20 12:01 PM, Roger Riggs wrote: Hi Mandy, There may be a missing "to" in: + * Platform classes are visible the platform class loader ++ * Platform classes are visible *via* the platform class loader I caught this accidental change too. The second change seems to be

Re: RFR: JDK-8242451 : ensure semantics of non-capturing lambdas are preserved independent of execution mode

2020-06-23 Thread Mandy Chung
On 6/23/20 11:38 AM, Brian Goetz wrote: On 6/23/2020 2:08 PM, Gilles Duboscq wrote: In 8232806, a system property was introduce to disable eager initialization of the classes generated by the InnerClassLambdaMetafactory (`jdk.internal.lambda.disableEagerInitialization`). However, when

Re: [15] RFR JDK-8247785: Small clarification to the javadoc about builtin class loaders

2020-06-23 Thread Mandy Chung
On 6/23/20 12:15 PM, Brent Christian wrote: Hi, Mandy For: @@ -152,7 +152,7 @@   * The system class loader is typically used to define classes on the   * application class path, module path, and JDK-specific tools.   * The platform class loader is a parent or an ancestor of

Re: [15] RFR JDK-8247785: Small clarification to the javadoc about builtin class loaders

2020-06-23 Thread Brent Christian
Hi, Mandy For: @@ -152,7 +152,7 @@ * The system class loader is typically used to define classes on the * application class path, module path, and JDK-specific tools. * The platform class loader is a parent or an ancestor of the system class - * loader that all platform

Re: [15] RFR JDK-8247785: Small clarification to the javadoc about builtin class loaders

2020-06-23 Thread Roger Riggs
Hi Mandy, There may be a missing "to" in: + * Platform classes are visible the platform class loader ++ * Platform classes are visible *via* the platform class loader The second change seems to be self referential using "parent" to define itself. And pre-existing in the

Re: RFR: JDK-8242451 : ensure semantics of non-capturing lambdas are preserved independent of execution mode

2020-06-23 Thread Brian Goetz
On 6/23/2020 2:08 PM, Gilles Duboscq wrote: In 8232806, a system property was introduce to disable eager initialization of the classes generated by the InnerClassLambdaMetafactory (`jdk.internal.lambda.disableEagerInitialization`). However, when `disableEagerInitialization` is true, even

Re: [15] RFR JDK-8247785: Small clarification to the javadoc about builtin class loaders

2020-06-23 Thread Alan Bateman
On 23/06/2020 19:03, Mandy Chung wrote: Small clarification about the parent of the system class loader in the ClassLoader class spec: diff --git a/src/java.base/share/classes/java/lang/ClassLoader.java b/src/java.base/share/classes/java/lang/ClassLoader.java ---

RFR: JDK-8242451 : ensure semantics of non-capturing lambdas are preserved independent of execution mode

2020-06-23 Thread Gilles Duboscq
In 8232806, a system property was introduce to disable eager initialization of the classes generated by the InnerClassLambdaMetafactory (`jdk.internal.lambda.disableEagerInitialization`). However, when `disableEagerInitialization` is true, even for non-capturing lambdas, the capturing lambda

[15] RFR JDK-8247785: Small clarification to the javadoc about builtin class loaders

2020-06-23 Thread Mandy Chung
Small clarification about the parent of the system class loader in the ClassLoader class spec: diff --git a/src/java.base/share/classes/java/lang/ClassLoader.java b/src/java.base/share/classes/java/lang/ClassLoader.java --- a/src/java.base/share/classes/java/lang/ClassLoader.java +++

RFR: JDK-8246212: JPKG001-012: application icon is missing in Control Panel Add/Remove

2020-06-23 Thread Alexey Semenyuk
Please review fix [2] for jpackage bug [1]. Fix how icon is configured for installers on Windows. The value of ARPPRODUCTICON property should point to an entry in Icon table of msi rather to a path of icon file in install directory. The issue shows up only in downgrade scenarios. - Alexey

Re: RFR: 8248131: Simplify ServicesCatalog provider handling

2020-06-23 Thread Claes Redestad
Hi Alan, Mandy, thanks for reviewing. I've ran all java/lang/instrument and the com/sun/tools/attach/modules tests, as well as added tier2 to the testing mix. /Claes On 2020-06-23 16:09, Alan Bateman wrote: On 23/06/2020 09:56, Claes Redestad wrote: Hi, the current implementation of

Re: RFR: 8247532, 8248135: Records deserialization is slow + Build microbenchmarks with --enable-preview

2020-06-23 Thread Chris Hegarty
> On 23 Jun 2020, at 14:49, Peter Levart wrote: > > ... > > Ok, I'm going to push this to jdk15. Thank you Peter. This is a really nice change. As a follow on, and not for JDK 15, I observe that Class::isRecord0 / JVM_IsRecord shows up as consuming a significant amount of time, more than

Re: RFR: 8248131: Simplify ServicesCatalog provider handling

2020-06-23 Thread Mandy Chung
On 6/23/20 1:56 AM, Claes Redestad wrote: Hi, the current implementation of ServicesCatalog uses an internal providers method, which is only used to add ServiceProviders. Providing an addProviders method instead means we can streamline this, to the tune of a very small startup win. Bug:   

Re: java.util.Base64 accepts non-canonical encodings

2020-06-23 Thread Raffaello Giulietti
OK, I'll avoid the checks in the patch intended for general publication but will add them separately just to see which of current tests would fail. I'll then later open a separate issue for discussing a possible extension of the API and a possible CSR. Greetings Raffaello On 2020-06-23

Re: java.util.Base64 accepts non-canonical encodings

2020-06-23 Thread Roger Riggs
Hi, This is a case where having some more interoperability testing could be informative though there are likely many adhoc Base64 encoders and its not practical to test against them. Introducing a new mode or option creates an undesirable fuzzyness to the API. It won't help existing uses

Re: Exceptions priority in InputStream.read(byte[], int, int)

2020-06-23 Thread Roger Riggs
Hi Raffaello, Exceptions related to the arguments should be highest priority so they are not obscured by any more dynamic state related to the stream. There's no specific order between checks of the arguments, though in this case the NPE would naturally occur before IOOBE. There are more

Re: Exceptions priority in InputStream.read(byte[], int, int)

2020-06-23 Thread Martin Buchholz
When there is a choice of exceptions to be thrown, it's not usually specified which one wins. Usually there's a natural order of operations that cause e.g. argument checking to happen first. Sometimes a code change can cause a different exception to be thrown, and that occasionally causes

java.util.Base64 accepts non-canonical encodings

2020-06-23 Thread Raffaello Giulietti
Hi Roger, I didn't yet implement the strict check since, as you point out, it could harm existing code in the wild, even if the OpenJDK test would all pass. That's why I'm wondering if it would make sense to extend the existing API to have the check as an additional option. Greetings

Exceptions priority in InputStream.read(byte[], int, int)

2020-06-23 Thread Raffaello Giulietti
Hi, the InputStream.read(byte[], int, int) method [1] can throw * IOException * NullPointerException * IndexOutOfBoundsException Is there a recommended priority for the conditions associated to the exceptions to be checked? For example, if the arguments are invalid and the stream is already

Re: java.util.Base64 accepts non-canonical encodings

2020-06-23 Thread Roger Riggs
Hi Raffaello, I think the concern over accepting non-canonical encodings would be compatibility. It would rude to implement the strictness and have applications start failing. But it is likely an oversight since existing code checks for other invalid encodings. Do any of the existing tests

Re: RFR: 8248131: Simplify ServicesCatalog provider handling

2020-06-23 Thread Alan Bateman
On 23/06/2020 09:56, Claes Redestad wrote: Hi, the current implementation of ServicesCatalog uses an internal providers method, which is only used to add ServiceProviders. Providing an addProviders method instead means we can streamline this, to the tune of a very small startup win. Bug:   

Re: RFR: 8247532, 8248135: Records deserialization is slow + Build microbenchmarks with --enable-preview

2020-06-23 Thread Peter Levart
On 6/23/20 2:47 PM, Chris Hegarty wrote: On 23 Jun 2020, at 10:46, Chris Hegarty wrote: On 23 Jun 2020, at 10:17, Peter Levart wrote: ... http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.08/ Good stuff. Reviewed. I am going to take this latest change and run

Re: Math.mutiplyHigh() is signed

2020-06-23 Thread Raffaello Giulietti
Hi, in defense of the current spec, integer arithmetic is signed by default and unsigned behavior is evident from the method names, like Long::compareUnsigned or Long::divideUnsigned But your spec is even clearer ;-) Greetings Raffaello In retrospect I don't know why I expected

Math.mutiplyHigh() is signed

2020-06-23 Thread David Lloyd
In retrospect I don't know why I expected Math.multiplyHigh() to return the high word of the unsigned product of two 64-bit numbers, given that long is signed; in my defence however, the docs don't actually seem to specify. WDYT about a patch like this to clarify? diff --git

java.util.Base64 accepts non-canonical encodings

2020-06-23 Thread Raffaello Giulietti
Hi, RFC 4648, in section "3.5. Canonical Encoding", prescribes that pad bits must be set to zero. However, the current decoder implementation in java.util.Base64 accepts non-canonical encodings as well. For example, all of the following four encodings KCk= KCl= KCm= KCn= where only the

Re: RFR: 8247532, 8248135: Records deserialization is slow + Build microbenchmarks with --enable-preview

2020-06-23 Thread Chris Hegarty
> On 23 Jun 2020, at 10:46, Chris Hegarty wrote: > > > >> On 23 Jun 2020, at 10:17, Peter Levart wrote: >> >> ... >> http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.08/ > > Good stuff. Reviewed. > > I am going to take this latest change and run it through our

Re: RFR: 8247532, 8248135: Records deserialization is slow + Build microbenchmarks with --enable-preview

2020-06-23 Thread Chris Hegarty
> On 23 Jun 2020, at 10:17, Peter Levart wrote: > > ... > http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.08/ Good stuff. Reviewed. I am going to take this latest change and run it through our internal build and test system. Will post the results here soon.

Re: RFR: 8247532, 8248135: Records deserialization is slow + Build microbenchmarks with --enable-preview

2020-06-23 Thread Magnus Ihse Bursie
On 2020-06-23 11:17, Peter Levart wrote: Including build-dev since this patch is adding new issue 8248135: https://bugs.openjdk.java.net/browse/JDK-8248135 So here's new webrev with a patch for building benchmarks with --enable-preview included:

Re: RFR: 8247532, 8248135: Records deserialization is slow + Build microbenchmarks with --enable-preview

2020-06-23 Thread Peter Levart
Thanks, Magnus. Peter On 6/23/20 11:27 AM, Magnus Ihse Bursie wrote: On 2020-06-23 11:17, Peter Levart wrote: Including build-dev since this patch is adding new issue 8248135: https://bugs.openjdk.java.net/browse/JDK-8248135 So here's new webrev with a patch for building benchmarks with

Re: RFR: 8247532, 8248135: Records deserialization is slow + Build microbenchmarks with --enable-preview

2020-06-23 Thread Claes Redestad
Looks good to me! /Claes On 2020-06-23 11:17, Peter Levart wrote: Including build-dev since this patch is adding new issue 8248135: https://bugs.openjdk.java.net/browse/JDK-8248135 So here's new webrev with a patch for building benchmarks with --enable-preview included:

Re: RFR: 8247532, 8248135: Records deserialization is slow + Build microbenchmarks with --enable-preview

2020-06-23 Thread Peter Levart
Including build-dev since this patch is adding new issue 8248135: https://bugs.openjdk.java.net/browse/JDK-8248135 So here's new webrev with a patch for building benchmarks with --enable-preview included: http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.08/

RFR: 8248131: Simplify ServicesCatalog provider handling

2020-06-23 Thread Claes Redestad
Hi, the current implementation of ServicesCatalog uses an internal providers method, which is only used to add ServiceProviders. Providing an addProviders method instead means we can streamline this, to the tune of a very small startup win. Bug:

Re: RFR: 8247532: Records deserialization is slow

2020-06-23 Thread Claes Redestad
On 2020-06-23 10:06, Claes Redestad wrote: Hi, On 2020-06-23 09:49, Peter Levart wrote: Hi Chris, Claes, Ok then, here's with benchmark included: http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.07/ I haven't been able to run the benchmark with "make test"

Re: RFR: 8247532: Records deserialization is slow

2020-06-23 Thread Claes Redestad
Hi, On 2020-06-23 09:49, Peter Levart wrote: Hi Chris, Claes, Ok then, here's with benchmark included: http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.07/ I haven't been able to run the benchmark with "make test" though. I have tried various ways to pass javac

Re: RFR: 8247532: Records deserialization is slow

2020-06-23 Thread Peter Levart
Hi Chris, Claes, Ok then, here's with benchmark included: http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.07/ I haven't been able to run the benchmark with "make test" though. I have tried various ways to pass javac options to build like: make test