November 2021 - core-libs-dev

A guide to PR3402 ("[Double,Float].toString()") code reviewers
by Raffaello Giulietti 16 Nov '21

16 Nov '21

Hello, as mentioned in [1], the Schubfach writing accompanying JBS issues [2] and [3] has recently undergone a thorough review by renowned researchers (in addition to Dmitry Nadezhin back in 2018-19 and myself on a continuous basis) [4]. For the *code* reviewers of PR [5] this means that there's no need for them to deal with the more mathematical aspects of the writing. They can instead concentrate in crosschecking that the Java code is in sync with the pseudo-code in the writing, as well as about its readability. To facilitate this review, there are comments scattered in the code that refer back to the writing for easy reference. # MathUtils For the flog*() methods in MathUtils refer to section 9.1.1. The large lookup table in MathUtils is defined in result 24 as well as in javadoc. MathUtilsChecker contains fully commented code that tests the correctness of the table as well as the flog*() methods to an extent sufficient for the conversions of doubles. (The table has also been checked independently by one of the article reviewers.) # DoubleToDecimal With regard to DoubleToDecimal, the fast path described in 8.3 is coded as part of toDecimal(double). In DoubleToDecimal, toDecimal(int,long,int) is a Java translation of the Schubfach skeleton in figure 7 combined with the suggestions in figure 9. Figure 8 is embodied in rop(long,long,long). §10 about digits extraction is implemented in toChars(long,int) and its dependencies. The rest of the code is just glue and should be easy to understand. # FloatToDecimal The code of FloatToDecimal is almost identical but adapted to the 32 bit case. Method rop(long,long) is from figure 11. # Test classes I hope it turns out that the test classes are either commented well enough to guide the reader or are kind of straightforward to understand. There are some package private constants in the code that are not referenced anywhere else in src/ and could thus be removed. However, since there's no great harm retaining them and since they might turn out to be useful in later work, I would suggest not to get rid of them. Of course, I'm at the disposal of this community for any question related to the PR and the writing. HTH Raffaello ---- [1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/083536.… [2] https://bugs.openjdk.java.net/browse/JDK-4511638 [3] https://bugs.openjdk.java.net/browse/JDK-8202555 [4] https://drive.google.com/file/d/1IEeATSVnEE6TkrHlCYNY2GjaraBjOT4f [5] https://github.com/openjdk/jdk/pull/3402

1 0

RFR: JDK-8277175 : Add a parallel multiply method to BigInteger
by kabutz 16 Nov '21

16 Nov '21

