April 2020 - core-libs-dev

Unexpected behaviour with larger Strings
by Raffaello Giulietti 20 Apr '20

20 Apr '20

Hi, I'm on Linux, but the explanation might be the same as the following one. An easier way to obtain the same error on OpenJDK8 + HotSpot is to execute byte[] b = new byte[Integer.MAX_VALUE]; which is exactly what happens behind the scenes in the UTF-8 case. The encoder pessimistically assumes that each char will be encoded to at most 3 bytes. The expansion factor 3, however, is expressed as the float 3.0f. This, in turn, is first converted to the double 3.0, multiplied by your length 1 << 30 and cast to int. As the product overflows the int range, the cast produces Integer.MAX_VALUE. While Integer.MAX_VALUE should be considered a legal array size, I recall to have read somewhere that implementations are allowed to be a little bit more restrictive. Experimentally, the maximum size for a byte[] on OpenJDK8 + HotSpot / Linux is Integer.MAX_VALUE - 2. I guess it is the same on macOS. Hope this helps. Greetings Raffaello

1 0

RFR: JDK-8243168 Remove addition preview adornment from String::stripIndent and String::translateEscapes
by Jim Laskey 20 Apr '20

20 Apr '20

Cleaning up the JavaDoc. webrev: http://cr.openjdk.java.net/~jlaskey/8243168/webrev-00 <http://cr.openjdk.java.net/~jlaskey/8243168/webrev-00> jbs: https://bugs.openjdk.java.net/browse/JDK-8243168 <https://bugs.openjdk.java.net/browse/JDK-8243168> csr: https://bugs.openjdk.java.net/browse/JDK-8243169 <https://bugs.openjdk.java.net/browse/JDK-8243169>

2 1

RFR: JDK-8241742 - Remove the preview status for methods introduced for Text Blocks
by Jim Laskey 20 Apr '20

20 Apr '20

Please take the time to review the code changes to remove preview status for String::stripIndent, String::translateEscapes and String::formatted. The changes are pretty light-weight and shouldn't take too long to review. Mostly involves removing "preview" trappings. Cheers, -- Jim webrev: http://cr.openjdk.java.net/~jlaskey/8241742/webrev-00 <http://cr.openjdk.java.net/~jlaskey/8241742/webrev-00> JBS: https://bugs.openjdk.java.net/browse/JDK-8241742 <https://bugs.openjdk.java.net/browse/JDK-8241742> CSR: https://bugs.openjdk.java.net/browse/JDK-8241747 <https://bugs.openjdk.java.net/browse/JDK-8241747> JEP: https://bugs.openjdk.java.net/browse/JDK-8236934 <https://bugs.openjdk.java.net/browse/JDK-8236934>

5 6

RFR: JDK-8243156 Fix deprecation and unchecked warnings in microbenchmark
by Magnus Ihse Bursie 20 Apr '20

20 Apr '20

This patch addresses the warnings deprecation and unchecked, which are impossible to fully suppress. With this patch, the test-image can be built fully without warnings. The patch updates non-critical code to use modern solutions for deprecated methods, where possible. Tested methods have been annotated with SuppressWarnings instead. I'm leaving it for a future update by the perf team to reconsider if it's worth testing deprecated methods. Bug: https://bugs.openjdk.java.net/browse/JDK-8243156 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8243156-fix-warnings-in-microbenchmark… /Magnus

3 2

17.7x speedup of Double.toString(double)
by Raffaello Giulietti 20 Apr '20

20 Apr '20

Hello, curious about Double.toString(double) performance? Here are the results obtained by jmh 1.23 on a recent changeset [1]. The benchmark is over the full range of bitwise uniformly distributed random doubles. # VM options: <none> # Warmup: 5 iterations, 10 s each # Measurement: 5 iterations, 10 s each ... Benchmark Mode Cnt Score Error Units DoubleToStringBenchmark.test avgt 25 1251.658 ± 211.068 ns/op showing quite a variability, probably attributable to intervening gc cycles. The same setup after the integration of the code in [2] to replace Double.toString(double) with a modern, better specified and correct implementation (as witnessed by many trillions of fully conformant outcomes). # VM options: <none> # Warmup: 5 iterations, 10 s each # Measurement: 5 iterations, 10 s each ... Benchmark Mode Cnt Score Error Units DoubleToStringBenchmark.test avgt 25 70.712 ± 1.461 ns/op showing both a speedup of 17.7x on my i7 based laptop of 2013 as well as predictable running times. The algorithm never generates temporary garbage. I'm still looking for enthusiastic reviewers: they seem hard to find, despite monthly reminders... More in [3] and similarly titled posts for the past 12 months. Greetings Raffaello ---- [1] https://hg.openjdk.java.net/jdk/jdk/rev/90882ba9f488 [2] https://cr.openjdk.java.net/~bpb/4511638/webrev.04/ [3] https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-March/065476.html

