June 2018 - core-libs-dev

RFR: 8205438: Re-enable shebang tests in test/jdk/tools/launchers/SourceMode.java
by Jonathan Gibbons 27 Jun '18

27 Jun '18

Please review a test fix to re-enable shebang tests in test/jdk/tools/launchers/SourceMode.java . The test cases for invoking the source launcher via the shebang mechanism have been disabled because they were adversely affected by the very long pathnames on our internal test infastructure, causing the test cases to overflow the system limit of 128 characters for the first line of a shebang file The fix is to run jlink to create a small image that can be addressed by a short relative path. Some of the test cases on macOS and Solaris remain explicitly disabled because of the inconsistencies in the system-level support for shebang execution. Finally, the use of hard-coded version numbers in the test is changed to avoid having to add this test to the list of tests that need to be modified at the beginning of each release. -- Jon JBS: https://bugs.openjdk.java.net/browse/JDK-8205438 Webrev: http://cr.openjdk.java.net/~jjg/8205438/webrev.00/

3 2

Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for reading/writing a string from/to a file
by Joe Wang 27 Jun '18

27 Jun '18

Hi all, It's been a while since the 1st round of reviews. Thank you all for the valuable comments and suggestions! Note that Roger's last response went to nio-dev, but not core-libs-dev, I've therefore attached it below. CSR [2]: the CSR is now approved. Note the write method has been changed to writeString. Impl [3]: for performance reason and the different requirement from byte-string conversion for malformed/unmappable character handing, the implementation uses a specific method separate from the existing ones. Both Sherman and Alan preferred specific method than adding parameters to the APIs that might be error prone. Thanks Sherman for the code! JMH benchmark tests showed the new read method performs better than an operation that reads the bytes and convert to string. The gains start to be significant even at quite reasonable sizes (< 10K). [1] JBS: https://bugs.openjdk.java.net/browse/JDK-8201276 [2] CSR: https://bugs.openjdk.java.net/browse/JDK-8202055 [3] webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8201276/webrev/ Please review. Thanks, Joe On 5/15/18, 7:51 AM, Roger Riggs wrote: > Hi Joe, et. al. > > My $.02 on line separators: > > - We should avoid clever tricks trying to solve problems that are > infrequent such as cross OS file line operations. > Most files will be read/written on a single system so the line > separators will be as expected. > > - Have/use APIs that split lines consistently accepting both line > separators so developer code can be agnostic to line separators. > aka BufferedReader.readLine for developers that are processing the > contents *as lines*. > Those other methods already exist; if there are any gaps in line > oriented processing that's a separate task. > > - These new file methods are defined to handle Charset > encoding/decoding and buffering. > Since there are other methods to deal with files as lines these > methods should not look for or break to lines. > > - Performance: adding code to look for line characters will slow it > down and in most cases would have no impact > since the line endings will match the system. > > - The strings passed to writeString (CharSequence) should have been > constructed using the proper line separators. > There are any number of methods that insert the os specific line > terminator and its easy to write File.separator as needed. > > As for the method naming: > > I'd prefer "writeString" as a familiar term that isn't stretched too > far by making the argument be a CharSequence. > its fine to call a CharSequence a string (with a lower case s). > > $.02, Roger > > > On 4/27/18 6:33 PM, Joe Wang wrote: >> >> >> On 4/27/2018 12:50 PM, forax(a)univ-mlv.fr wrote: >>> ----- Mail original ----- >>>> De: "Joe Wang" <huizhe.wang(a)oracle.com> >>>> À: "Remi Forax" <forax(a)univ-mlv.fr>, "Alan Bateman" >>>> <Alan.Bateman(a)oracle.com> >>>> Cc: "nio-dev" <nio-dev(a)openjdk.java.net>, "core-libs-dev" >>>> <core-libs-dev(a)openjdk.java.net> >>>> Envoyé: Vendredi 27 Avril 2018 21:21:01 >>>> Objet: Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for >>>> reading/writing a string from/to a file >>>> Hi Rémi, Alan, >>> Hi Joe, >>> >>>> I'm not sure we'd want to replace system dependent line separators >>>> with >>>> '\n'. There are cases as you described where the replacement makes >>>> sense. However, there are other cases too. For example, the >>>> purpose is >>>> to read, edit a text file and then write it back. If this is on >>>> Windows, >>>> and if line separators are replaced with '\n', that would cause the >>>> resulting file not formatted properly in for example Notepad. There >>>> are >>>> cases where users write text files on Windows using Java, and only >>>> found >>>> the lines were not separated in Notepad. >>> I agree that why the counterpart of readString() that write the >>> string should inserts the platform's line separator. >>> BTW, i just found that those methods are not named writeString, or >>> writeCharSequence, i think they should. >> >> While readString() does not modify the original content (e.g. by >> replacing the platform's line separator with '\n'), write(String) >> won't either, by adding extra characters such as the line separator. >> >> I would think interfaces shall read along with the parameters. >> readString(Path) == read as a String from the specified Path (one >> could argue for readToString, readAsString, but we generally omit the >> preps) >> write(Path, CharSequence) == write the CharSequence to the file, >> since CharSequence is already in the method signature as a parameter, >> we probably don't want to add that to the name, otherwise it would >> read like repeating the word CharSequence. >> >> It is in a similar situation as write(Path, Iterable) where it was >> defined as writeLines(Path, Iterable). >> >>> >>>> Files.write(Path, Iterable) is also specified to terminate each line >>>> with the platform's line separator. If readString does the >>>> replacement, >>>> it would be inconsistent. >>> >>> Anyway, if we look for consistency the methods writeCharSequence >>> should transform the \n in the CharSequence to the platform's line >>> separator. >>> >>> Files.write(Path, Iterable) is is not a counterpart of readString(), >>> it's consistent with Files.lines() or Files.readLines() (or >>> BufferedReader.readLine()) that all suppress the line separators. >>> Anyway, Files.write(path, readString(path)::line) will be consistent >>> if you replace the line separators or not because String.line() >>> suppresses the line separators. >> >> readString pairs with write(String), therefore it's more like >> Files.write(path, readString(path)) than readString(path)::line. The >> use case: >> >> String s = Files.read(path); >> Files.write(path, s.replace("/config/location", "/new/location")); >> >> would then work as expected. >> >> These two methods are one-off (open/read/write/close file) operation. >> write(String) therefore is not intended for adding/appending multiple >> Strings from other operations such as String.line(). If an app needs >> to put the result of String.line() or any other processes into a file >> using this method, it needs to take care of adding the line separator >> itself when necessary. "when necessary" would be a judgement call by >> the developer. >> >> That said, if there's a consensus on the idea of terminating the >> string with a line separator, then readString method would need to >> strip it off to go with the write method. >> >>> >>>> I would think readString behaves like readAllBytes, that would >>>> simply read all content faithfully. >>> readString does an interpolation due to the Charset so it's not the >>> real content, again, the idea is that developers should not have to >>> care about the platform's line separators (they have more important >>> things to think). >>> >>> The other solution is to just not introduce those new methods, after >>> all Files.lines().collect(Collectors.joining("\n")) already does the >>> job, no ? >> >> While there are many ways to do it, none is as straight-forward. >> "Read (entire) file to String"/"Write String to file" are popular >> requests from users. Read to string -> do some String manipulation -> >> write it back is such a simple use case, it really needs to be very >> easy to do as illustrated in the above code snippet. >> >> Cheers, >> Joe >> >>> >>>> Best, >>>> Joe >>> regards, >>> Rémi >>> >>>> On 4/27/2018 4:43 AM, forax(a)univ-mlv.fr wrote: >>>>> Hi Alan, >>>>> >>>>> People do not read the documentation. >>>>> So adding something in the documentation about when a method >>>>> should be used or >>>>> not is never a solution. >>>>> >>>>> Here the user want a String and provides a charset so you have no >>>>> way but to >>>>> decode the content to substitute the line separator. >>>>> >>>>> cheers, >>>>> Rémi >>>>> >>>>> ----- Mail original ----- >>>>>> De: "Alan Bateman" <Alan.Bateman(a)oracle.com> >>>>>> À: "Remi Forax" <forax(a)univ-mlv.fr>, "Joe Wang" >>>>>> <huizhe.wang(a)oracle.com> >>>>>> Cc: "nio-dev" <nio-dev(a)openjdk.java.net>, "core-libs-dev" >>>>>> <core-libs-dev(a)openjdk.java.net> >>>>>> Envoyé: Vendredi 27 Avril 2018 13:34:12 >>>>>> Objet: Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for >>>>>> reading/writing a string from/to a file >>>>>> On 27/04/2018 12:29, Remi Forax wrote: >>>>>>> I think that having a readString that includes OS dependent line >>>>>>> separators is a >>>>>>> mistake, >>>>>>> Java does a great job to try to shield the developer from the >>>>>>> kind of things >>>>>>> that makes programs behave differently on different platforms. >>>>>>> >>>>>>> readString should subtitute (\r)?\n to \n otherwise either >>>>>>> people will do a call >>>>>>> replace() which is less efficient or will learn the lesson the >>>>>>> hard way. >>>>>>> >>>>>>> raw string literal does the same substitution for the same reason. >>>>>>> >>>>>> Yes, there are several discussion points around this and somewhat >>>>>> timely >>>>>> with multi-string support. >>>>>> >>>>>> One thing that I think Joe will need in this API is some note to >>>>>> make it >>>>>> clearer what the intended usage is (as I think the intention is >>>>>> simple >>>>>> cases with mostly single lines of text). >>>>>> >>>>>> -Alan. >> >