BigInteger currently uses three different algorithms for multiply. The simple quadratic algorithm, then the slightly better Karatsuba if we exceed a bit count and then Toom Cook 3 once we go into the several thousands of bits. Since Toom Cook 3 is a recursive algorithm, it is trivial to parallelize it. I have demonstrated this several times in conference talks. In order to be consistent with other classes such as Arrays and Collection, I have added a parallelMultiply() method. Internally we have added a parameter to the private multiply method to indicate whether the calculation should be done in parallel. The performance improvements are as should be expected. Fibonacci of 100 million (using a single-threaded Dijkstra's sum of squares version) completes in 9.2 seconds with the parallelMultiply() vs 25.3 seconds with the sequential multiply() method. This is on my 1-8-2 laptop. The final multiplications are with very large numbers, which then benefit from the parallelization of Toom-Cook 3. Fibonacci 100 million is a 347084 bit number. We have also parallelized the private square() method. Internally, the square() method defaults to be sequential. Benchmark (n) Mode Cnt Score Error Units BigIntegerParallelMultiply.multiply 1000000 ss 4 68,043 ± 25,317 ms/op BigIntegerParallelMultiply.multiply 10000000 ss 4 1073,095 ± 125,296 ms/op BigIntegerParallelMultiply.multiply 100000000 ss 4 25317,535 ± 5806,205 ms/op BigIntegerParallelMultiply.parallelMultiply 1000000 ss 4 56,552 ± 22,368 ms/op BigIntegerParallelMultiply.parallelMultiply 10000000 ss 4 536,193 ± 37,393 ms/op BigIntegerParallelMultiply.parallelMultiply 100000000 ss 4 9274,657 ± 826,197 ms/op ------------- Commit messages: - Update comments - Added parallelMultiply() method to BigInteger to allow large multiplications to run in parallel - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'upstream/master' - Merge remote-tracking branch 'origin/master' - 8176501: Method Shape.getBounds2D() incorrectly includes Bezier control points in bounding box - 8176501: Method Shape.getBounds2D() incorrectly includes Bezier control points in bounding box Changes: https://git.openjdk.java.net/jdk/pull/6391/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6391&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277175 Stats: 731 lines in 8 files changed: 565 ins; 136 del; 30 mod Patch: https://git.openjdk.java.net/jdk/pull/6391.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6391/head:pull/6391 PR: https://git.openjdk.java.net/jdk/pull/6391

2 10

JDK-8277175: Suggestion for improving BigInteger by adding a parallelMultiply() method
by Dr Heinz M. Kabutz 16 Nov '21

16 Nov '21

Good afternoon, BigInteger was changed in Java 8 to have faster multiply algorithms for large numbers (in the thousands of bits). Since the algorithm uses recursive decomposition (Toom Cook 3) it is fairly easy to parallelize. I would like to propose that we add a parallelMultiply() method to BigInteger. The speedup for very large numbers is impressive, obviously depending on hardware. I have created a PR https://github.com/openjdk/jdk/pull/6391 and submitted a request for enhancement http://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8277175 I would be very grateful for your ideas about this idea. Regards Heinz -- Dr Heinz M. Kabutz (PhD CompSci) Author of "The Java™ Specialists' Newsletter" - www.javaspecialists.eu Java Champion - www.javachampions.org JavaOne Rock Star Speaker Tel: +30 69 75 595 262 Skype: kabutz

1 0

RFR: 8227020: Unclosed input streams in URL Class Loader for JARs with META-INF/INDEX.LIST
by Jaikiran Pai 16 Nov '21

16 Nov '21

Can I please get a review for this change which proposes to fix the issue noted in https://bugs.openjdk.java.net/browse/JDK-8227020? The linked issue talks about non-normalized paths contributing to the leak. However, the root cause is applicable for regular normalized paths too. The `URLClassLoader` is backed by an instance of `jdk.internal.loader.URLClassPath`. This `URLClassPath` keeps track of a `loader` for each of the paths that are either provided directly as the `URL[]` or are inferred through the `Class-Path` attribute of each loaded jar. This it does in 2 different fields. One is a mapping between the String representation of the URL for which the loader was created. This field is the `lmap` and the other is a list of `loaders`. The list of `loaders` is the search path for loading any resources from this `URLClassPath`. When the `closeLoaders()` is called by the `URLClassLoader`, this `loaders` list is iterated to close each of the `loader` for the opened resources (which in the context of this issue translates to closing a `java.util.jar.JarFile`). A jar can have a `META-INF/INDEX.LIST`, which is a index of resources listed against each jar file. The `URLClassPath` when looking for resources in a `Loader` parses this index to search for resources. While doing so, if it finds a jar listed in the index, for which a `Loader` hasn't yet been created then it creates a new loader for such a jar and adds it to the `lmap`. However, it doesn't add this to the `loaders` list, the one which is used to close the opened jar files. As a result, it ends up opening a `JarFile` which is never closed, even if the `URLClassPath`'s `closeLoaders()` is called, thus triggering a leak. The commit in this PR adds the newly created loader to the list of `loaders` so that it is properly closed, when the `closeLoaders` gets called. I have reviewed the code to see if this has any other side effects and from what I can see, the addition of this new loader to the list shouldn't cause any other issues for reasons that follow: - the order in the `loaders` list doesn't seem to have any one-to-one mapping with the URL[] of classpath elements. In fact, loaders for newly discovered URLs from the `Class-Path` attribute get added dynamically to this list. So adding this new loader found in the index shouldn't cause any issues when it comes to the order of loaders in the list. - The `URLClassPath` is already returning resources from a loader corresponding to a jar `X` listed in the index even if that jar `X` isn't part of the original URL[] forming the classpath of the `URLClassPath`. So adding this `loader` to the "search path" won't introduce any new behaviour. A new jtreg test has been introduced to reproduce this issue and verify the fix. The test has been run on a Windows setup where the test reproduces this issue without this fix and verifies the fix. Recently, the jar index feature has been disabled as part of https://bugs.openjdk.java.net/browse/JDK-8273401. However, since users can enable this feature by using a specific system property and since this issue relates to a leak, I decided to see if there is interest in this fix. ------------- Commit messages: - 8227020: Unclosed input streams in URL Class Loader for JARs with META-INF/INDEX.LIST Changes: https://git.openjdk.java.net/jdk/pull/6389/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6389&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8227020 Stats: 121 lines in 2 files changed: 117 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/6389.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6389/head:pull/6389 PR: https://git.openjdk.java.net/jdk/pull/6389

4 9

RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle
by Mandy Chung 16 Nov '21

16 Nov '21

This reimplements core reflection with method handles. For `Constructor::newInstance` and `Method::invoke`, the new implementation uses `MethodHandle`. For `Field` accessor, the new implementation uses `VarHandle`. For the first few invocations of one of these reflective methods on a specific reflective object we invoke the corresponding method handle directly. After that we spin a dynamic bytecode stub defined in a hidden class which loads the target `MethodHandle` or `VarHandle` from its class data as a dynamically computed constant. Loading the method handle from a constant allows JIT to inline the method-handle invocation in order to achieve good performance. The VM's native reflection methods are needed during early startup, before the method-handle mechanism is initialized. That happens soon after System::initPhase1 and before System::initPhase2, after which we switch to using method handles exclusively. The core reflection and method handle implementation are updated to handle chained caller-sensitive method calls [1] properly. A caller-sensitive method can define with a caller-sensitive adapter method that will take an additional caller class parameter and the adapter method will be annotated with `@CallerSensitiveAdapter` for better auditing. See the detailed description from [2]. Ran tier1-tier8 tests. [1] https://bugs.openjdk.java.net/browse/JDK-8013527 [2] https://bugs.openjdk.java.net/browse/JDK-8271820?focusedCommentId=14439430&… ------------- Commit messages: - fix whitespace - Merge branch 'master' of https://github.com/openjdk/jdk into reimplement-method-invoke - Merge branch 'master' of https://github.com/openjdk/jdk into reimplement-method-invoke - Merge - clean up test - Merge branch 'master' of https://github.com/openjdk/jdk into reimplement-method-invoke - cleanup - Merge branch 'master' of https://github.com/openjdk/jdk into reimplement-method-invoke - fix ClassByteBuilder to handle short field type properly. Support volatile fields - minor cleanup - ... and 5 more: https://git.openjdk.java.net/jdk/compare/cb368802...c575c3db Changes: https://git.openjdk.java.net/jdk/pull/5027/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5027&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271820 Stats: 7572 lines in 75 files changed: 7103 ins; 288 del; 181 mod Patch: https://git.openjdk.java.net/jdk/pull/5027.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5027/head:pull/5027 PR: https://git.openjdk.java.net/jdk/pull/5027

15 92

RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator)
by Paul Sandoz 15 Nov '21

