Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

2018-12-03 Thread Alan Bateman
On 03/12/2018 16:50, Claes Redestad wrote: : The extra Names added to KNOWN_NAMES was my doing, and it was based on commonly used attributes in Jar file manifests scanned from a set commonly used applications as an alternative to building up a Name cache dynamically (which is frought with oth

Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()

2018-12-03 Thread Tagir Valeev
Hello! > In CopiesList.equals() please use eq() instead of Objects.equal() (see a > comment at the line 5345). Ok > I think you should use iterator() instead of listIterator(). See the > explanation here: > http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-April/052472.html Ok. I won

Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()

2018-12-03 Thread Zheka Kozlov
I think you should use iterator() instead of listIterator(). See the explanation here: http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-April/052472.html вт, 4 дек. 2018 г. в 12:23, Tagir Valeev : > Hello! > > Thank you for your comments! > > Yes, deserialization will be broken if we ass

Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()

2018-12-03 Thread Ivan Gerasimov
Hi Tagir! I think what you have in the last webrev looks good! In CopiesList.equals() please use eq() instead of Objects.equal() (see a comment at the line 5345). With kind regards, Ivan On 12/3/18 9:22 PM, Tagir Valeev wrote: Hello! Thank you for your comments! Yes, deserialization will

Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()

2018-12-03 Thread Tagir Valeev
Hello! Thank you for your comments! Yes, deserialization will be broken if we assume that size is never 0. Also we'll introduce referential identity Collections.nCopies(0, x) == Collections.nCopies(0, y) which might introduce slight semantics change in existing programs. Once I suggested to wire

Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

2018-12-03 Thread David Holmes
Hi Claes, Meta-comment: are these Names candidates for the forthcoming compile-time evaluation of constants? Just wondering if these optimizations (and even the archiving itself) will be moot in the future? Thanks, David On 4/12/2018 2:02 am, Claes Redestad wrote: Hi, initializing java.uti

Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()

2018-12-03 Thread Stuart Marks
I believe it makes sense to override CopiesList.equals() Also: contains(), iterator(), listIterator() equals(): sure contains() is already overridden. Not sure there's much benefit to overriding the iterators. s'marks

Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()

2018-12-03 Thread Stuart Marks
The general rule for JDK core classes (including collections, even private implementations) is that we try to preserve backward *and* forward serialization compatibility. This doesn't apply to all classes in the JDK, though. For example, javax.swing.JComponent is serializable, but it includes a

Re: RFR: 8212794 IBM-964 is required for AIX default charset

2018-12-03 Thread Ichiroh Takiguchi
Hello Roger. Thank you for your suggestion. src/jdk.charsets/share/classes/sun/nio/cs/ext/IBM33722.java: I think this is no longer needed since it only has imports. By the way, the style is to import each specific class and avoid wild card imports. "import sun.nio.cs.*;" is required be

Re: Optimized version of CopiesList.hashCode()

2018-12-03 Thread Stuart Marks
On 11/30/18 8:34 PM, Zheka Kozlov wrote: I think we should choose Tagir's version so you don't need my OCA. OK, thanks. Let me know if you need any assistance with the OCA, should you decide to proceed with it. s'marks

Re: RFR: XXXS: JDK-8214745: Bad link in coll-reference.html

2018-12-03 Thread Brian Burkhalter
+1 Brian > On Dec 3, 2018, at 4:22 PM, Jonathan Gibbons > wrote: > > --- a/src/java.base/share/classes/java/util/doc-files/coll-reference.html Mon > Dec 03 16:14:15 2018 -0800 > +++ b/src/java.base/share/classes/java/util/doc-files/coll-reference.html Mon > Dec 03 16:17:11 2018 -0800 > @@ -4

Re: RFR: XXXS: JDK-8214745: Bad link in coll-reference.html

2018-12-03 Thread Martin Buchholz
LGTM On Mon, Dec 3, 2018 at 4:27 PM Jonathan Gibbons wrote: > Please review a tiny patch, given below, to fix a broken link in > coll-reference.html. The problem is bad/misplaced punctuation characters > within the link. This is believed to be the last broken link in the > java.base module, and

Re: RFR: XXXS: JDK-8214745: Bad link in coll-reference.html

2018-12-03 Thread Lance Andersen
+1 > On Dec 3, 2018, at 7:22 PM, Jonathan Gibbons > wrote: > > Please review a tiny patch, given below, to fix a broken link in > coll-reference.html. The problem is bad/misplaced punctuation characters > within the link. This is believed to be the last broken link in the java.base > module,

RFR: XXXS: JDK-8214745: Bad link in coll-reference.html

2018-12-03 Thread Jonathan Gibbons
Please review a tiny patch, given below, to fix a broken link in coll-reference.html. The problem is bad/misplaced punctuation characters within the link. This is believed to be the last broken link in the java.base module, and one of the last in the overall documentation. JBS: https://bugs.op

Re: RFR: [XXS] 8214744: Unnecessary tags in java.util.zip.Deflater

2018-12-03 Thread Martin Buchholz
LGTM On Mon, Dec 3, 2018 at 4:07 PM Jonathan Gibbons wrote: > Please review this small patch, given below, to remove unnecessary > reported by `tidy` as the last remaining HTML issues in the > documentation for java.base > > build/linux-x86_64-server-release/images/docs/api/java.base/java/util/

Re: RFR: [XXS] 8214744: Unnecessary tags in java.util.zip.Deflater

2018-12-03 Thread Lance Andersen
+1 > On Dec 3, 2018, at 7:02 PM, Jonathan Gibbons > wrote: > > Please review this small patch, given below, to remove unnecessary > reported by `tidy` as the last remaining HTML issues in the documentation for > java.base > > build/linux-x86_64-server-release/images/docs/api/java.base/java/u

Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

2018-12-03 Thread Jiangli Zhou
Hi Claes, Looks good from object graph archiving perspective. Placing the KNOWN_NAMES map in the closed archive looks safe since no additional names are added after initialization. If you also agree, maybe add a comment above the KNOWN_NAMES field declaration with those information. Thanks!

Re: RFR: [XXS] 8214744: Unnecessary tags in java.util.zip.Deflater

2018-12-03 Thread Mandy Chung
+1 Mandy On 12/3/18 4:02 PM, Jonathan Gibbons wrote: Please review this small patch, given below, to remove unnecessary reported by `tidy` as the last remaining HTML issues in the documentation for java.base build/linux-x86_64-server-release/images/docs/api/java.base/java/util/zip/Deflater.

RFR: [XXS] 8214744: Unnecessary tags in java.util.zip.Deflater

2018-12-03 Thread Jonathan Gibbons
Please review this small patch, given below, to remove unnecessary reported by `tidy` as the last remaining HTML issues in the documentation for java.base build/linux-x86_64-server-release/images/docs/api/java.base/java/util/zip/Deflater.html:827:2: Warning: trimming empty build/linux-x86_64

Re: RFR 8171426 : java/lang/ProcessBuilder/Basic.java failed with Stream closed

2018-12-03 Thread Brian Burkhalter
Hi Roger, L2: copyright year, of course L2119: The verbiage seems a little obtuse. Otherwise, +1. Thanks, Brian On 11/27/2018 02:04 PM, Roger Riggs wrote: > Please review a test update to address an intermittent test failure. > The test is modified to accept the current implementation beha

Re: RFR: JDK-8066619: String(byte[],int,int,int) in String has been deprecated in Manifest and Attributes

2018-12-03 Thread Roger Riggs
Hi Phillip, The amount detail obscures the general purpose. And there appears to be more than 1. The Jira issue IDs mentioned are 8066619 and 6202130. Is this functionally neutral and only fixes the deprecations? There is a mention that a test is needed for multi-byte chars, but a test is not i

Re: RFR 8171426 : java/lang/ProcessBuilder/Basic.java failed with Stream closed

2018-12-03 Thread Roger Riggs
Ping? On 11/27/2018 02:04 PM, Roger Riggs wrote: Please review a test update to address an intermittent test failure. The test is modified to accept the current implementation behavior of the input streams from processes that may under race conditions with close throw IOE("Stream closed"). D

Re: RFR: 8212794 IBM-964 is required for AIX default charset

2018-12-03 Thread Roger Riggs
Hi Ichiroh, src/jdk.charsets/share/classes/sun/nio/cs/ext/IBM33722.java:     I think this is no longer needed since it only has imports.     By the way, the style is to import each specific class and avoid wild card imports. TestIBMBugs:   - Please use a style consistent with other methods in

Re: RFR: 8170769 Provide a simple hexdump facility for binary data

2018-12-03 Thread Roger Riggs
Hi Vincent, On 12/02/2018 05:54 PM, Vincent Ryan wrote: Thanks for the thorough review. Responses in-line. On 26 Nov 2018, at 19:49, Roger Riggs > wrote: Hi, Thanks for updating this proposal.  Some comments on the javadoc/spec. The class name Hex isn't very e

Re: RFR 8177552: Compact Number Formatting support

2018-12-03 Thread Roger Riggs
Hi Nishit, Thanks for the updates and cleanup. CompactNumberFormat.java: - 827: To locate a single character use:     if (pattern.indexOf(QUOTE) < 0) { ... } - 1488:  Since infinite returns do not depend on any of the code after line 1454,    the 1488- 1494 could be moved to 1454. (It is

Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-12-03 Thread Vicente Romero
Hi all, Can I have the final nod to the JVM constants API, there have been some changes since the last review iteration. Thanks to the internal and external developers that have taken the time to provide feedback so far. The links to the last versions are: webrev: http://cr.openjdk.java.net/

RE: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()

2018-12-03 Thread Andrew Luo
CopiesList is private anyways, so are you suggesting that someone might call nCopies(0) in a previous version of the JDK, serialize the return value, and then unserialize in a later version of the JRE? Is this even a supported use case (serialization/deserialization of JRE classes across versio

Re: jar --verify operation mode checking mrjar validity

2018-12-03 Thread Jaikiran Pai
On 02/12/18 3:43 PM, Alan Bateman wrote: > On 01/12/2018 08:45, Christian Stein wrote: >> Hi, >> >> jar --create (and --update) perform type API validation checks when >> used in >> combination with --release option. This detects invalid "version >> overlays" >> at package time, where the API doe

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

2018-12-03 Thread Tomasz Linkowski
Hi, My 2 cents: 1) I agree that `String.map` is a really unfortunate name - Peter explained it very well! I'd rather wait than have `String.map`. 2) I absolutely agree that `String`, `Stream`, and `Optional` should share the name. 3) I prefer `Stream.transform` to `Stream.chain` but I understan

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

2018-12-03 Thread Brian Burkhalter
HI Daniel, That does look messed up. Thanks for pointing it out. I will investigate. Thanks, Brian > On Dec 3, 2018, at 9:27 AM, Daniel Fuchs wrote: > > Looks good to me, though I don't understand the change > at line 154: > > The comment says: > > 152 // skip(n) returns negative va

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

2018-12-03 Thread Daniel Fuchs
typo in my previous mail: On 03/12/2018 17:27, Daniel Fuchs wrote: Hi Brian, Looks good to me, though I don't understand the change at line 214: ^^^ 154 sorry for the noise and confusion... -- daniel The comment says: 152 // skip(n) returns negative value: IOE

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

2018-12-03 Thread Daniel Fuchs
Hi Brian, Looks good to me, though I don't understand the change at line 214: The comment says: 152 // skip(n) returns negative value: IOE but if you pass -1 - you're no longer calling skip(n)? best regards, -- daniel On 30/11/2018 19:55, Brian Burkhalter wrote: Loathe though I am

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

2018-12-03 Thread Brian Burkhalter
ping ... > Begin forwarded message: > > From: Brian Burkhalter > Subject: Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes > Date: November 30, 2018 at 11:55:08 AM PST > To: Java Core Libs > > Loathe though I am to resurrect this thread, one problem arose after testing > with

Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

2018-12-03 Thread Claes Redestad
On 2018-12-03 17:06, Alan Bateman wrote: On 03/12/2018 16:02, Claes Redestad wrote: Hi, http://cr.openjdk.java.net/~redestad/8214712/jdk.00/ This looks okay to me but the changes remind me that there are several attributes name in KNOWN_NAMES are should probably be removed as they aren'

Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

2018-12-03 Thread Alan Bateman
On 03/12/2018 16:02, Claes Redestad wrote: Hi, initializing java.util.jar.Attributes.Name. executes ~20k bytecodes setting up and eagerly calculating case-insensitive hash codes for a slew of Name objects. By archiving the resulting set of Names and initializing public constants from the

RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

2018-12-03 Thread Claes Redestad
Hi, initializing java.util.jar.Attributes.Name. executes ~20k bytecodes setting up and eagerly calculating case-insensitive hash codes for a slew of Name objects. By archiving the resulting set of Names and initializing public constants from the archived map, we reduce time spent starting up

Re: RFR 8177552: Compact Number Formatting support

2018-12-03 Thread Nishit Jain
Thanks Roger, Updated the webrev based on thebelow suggested changes and some clean up. http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.05/ On 01-12-2018 01:03, Roger Riggs wrote: Hi Nishit, Some comments, a couple of possible bugs and several  performance related issues could be

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

2018-12-03 Thread Tagir Valeev
+1 to Stephen and Remi. I personally see no reason to hurry up with this API: unlike trimming/indenting methods this one doesn't look crucial for raw string literals. I don't see that this method blocks anything else. > For info, AWS SDK v2 has chosen applyMutation() for a similar concept: Unfort

Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()

2018-12-03 Thread Zheka Kozlov
> Would it make sense to use CopiesList only for n > 0 This can break serialization. When CopiesList(0, e) is serialized and deserialized back, it will be in an invalid state. > I believe it makes sense to override CopiesList.equals() Also: contains(), iterator(), listIterator() пн, 3 дек. 2018

Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()

2018-12-03 Thread Ivan Gerasimov
Also, I believe it makes sense to override CopiesList.equals() to at least optimize the case when the other list is a CopiesList too. With kind regards, Ivan On 12/2/18 11:23 PM, Ivan Gerasimov wrote: Thank you Tagir! I think your solution is quite clever, and the fix looks good. While we