5 19

RFR JDK-8200243: System error message is decoded as invalid encoding in Windows.
by Xueming Shen 27 Jun '18

27 Jun '18

Hi, Please help review the change for JDK-8200243 issue: https://bugs.openjdk.java.net/browse/JDK-8200243 webrev: http://cr.openjdk.java.net/~sherman/8200243/webrev This is a regression. The root cause is one of the change we put in jdk9 for JDK-8057777 [1], which is to remove those unused vm interfaces. The webrev [1] indicates that the changeset added a platform dependent version of GetLastErrorString(), which uses utf8, to replace the original jvm version of JVM_GetLastErrorString() (in jni_util.c). However the encoding of the char* interface between jvm and jdk is always interpreted/specified as "native encoding". So if the platform's default encoding is NOT utf-8, it never uses utf8. This is what we have on Windows platform. The fix is to use/copy-paste the "original" vm implementation in hotspot repo for JVM_GetLastErrorString() (from os_windows.cpp). The fix has been verified on a Chinese version of windows. Thanks Felix! Thanks, Sherman [1] https://bugs.openjdk.java.net/browse/JDK-8057777

2 2

RFR JDK-8066709 Make some JDK system properties read only
by Roger Riggs 27 Jun '18

27 Jun '18

Please review a change to make the values of java.home, user.home, user.dir, and user.name effectively read-only for internal use. The values are cached during initialization and the cached values are used. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-static-property-8066709/ Issue: https://bugs.openjdk.java.net/browse/JDK-8066709 CSR: https://bugs.openjdk.java.net/browse/JDK-8204235 Thanks, Roger