15 Nov '21

This PR improves the performance of vector operations that accept masks on architectures that support masking in hardware, specifically Intel AVX512 and ARM SVE. On architectures that do not support masking in hardware the same technique as before is applied to most operations, specifically composition using blend. Masked loads/stores are a special form of masked operation that require additional care to ensure out-of-bounds access throw exceptions. The range checking has not been fully optimized and will require further work. No API enhancements were required and only a few additional tests were needed. ------------- Commit messages: - Apply patch from https://github.com/openjdk/panama-vector/pull/142 - Apply patch from https://github.com/openjdk/panama-vector/pull/139 - Apply patch from https://github.com/openjdk/panama-vector/pull/151 - Add new files. - 8271515: Integration of JEP 417: Vector API (Third Incubator) Changes: https://git.openjdk.java.net/jdk/pull/5873/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5873&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271515 Stats: 21999 lines in 106 files changed: 16228 ins; 2077 del; 3694 mod Patch: https://git.openjdk.java.net/jdk/pull/5873.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5873/head:pull/5873 PR: https://git.openjdk.java.net/jdk/pull/5873

5 28

RFR: 8273660: Propagate CNF exception in FieldValues.get via an IOException
by Roger Riggs 15 Nov '21

15 Nov '21

The ObjectInputStream.GetField method `get(String name, Object val)` should have been throwing a ClassNotFoundException if the class was not found. Instead the implementation was returning null. A design error does not allow the `get(String name, Object val)` method to throw CNFE as it should. However, an exception must be thrown to prevent invalid data from being returned. Wrapping the CNFE in IOException allows it to be thrown and the exception handled. The call to `get(String name, Object val)` is always from within a `readObject` method so the deserialization logic can catch the IOException and unwrap it to handle the CNFE. ------------- Commit messages: - 8273660: Propagate CNF exception in FieldValues.get via an IOException Changes: https://git.openjdk.java.net/jdk/pull/6053/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6053&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273660 Stats: 112 lines in 2 files changed: 109 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/6053.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6053/head:pull/6053 PR: https://git.openjdk.java.net/jdk/pull/6053

