RFR (JDK 12/java.base) 8213325: (props) Properties.loadFromXML does not fully comply with the spec

Hi, Please review a patch to bring the small footprint parser for java.util.Properties compliant with the specification. JBS: https://bugs.openjdk.java.net/browse/JDK-8213325 webrevs: http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev/ Thanks, Joe

Re: RFR: JDK-8213480: update internal ASM version to 7.0

On 14/11/2018 12:40 am, Brian Goetz wrote: But don’t they?  We intend to start generating condy in classfiles from javac soon; at that point, anyone not using ASM7 will fail when reading classfiles generated by javac. See later email. They need this for nestmates now as well. Cheers, David O

Re: RFR: JDK-8213480: update internal ASM version to 7.0

Hi Igor, Thanks for your comment. I have already applied in my local copy. Vicente On 11/13/18 8:30 PM, Igor Ignatyev wrote: Hi Vicente, you need to replace "@ignore 8194951: some mlvm tests fail w/ ASMv7" w/ "@ignore 8194951" in all the occurrences, as we have monitoring tools which expect

Re: RFR: JDK-8213480: update internal ASM version to 7.0

Hi Vicente, you need to replace "@ignore 8194951: some mlvm tests fail w/ ASMv7" w/ "@ignore 8194951" in all the occurrences, as we have monitoring tools which expect @ignore to be followed by a space-separated list of bug ids. the rest looks good to me. Thanks, -- Igor > On Nov 13, 2018, at

Re: RFR: 8148188: Enhance the security libraries to record events of interest

Confused. Aren't all Security properties security-related? This is not about normal system properties. And the method name in the latest webrev is "isSecurityProperty" without the "JDK" word. I assume this means you don't care about the difference between SE properties and JDK properties. --Ma

Re: RFR: JDK-8213480: update internal ASM version to 7.0

Hi Alan, On 11/13/18 9:18 AM, Alan Bateman wrote: On 13/11/2018 14:00, Vicente Romero wrote: any other comment after the last iteration? we are in a bit of a hurry to push this before the JDK 12 train departs :( The original patch updated all the use sites (and tests) to specify ASM7 for the A

Re: RFR: XXXS fix 1-char typo in doc comment; JDK-8213820: unknown tag: @returns

+1 Mandy On 11/13/18 4:12 PM, Jonathan Gibbons wrote: Please review this trivial fix for an incorrectly spelt tag name in public documentation. $ hg diff -R open diff -r bbbcd90f0adb src/java.base/share/classes/java/lang/ref/Reference.java --- a/src/java.base/share/classes/java/lang/ref/Ref

RFR: XXXS fix 1-char typo in doc comment; JDK-8213820: unknown tag: @returns

Please review this trivial fix for an incorrectly spelt tag name in public documentation. $ hg diff -R open diff -r bbbcd90f0adb src/java.base/share/classes/java/lang/ref/Reference.java --- a/src/java.base/share/classes/java/lang/ref/Reference.java  Tue Nov 13 16:49:58 2018 -0500 +++ b/src/jav

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

On 11/13/18, 12:52 PM, Roger Riggs wrote: Hi, There are enough files unique to each platform to put them in separate packages otherwise you get too many (IMHO) files in a single package/directory and its harder to tell which go with which. There isn't much of a problem with classes being

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

Hello, May I chime in a little on the jpackager. I have been using it with OpenJDK 11, as backported by Johan Vos from Gluon. It has worked fine, but I have noticed some flaws. 1) The control file for DEB package does not set correct description --name test --description This is a Test Applicatio

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

On 11/13/2018 4:08 PM, Roger Riggs wrote: Hi, A few high level comments: The JDK already has a command option parser (JoptSimple) in the module jdk.internal.opt and the System Logger.  Why not use them for the argument parsing and logging? We have an RFE to convert argument parsing to jop

Re: RFR 4947890 : Minimize JNI upcalls in system-properties initialization