12 36

RFR(s): 8203670: unmodifiable List iterator() implementations should not be ListIterators
by Stuart Marks 26 Jun '18

26 Jun '18

Hi all, Please review this small changeset that ensures that calling iterator() on an unmodifiable List (from List.of, et. al.) returns an instance that cannot be downcast to ListIterator and used as such. Bug: https://bugs.openjdk.java.net/browse/JDK-8203670 Webrev: http://cr.openjdk.java.net/~smarks/reviews/8203670/webrev.0/ Thanks, s'marks

5 7

Japanese New Era supporting does not accept Heisei 32
by kishida naoki 26 Jun '18

26 Jun '18

I'm trying to use the Japanese new era supporting and have found that it does not accept Heisei 32. There are many document that include Heisei 32 or later at present. For example, the expiration date of my driver license is Heisei 32 March. JDK10 can accept Heisei 32. I think the change of the behavior make some trouble. Additionally, Japanese government says some system use Heisei after 5/1/2019 for a while. The JDK version that include the new era support can not be used on such system. https://digital.asahi.com/articles/DA3S13491490.html There are 3 ways to support the new era. 1. H32 is not accepted and S90 is not accepted. (current new era support behavior) 2. H32 is accepted and S90 is not accepted. (current JDK behavior) 3. H32 is accepted and S90 is accepted. (to conformity) I prefer 2 not to change the current JDK behavior. With taking 2 or 3, we might have a new API like JapaneseDate.from(JapaneseEra, TemporalAccessor) to specify an era on making JapaneseDate. -- Naoki Kishdia