7 12

RFR: 8274856: Failing jpackage tests with fastdebug/release build
by Alexey Semenyuk 15 Nov '21

15 Nov '21

The fix is to isolate C++ calls in the separate forked child process on Linux. This change requires the passing of JLI command line arguments and values of environment variables between two processes. ------------- Commit messages: - Whitespace clean up - 8274856: Failing jpackage tests with fastdebug/release build Changes: https://git.openjdk.java.net/jdk/pull/6283/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6283&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274856 Stats: 278 lines in 8 files changed: 232 ins; 20 del; 26 mod Patch: https://git.openjdk.java.net/jdk/pull/6283.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6283/head:pull/6283 PR: https://git.openjdk.java.net/jdk/pull/6283

4 4

RFR: 8276084: Linux DEB Bundler: release number in outputted .deb file should be optional
by Alexey Semenyuk 15 Nov '21

15 Nov '21

8276084: Linux DEB Bundler: release number in outputted .deb file should be optional ------------- Commit messages: - Bugfix - Bugfix - Bugfix - 8276084: Linux DEB Bundler: release number in outputted .deb file should be optional Changes: https://git.openjdk.java.net/jdk/pull/6345/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6345&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276084 Stats: 65 lines in 7 files changed: 52 ins; 1 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/6345.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6345/head:pull/6345 PR: https://git.openjdk.java.net/jdk/pull/6345

4 5

RFR: 8277048: Tiny improvements to the specification text for java.util.Properties.load
by Pavel Rappo 15 Nov '21

15 Nov '21

Please review this simple two-hunk fix to the documentation comment of java.util.Properties#load(java.io.Reader). The first hunk (line/lines) is a suggestion. I believe it reads better since the plurality is more idiomatic; native English speakers should correct me if I'm wrong. The second hunk picks up what was overlooked in JDK-8274075 (https://git.openjdk.java.net/jdk/pull/5610) ------------- Commit messages: - Initial commit Changes: https://git.openjdk.java.net/jdk/pull/6367/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6367&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277048 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/6367.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6367/head:pull/6367 PR: https://git.openjdk.java.net/jdk/pull/6367

4 4