Hi Roger, I somehow wonder whether that coding could be further simplified and maybe made faster by just returning a big byte array or String from the hotspot containing a concatenation of key-value pairs, each separated by \0, e.g. like this: "os.name\0linux\0os.arch\0i386\0java.io.tmpdir\0/tmp\

Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

Along the same lines, if subclasses would typically benefit from overriding both skip() and skipNBytes(), how about we call out skipNBytes() in the skip() docs? Either add a @see, or maybe append: "Subclasses are encouraged to provide a more efficient implementation of this method" + " (as we

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

Hi, A few high level comments: The JDK already has a command option parser (JoptSimple) in the module jdk.internal.opt and the System Logger.  Why not use them for the argument parsing and logging? Similarly, we've been encouraging developers to use the java.nio.file APIs and get away from

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

I agree with this and would take it further. 1 file is in ./share/classes/jdk/jpackager/internal/builders - why not just ./share/classes/jdk/jpackager/internal 2 files are in ./share/classes/jdk/jpackager/internal/bundlers - why not just in ./share/classes/jdk/jpackager/internal 1 file is i

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

Question .. why is "mac", "linux" and "windows" necessary in the package name here ?  src/jdk.jpackager/macosx/classes/jdk/jpackager/internal/mac/MacAppBundler.java   src/jdk.jpackager/windows/classes/jdk/jpackager/internal/builders/windows/WindowsAppImageBuilder.java src/jdk.jpackager/linux/cla

Re: RFR 4947890 : Minimize JNI upcalls in system-properties initialization

Hi Andrew, For the sake of brevity in the source, the PUTPROP and PUTPROP_PlatformString macros prefix those "_xxx" arguments with "jdk_internal_util_SystemProps_Raw_" so they refer to the indexes defined by @native in SystemProps.java. Possibly more readable (but still shortish) would be to i

RE: RFR 4947890 : Minimize JNI upcalls in system-properties initialization

I'm not a reviewer, but aren't names that begin with _ in the global namespace (_awt_headless_NDX for example) reserved for the C/C++ library? Thanks, Andrew -Original Message- From: core-libs-dev On Behalf Of Roger Riggs Sent: Tuesday, November 13, 2018 8:00 AM To: Core-Libs-Dev ; h

Re: RFR: 8148188: Enhance the security libraries to record events of interest

Looking good, a couple of comments/questions: * src/java.base/share/classes/java/security/Security.java The isJdkSecurityProperty method could return false positives, for example there may be a non-JDK property starting with "security.". I was thinking it would be better to put all the JDK pro

RFR 4947890 : Minimize JNI upcalls in system-properties initialization

Please review a re-implementation of the initialization of System properties moving some functions from native to Java. The upcalls from native to java for each property are replaced by a mechanism to gather the platform, VM and command line properties and return them from a single JNI call.  T

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

On 11/13/2018 3:39 AM, Alan Bateman wrote: On 12/11/2018 21:40, Philip Race wrote:   74   75 static String getTmpDir() {   76 String os = System.getProperty("os.name").toLowerCase();   77 if (os.contains("win")) {   78 return System.getProperty("user.home")   79 

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

On 11/12/2018 6:36 PM, Alan Snyder wrote: Can someone say whether the two-step package creation feature is implemented and explain how to use it? Yes - 1.) "jpackager create-image -o output -n app-name ..." 2.) "jpackager create-installer -o installer-output -n installer-name --app-image

Both Collector.of() are not correctly typed

Last year, a student of mine as remarked that the two variants of Collector.of() are not correctly typed (the wildcards are missing) and obviously, this year it's one of my teaching assistant that has found exactly the same issue. So let's fix this issue. Adding the wildcards is both source and

Re: Stream Method Proposal: long count(Predicate predicate)

Exactly right. Note, though, that count() is _already_ a convenience method! You could call .collect(counting()) instead. So, why were we willing to add count() and similar methods, when we’re not willing to add this one? Several reasons: - Discoverability. Collect() is complicated, and un

Re: RFR: JDK-8213480: update internal ASM version to 7.0