1 0

Type-parameterized complement to Object.equals(Object)
by dmytro sheyko 19 Apr '20

19 Apr '20

Hello, I need a sponsor for following JDK change. Summary ------- Provide means for type safe comparison for equality by introducing type-parameterized interface `java.lang.Equable<T>` with method `boolean equ(T)`. (The actual names of the interface and the method is subject to discussion.) Goals ----- The primary goal is to avoid programmer mistakes when values of different and incomparable types are compared using `equals` method, which always returns `false` in such cases according to its contract. Non-Goals --------- Custom hashers and equators for Collection Framework are out of the scope of this proposal. Motivation ---------- This is quite common approach to use `equals` method to compare value for equality. However, it may happen that the values have different incomparable types and, therefore, the result of such a comparison is doomed to `false`. For example, instances of `java.lang.Integer` and `java.lang.Long` will never be equal even though they represent the same numerical value. I.e. this code Long l = 42L; Integer i = 42; if (l.equals(i)) { // always false ... } else { ... } is very likely a mistake because the else-branch will never be taken. The same applies to attempts to compare among themselves instances of `java.lang.Short`, `java.lang.String`, `java.util.Date`, `java.util.List`, `java.util.Set`, `java.util.Map` etc. The reason why such erroneous comparisons remain unnoticed is the fact that the `equals` method accepts any value. One way to make mistakes with such comparisons more visible it to use `compareTo` method. Long l = 42L; Integer i = 42; if (l.compareTo(i) == 0) { // compilation error ... } else { ... } However this approach has some drawbacks: * It's counter-intuitive and unnecessarily verbose. It's not clear at glance why `compareTo` is used instead of `equals`. * It can be inefficient because there are cases when an attempt to figure out whether given value is less or greater that the other value requires more effort than an attempt to figure out whether the values are just not equal. For example, when we compare strings and have noticed that the strings have different length we can stop here and do not scan them in order to figure out, which one is greater than the other. Another example is the class that represents rational number as a reduced fraction. To make sure that the rational numbers are equal, we can just compare their numerators and denominators. But in order to find out which one is greater than the other we can't avoid arithmetic operations (multiplications). * It's not applicable to the classes that do not implement `java.lang.Comparable<T>`. For example, collections, complex numbers, points in 2D or 3D space etc. Therefore it's proposed to introduce interface `Equable` in order to explicitly mark comparable classes. public interface Equable<T> { boolean equ(T o); } Let `Comparable` extend it and provide default method for `equ`. public interface Comparable<> extends Equable<T> { int compareTo(T o); default boolean equ(T o) { return compareTo(o) == 0; } } Provide efficient implementation of `equ` method for some JDK classes, at least those that are in `java.lang` package. Also it would be desiable to let `java.util.List`, `java.util.Set`, `java.util.Map` and `java.util.Map.Entry` to extend `Equable` interface as well (but without extending `Comparable`). === I am sending a set of patches. I split the whole change into several logical parts. * equable-0.patch contain `java.lang.Equable`, `java.lang.Comparable`, `java.util.Objects.equ` * equable-1.patch contain `j.l.Boolean`, `j.l.Byte`, `j.l.Character`, `j.l.Double`, `j.l.Enum`, `j.l.Float`, `j.l.Integer`, `j.l.Long`, `j.l.Short` and `j.l.String` * equable-2.patch contain `j.u.Optional`, `j.u.OptionalDouble`, `j.u.OptionalInt`, `j.u.OptionalLong` * equable-3.patch contain `j.u.List`, `j.u.AbstractList`, `j.u.Set`, `j.u.AbstractSet`, `j.u.Map`, `j.u.AbstractMap` * equable-4.patch is a tricky one. It contains `com.sun.tools.jdi.EventSetImpl`. Previously this class extended `java.lang.ArrayList` and implemented `java.lang.Set` through `EventSet`. Therefore it appeared that it implemented both `java.util.List` and `java.util.Set` at the same time. This is not right, because these interfaces have different contracts regarding equality. So I had to change it by extending `java.lang.AbstractSet`. Thank you, Dmytro

2 2

JarFile.getVersionedEntry scalability with new release cadence
by Eirik Bjørsnøs 19 Apr '20

19 Apr '20

I recently needed to re-implement multi-release lookup logic for a ModuleReader capable of reading modules from unpacked (exploded) jar files [1] It occurred to me that JarFile.getVersionedEntry checks _every_ version between 8 and the runtime version when looking up paths. Since META-INF/versions will probably be sparsely populated, I'm wondering if something could be done to avoid checking 20 different paths in OpenJDK 28. Perhaps scanning META-INF/versions once when opening the file could work, then only check existing versions in getVersionedEntry? Maybe a premature optimization today, but with the new release cadence, this problem is going to surface at some point in the future, right? [1] https://mail.openjdk.java.net/pipermail/jigsaw-dev/2020-April/014414.html Eirik.

4 15

Review Request: 8238358: Implementation of JEP 371: Hidden Classes
by Mandy Chung 18 Apr '20

18 Apr '20

Please review the implementation of JEP 371: Hidden Classes. The main changes are in core-libs and hotspot runtime area. Small changes are made in javac, VM compiler (intrinsification of Class::isHiddenClass), JFR, JDI, and jcmd. CSR [1]has been reviewed and is in the finalized state (see specdiff and javadoc below for reference). Webrev: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03 Hidden class is created via `Lookup::defineHiddenClass`. From JVM's point of view, a hidden class is a normal class except the following: - A hidden class has no initiating class loader and is not registered in any dictionary. - A hidden class has a name containing an illegal character `Class::getName` returns `p.Foo/0x1234` whereas `GetClassSignature` returns "Lp/Foo.0x1234;". - A hidden class is not modifiable, i.e. cannot be redefined or retransformed. JVM TI IsModifableClass returns false on a hidden. - Final fields in a hidden class is "final". The value of final fields cannot be overriden via reflection. setAccessible(true) can still be called on reflected objects representing final fields in a hidden class and its access check will be suppressed but only have read-access (i.e. can do Field::getXXX but not setXXX). Brief summary of this patch: 1. A new Lookup::defineHiddenClass method is the API to create a hidden class. 2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG option that can be specified when creating a hidden class. 3. A new Class::isHiddenClass method tests if a class is a hidden class. 4. Field::setXXX method will throw IAE on a final field of a hidden class regardless of the value of the accessible flag. 5. JVM_LookupDefineClass is the new JVM entry point for Lookup::defineClass and defineHiddenClass to create a class from the given bytes. 6. ClassLoaderData implementation is not changed. There is one primary CLD that holds the classes strongly referenced by its defining loader. There can be zero or more additional CLDs - one per weak class. 7. Nest host determination is updated per revised JVMS 5.4.4. Access control check no longer throws LinkageError but instead it will throw IAE with a clear message if a class fails to resolve/validate the nest host declared in NestHost/NestMembers attribute. 8. JFR, jcmd, JDI are updated to support hidden classes. 9. update javac LambdaToMethod as lambda proxy starts using nestmates and generate a bridge method to desuger a method reference to a protected method in its supertype in a different package This patch also updates StringConcatFactory, LambdaMetaFactory, and LambdaForms to use hidden classes. The webrev includes changes in nashorn to hidden class and I will update the webrev if JEP 372 removes it any time soon. We uncovered a bug in Lookup::defineClass spec throws LinkageError and intends to have the newly created class linked. However, the implementation in 14 does not link the class. A separate CSR [2] proposes to update the implementation to match the spec. This patch fixes the implementation. The spec update on JVM TI, JDI and Instrumentation will be done as a separate RFE [3]. This patch includes new tests for JVM TI and java.instrument that validates how the existing APIs work for hidden classes. javadoc/specdiff http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/api/ http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff/ JVMS 5.4.4 change: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/Draft-JV… CSR: https://bugs.openjdk.java.net/browse/JDK-8238359 Thanks Mandy [1] https://bugs.openjdk.java.net/browse/JDK-8238359 [2] https://bugs.openjdk.java.net/browse/JDK-8240338 [3] https://bugs.openjdk.java.net/browse/JDK-8230502

14 52

RFR: 8242959: Optimize ZipFile.getEntry by folding lookups for name and name+'/'
by Claes Redestad 17 Apr '20

17 Apr '20

Hi, please review this patch to get rid of the back-to-back lookup of name and name+'/' in ZipFile.getEntry. This is done by first adjusting the hash function so that a trailing slash is not included in an entry's hash code, and adjusting appropriately when matching the arrays. This means we do fewer table lookups, less arithmetic and potentially less allocation on lookup misses. Patch contributed by Eirik Bjørsnøs. Webrev: http://cr.openjdk.java.net/~redestad/8242959/open.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8242959 Testing: tier1+2, verified neutral performance for hits and a significant reduction in cost of misses (~30-50ms improvement on Spring PetClinic startup). Thanks! /Claes

2 2

RFR 8242859: test/jdk/tools/jlink/JLinkTest.java uses nashorn module
by sundararajan.athijegannathan＠oracle.com 17 Apr '20

17 Apr '20

Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8242859 Webrev: http://cr.openjdk.java.net/~sundar/8242859/webrev.00/ Relevant JLinkTest.java section deleted as part of nashorn removal is as follows: http://cr.openjdk.java.net/~sundar/8241749/webrev.00/test/jdk/tools/jlink/J… It was using nashorn module as an example. Using jdk.jshell with the current change. Thanks -Sundar

2 1