2 2

Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 Instructions
by Vladimir Kozlov 26 Jun '18

26 Jun '18

Hi Smita, I CCing to Libs to review code changes in Base64.java. Looks like you changed all need place to implement intrinsic. One question so: why you have own copy of base64 charsets and not using one in library: private int encode0(byte[] src, int off, int end, byte[] dst) { char[] base64 = isURL ? toBase64URL : toBase64; Some indents in new and old Assembler::emit_operand() are off. In new Assembler::emit_operand() is better use } else { instead of 'return' in one branch. This is first time I see that XMM register can be used for index in address. Is it true? Can you point to Intel's document which describes it. What testing you did? Please, add tests to test/hotspot/jtreg/compiler/intrinsics/base64 (may be similar to sha or AES in compiler/codegen/aes) to make sure that all flags, intrinsic is used and it produces correct result. I know there is test/jdk/java/util/Base64/ tests but they may not trigger intrinsic usage. But you can use them as starting point for new tests. Thanks, Vladimir On 6/22/18 11:47 AM, Kamath, Smita wrote: > Hi Vladimir, > > I’d like to contribute an optimization for Base64 Encoding Algorithm > using AVX512 Instructions. This optimization shows 1.5x improvement on > x86_64 platform(SKL). > > Link to Bug: https://bugs.openjdk.java.net/browse/JDK-8205528 > > Link to webrev: http://cr.openjdk.java.net/~vdeshpande/Base64/webrev.00/ > > For testing the implementation, I have run tests under > test/jdk/java/util/Base64/ and also ran > test/jdk/com/sun/jndi/ldap/Base64Test.java > > I have also run jtreg. > > Please review and let me know if you have any comments. > > Thanks and Regards, > > Smita >

3 5

RFR: 8205184: Delegating Iterator implementations that don't delegate forEachRemaining()
by Martin Buchholz 26 Jun '18

26 Jun '18

8205184: Delegating Iterator implementations that don't delegate forEachRemaining() http://cr.openjdk.java.net/~martin/webrevs/jdk/delegate-forEachRemaining/ https://bugs.openjdk.java.net/browse/JDK-8205184

2 3

Proposal: optimization of Map.copyOf and Collectors.toUnmodifiableMap
by Peter Levart 26 Jun '18

26 Jun '18

Hi, Those two methods were added in JDK 10 and they are not very optimal. Map.copyOf(Map map) 1st dumps the source map into an array of Map.Entry(s) (map.entrySet().toArray(new Entry[0])), which typically creates new Map.Entry objects, then passes the array to Map.ofEntries(Map.Entry[] entries) factory method that iterates the array and constructs a key/value interleaved array from it which is passed to ImmutableCollections.MapN constructor, which then constructs a linear-probing hash table from it. So each key and value is copied 3 times, while several temporary objects are created in the process. One copying step could be eliminated and construction of temporary Map.Entry objects too: http://cr.openjdk.java.net/~plevart/jdk-dev/UnmodifiableMap_copyOf/webrev.0… Collecting stream(s) using Collectors.toUnmodifiableMap() 1st collects key/value pairs into a HashMap, then it performs equivalent operations as Map.copyOf(hashMap) at the end. Using Map.copyOf() directly benefits this collection operation too. The following benchmark: http://cr.openjdk.java.net/~plevart/jdk-dev/UnmodifiableMap_copyOf/Unmodifi… Shows up to 30% improvement of .copyOf operation with this patch applied: Original: Benchmark (size) Mode Cnt Score Error Units UnmodifiableMapBench.copyOf 10 avgt 10 403.633 ± 2.640 ns/op UnmodifiableMapBench.copyOf 100 avgt 10 3489.623 ± 44.590 ns/op UnmodifiableMapBench.copyOf 1000 avgt 10 40030.572 ± 277.075 ns/op UnmodifiableMapBench.toUnmodifiableMap 10 avgt 10 831.221 ± 3.816 ns/op UnmodifiableMapBench.toUnmodifiableMap 100 avgt 10 9783.519 ± 43.097 ns/op UnmodifiableMapBench.toUnmodifiableMap 1000 avgt 10 96524.536 ± 670.818 ns/op Patched: Benchmark (size) Mode Cnt Score Error Units UnmodifiableMapBench.copyOf 10 avgt 10 264.172 ± 1.882 ns/op UnmodifiableMapBench.copyOf 100 avgt 10 2318.974 ± 15.877 ns/op UnmodifiableMapBench.copyOf 1000 avgt 10 29291.782 ± 3139.737 ns/op UnmodifiableMapBench.toUnmodifiableMap 10 avgt 10 771.221 ± 65.432 ns/op UnmodifiableMapBench.toUnmodifiableMap 100 avgt 10 9275.016 ± 725.722 ns/op UnmodifiableMapBench.toUnmodifiableMap 1000 avgt 10 82204.342 ± 851.741 ns/op Production of garbage is also reduced, since no Map.Entry temporary objects are constructed: Original: Benchmark (size) Mode Cnt Score Error Units UnmodifiableMapBench.copyOf:·gc.alloc.rate.norm 10 avgt 10 416.001 ± 0.002 B/op UnmodifiableMapBench.copyOf:·gc.alloc.rate.norm 100 avgt 10 2936.005 ± 0.019 B/op UnmodifiableMapBench.copyOf:·gc.alloc.rate.norm 1000 avgt 10 28136.059 ± 0.199 B/op UnmodifiableMapBench.toUnmodifiableMap:·gc.alloc.rate.norm 10 avgt 10 1368.001 ± 0.004 B/op UnmodifiableMapBench.toUnmodifiableMap:·gc.alloc.rate.norm 100 avgt 10 10208.139 ± 0.045 B/op UnmodifiableMapBench.toUnmodifiableMap:·gc.alloc.rate.norm 1000 avgt 10 93025.923 ± 0.573 B/op Patched: Benchmark (size) Mode Cnt Score Error Units UnmodifiableMapBench.copyOf:·gc.alloc.rate.norm 10 avgt 10 304.000 ± 0.001 B/op UnmodifiableMapBench.copyOf:·gc.alloc.rate.norm 100 avgt 10 2464.004 ± 0.012 B/op UnmodifiableMapBench.copyOf:·gc.alloc.rate.norm 1000 avgt 10 24064.040 ± 0.137 B/op UnmodifiableMapBench.toUnmodifiableMap:·gc.alloc.rate.norm 10 avgt 10 1256.001 ± 0.003 B/op UnmodifiableMapBench.toUnmodifiableMap:·gc.alloc.rate.norm 100 avgt 10 9720.153 ± 0.055 B/op UnmodifiableMapBench.toUnmodifiableMap:·gc.alloc.rate.norm 1000 avgt 10 88905.688 ± 0.574 B/op So what do you think? Is this an improvement? Regards, Peter

3 8

RFR(xs): JDK-8201610: fix broken link in UnicastRemoteObject
by Stuart Marks 25 Jun '18

25 Jun '18

Hi all, Please review this small fix for a broken link. Patch appended below. Bug: https://bugs.openjdk.java.net/browse/JDK-8201610 Thanks, s'marks diff -r 8b98dcf37891 -r 3da3bcc7b28b src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java --- a/src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java Mon Jun 25 16:21:14 2018 -0700 +++ b/src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java Mon Jun 25 16:43:57 2018 -0700 @@ -126,7 +126,7 @@ * <ul> * * <li>The proxy's class is defined according to the specifications for the - * <a href="{(a)docRoot}/java/lang/reflect/Proxy.html#membership"> + * <a href="{(a)docRoot}/java.base/java/lang/reflect/Proxy.html#membership"> * {@code Proxy} * </a> * class, using the class loader of the remote object's class.

3 2