But don’t they? We intend to start generating condy in classfiles from javac soon; at that point, anyone not using ASM7 will fail when reading classfiles generated by javac. > On Nov 8, 2018, at 4:03 AM, David Holmes wrote: > > Is it that case that the code the uses the ASM library, like th

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

Yes - The intent of getTmpDir() here is to match the GetTempDirectory() in launcher, which this does for the three supported platforms, but there is no need to check for the unsupported platforms. I will clean this up as you suggest as part ofJDK-8213756

Re: RFR - JDK-8203442 String::transform (Code Review)

> An argument against re-using the name map() for this String method is that > Stream.map() and Optional.map() act on the element(s) of the "container" the > method is invoked upon, and return the same raw part of type with type > parameter adjusted, while String.map() would be passing the targe

Re: Stream Method Proposal: long count(Predicate predicate)

It is entirely feasible. But, that’s not the criteria for adding it. There are frequent calls for fusing methods that are commonly used together, such as filter+map. Our position has been that the stream API is better served by providing mostly orthogonal primitives, and letting developers c

Re: Method references in annotations

The story here is: we’ve completed our analysis on the feature, and concluded that the feature is sound (many such features don’t survive this cut.) But, there is actually quite a bit of work needed in the underlying infrastructure to implement it, largely having to do with annotation processin

Re: RFR: JDK-8213480: update internal ASM version to 7.0

On 13/11/2018 14:00, Vicente Romero wrote: any other comment after the last iteration? we are in a bit of a hurry to push this before the JDK 12 train departs :( The original patch updated all the use sites (and tests) to specify ASM7 for the API version. I just checked the webrev again now and

Re: RFR: JDK-8213480: update internal ASM version to 7.0

any other comment after the last iteration? we are in a bit of a hurry to push this before the JDK 12 train departs :( Thanks, Vicente On 11/8/18 11:39 AM, Vicente Romero wrote: Hi David, Igor On 11/7/18 10:03 PM, David Holmes wrote: Hi Vicente, All of the javadoc comment reformatting makes

Re: RFR - JDK-8203442 String::transform (Code Review) (was: RFR - JDK-8203442 String::transform (Code Review))

I'm very uncomfortable about adding this method as is. Firstly, as Peter says (in a different thread), naming this method `map()` is inconsistent with `Stream` and `Optional`. Adding `map()` to `String` would be: public String map(Function); Not that such a method would be particularly useful!

Re: RFR - JDK-8203442 String::transform (Code Review)

On 9/21/18 12:22 PM, Alan Bateman wrote: On 18/09/2018 18:52, Jim Laskey wrote: Please review the code for String::transform. The goal is to provide a String instance method to allow function application of custom transformations applied to an instance of String. webrev: http://cr.openjdk.

Re: The new optimized version of Dual-Pivot Quicksort

Thanks, Tagir. The benchmark was indeed incorrect. There was also another mistake: dualPivot() and radix() sorted different arrays. I believe I fixed it now: https://gist.github.com/orionll/595d10ff37ffe0d8c3824e734055cf00 Benchmark (seed)(size) Mode Cnt Score Error Units

Re: RFR - JDK-8203442 String::transform (Code Review) (was: RFR - JDK-8203442 String::transform (Code Review))

Hi Jim, > updated webrev: > http://cr.openjdk.java.net/~jlaskey/8203442/webrev-02/index.html > src/java.base/share/classes/java/lang/String.java 2983 * @param class of the result Maybe "the type of the result" would be

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

On 12/11/2018 21:40, Philip Race wrote:   74   75 static String getTmpDir() {   76 String os = System.getProperty("os.name").toLowerCase();   77 if (os.contains("win")) {   78 return System.getProperty("user.home")   79 + "\\AppData\\LocalLow\\

Re: The new optimized version of Dual-Pivot Quicksort

Hello, Zheka! Seems that your benchmark is wrong: after the first iteration (which is part of warmup) you're always sorting an array which is already sorted, so in fact you are testing how algorithms behave on already sorted arrays. With best regards, Tagir Valeev. On Mon, Nov 12, 2018 at 10:08 A