May 2021 - core-libs-dev

RFR: 8267612: Declare package-private VarHandle.AccessMode/-Type counts
by Claes Redestad 25 May '21

25 May '21

Slightly reduce VarHandle startup overhead by introducing package-private COUNT constants for two enums ------------- Commit messages: - Declare package-private VarHandle.AccessMode/-Type counts Changes: https://git.openjdk.java.net/jdk/pull/4164/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4164&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267612 Stats: 18 lines in 4 files changed: 8 ins; 2 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/4164.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4164/head:pull/4164 PR: https://git.openjdk.java.net/jdk/pull/4164

2 5

Re: RFR: 8265768 [aarch64] Use glibc libm impl for dlog, dlog10, dexp iff 2.29 or greater on AArch64.
by Andrew Dinn 25 May '21

25 May '21

On Thu, 15 Apr 2021 08:33:47 GMT, gregcawthorne <github.com+73799211+gregcawthorne(a)openjdk.org> wrote: > Glibc 2.29 onwards provides optimised versions of log,log10,exp. > These functions have an accuracy of 0.9ulp or better in glibc > 2.29. > > Therefore this patch adds code to parse, store and check > the runtime glibcs version in os_linux.cpp/hpp. > This is then used to select the glibcs implementation of > log, log10, exp at runtime for c1 and c2, iff we have > glibc 2.29 or greater. > > This will ensure OpenJDK can benefit from future improvements > to glibc. > > Glibc adheres to the ieee754 standard, unless stated otherwise > in its spec. > > As there are no stated exceptions in the current glibc spec > for dlog, dlog10 and dexp, we can assume they currently follow > ieee754 (which testing confirms). As such, future version of > glibc are unlikely to lose this compliance with ieee754 in > future. > > W.r.t performance this patch sees ~15-30% performance improvements for > log and log10, with ~50-80% performance improvements for exp for the > common input ranged (which output real numbers). However for the NaN > and inf output ranges we see a slow down of up to a factor of 2 for > some functions and architectures. > > Due to this being the uncommon case we assert that this is a > worthwhile tradeoff. > [ One thing: Java uses the term "semi-monotonic" to > mean "whenever the mathematical function is non-decreasing, so is > the floating-point approximation, likewise, whenever the > mathematical function is non-increasing, so is the floating-point > approximation." I don't really understand what distinction means. ] I believe this is to allow for the fact that the function is continuous and the floating-point approximation is discrete. Let F be the actual function and f the floating point approximation. Assume we have two successive floating point values x, x' and, without loss of generality, F(x) <= F(x'). What are the circumstances under which we require f(x) =< f(x')? Semi-monotonicity says that is only needed when F is non-decreasing on the interval [x, x']. Expressed more precisely, the condition that F is non-decreasing is for all y such that x =< y =< x' : F(x) <= F(y) <= F(x'). In other words: if the graph only ever stays level or increases across the interval [x, x'] then we must have f(x) =< f(x') If the graph wiggles *up* and *down* across the interval [x, x'] we can allow f(x) > f(x'). ------------- PR: https://git.openjdk.java.net/jdk/pull/3510

1 0

RFR: 8267110: Update java.util to use instanceof pattern variable
by Patrick Concannon 25 May '21

25 May '21

Hi, Could someone please review my code for updating the code in the `java.util` package to make use of the `instanceof` pattern variable? Kind regards, Patrick ------------- Commit messages: - 8267110: Update java.util to use instanceof pattern variable Changes: https://git.openjdk.java.net/jdk/pull/4088/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4088&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267110 Stats: 304 lines in 35 files changed: 1 ins; 144 del; 159 mod Patch: https://git.openjdk.java.net/jdk/pull/4088.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4088/head:pull/4088 PR: https://git.openjdk.java.net/jdk/pull/4088

5 9

RFR: 8224243: Make AccessibleObject a sealed class
by Joe Darcy 25 May '21

25 May '21

Conceptually, AccessbileObject is a sealed class with a protected constructor stating Constructor: only used by the Java Virtual Machine. With the language now supporting sealed classes, the AccessbileObject should be marked as sealed. Executable and Field are the subclasses of AccessbileObject in the JDK; as Executable has subclasses, it is marked as non-sealed. Please also review the corresponding CSR: https://bugs.openjdk.java.net/browse/JDK-8224243 ------------- Commit messages: - 8224243: Make AccessibleObject a sealed class Changes: https://git.openjdk.java.net/jdk/pull/4133/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4133&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8224243 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4133.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4133/head:pull/4133 PR: https://git.openjdk.java.net/jdk/pull/4133

7 15

Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v9]
by Yi Yang 25 May '21

25 May '21

> The JDK codebase re-created many variants of checkIndex(`grep -I -r 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which annotated with @IntrinsicCandidate and it only has a corresponding C1 intrinsic version. > > In fact, there is an utility method `jdk.internal.util.Preconditions.checkIndex`(wrapped by java.lang.Objects.checkIndex) that behaves the same as these variants of checkIndex, we can replace these re-created variants of checkIndex by Objects.checkIndex, it would significantly reduce duplicated code and enjoys performance improvement because Preconditions.checkIndex is @IntrinsicCandidate and it has a corresponding intrinsic method in HotSpot. > > But, the problem is currently HotSpot only implements the C2 version of Preconditions.checkIndex. To reuse it global-widely in JDK code, I think we can firstly implement its C1 counterpart. There are also a few kinds of stuff we can do later: > > 1. Replace all variants of checkIndex by Objects.checkIndex in the whole JDK codebase. > 2. Remove Buffer.checkIndex and obsolete/deprecate InlineNIOCheckIndex flag > > Testing: cds, compiler and jdk Yi Yang has updated the pull request incrementally with one additional commit since the last revision: x86_32 fails ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3615/files - new: https://git.openjdk.java.net/jdk/pull/3615/files/f996c99f..307d7a10 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3615&range=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3615&range=07-08 Stats: 9 lines in 1 file changed: 7 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3615.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3615/head:pull/3615 PR: https://git.openjdk.java.net/jdk/pull/3615

2 1

RFR: 8267614: Outline VarHandleGuards exact behavior checks
by Claes Redestad 24 May '21

24 May '21

Extract some common behavior checks from the generated VarHandleGuards code to a package-private VarHandle method. This reduces the code size (-5.2kb lib/modules) and slightly improves startup, while being performance neutral on microbenchmarks. ------------- Commit messages: - Outline VarHandleGuard exact behavior checks Changes: https://git.openjdk.java.net/jdk/pull/4165/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4165&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267614 Stats: 338 lines in 3 files changed: 14 ins; 243 del; 81 mod Patch: https://git.openjdk.java.net/jdk/pull/4165.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4165/head:pull/4165 PR: https://git.openjdk.java.net/jdk/pull/4165

3 4

RFR: JDK-8267423: Fix copyrights in jpackage tests
by Andy Herrick 24 May '21

24 May '21

JDK-8267423: Fix copyrights in jpackage tests ------------- Commit messages: - JDK-8267423: Fix copyrights in jpackage tests - JDK-8267423: Fix copyrights in jpackage tests - JDK-8267423: Fix copyrights in jpackage tests - JDK-8267423: Fix copyrights in jpackage tests Changes: https://git.openjdk.java.net/jdk/pull/4144/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4144&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267423 Stats: 96 lines in 10 files changed: 69 ins; 14 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/4144.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4144/head:pull/4144 PR: https://git.openjdk.java.net/jdk/pull/4144

3 3

Obviously erroneous code in JapaneseImperialCalendar::actualMonthLength
by Tagir Valeev 24 May '21

24 May '21

Hello! When testing our static analyzer, I've found obviously erroneous code in java.util.JapaneseImperialCalendar::actualMonthLength int eraIndex = getTransitionEraIndex(jdate); if (eraIndex == -1) { long transitionFixedDate = sinceFixedDates[eraIndex]; CalendarDate d = eras[eraIndex].getSinceDate(); if (transitionFixedDate <= cachedFixedDate) { length -= d.getDayOfMonth() - 1; } else { length = d.getDayOfMonth() - 1; } } if this 'if' statement is visited then sinceFixedDates[eraIndex] will surely fail with ArrayIndexOutOfBoundsException. However, judging from the usages of this method, eraIndex is never -1 here, so the whole 'if' statement is a dead code. I'm not sure whether it should be removed, or the condition should be changed to 'eraIndex != -1' (and probably some unit-tests should be written to cover this). I have no expertise in this code so I'm not sure what would be a proper way to fix it. Please take a look. With best regards, Tagir Valeev.

2 1

RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager
by Weijun Wang 24 May '21

24 May '21

Please review the test changes for [JEP 411](https://openjdk.java.net/jeps/411). With JEP 411 and the default value of `-Djava.security.manager` becoming `disallow`, tests calling `System.setSecurityManager()` need `-Djava.security.manager=allow` when launched. This PR covers such changes for tier1 to tier3 (except for the JCK tests). To make it easier to focus your review on the tests in your area, this PR is divided into multiple commits for different areas. Mostly the rule is the same as how Skara adds labels, but there are several small tweaks: 1. When a file is covered by multiple labels, only one is chosen. I make my best to avoid putting too many tests into `core-libs`. If a file is covered by `core-libs` and another label, I categorized it into the other label. 2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the `xml` commit. 3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the `rmi` commit. 4. One file not covered by any label -- `test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in the `swing` commit. Due to the size of this PR, no attempt is made to update copyright years for all files to minimize unnecessary merge conflict. Please note that this PR can be integrated before the source changes for JEP 411, as the possible values of this system property was already defined long time ago in JDK 9. Most of the change in this PR is a simple adding of `-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes it was not `othervm` and we add one. Sometimes there's no `@run` at all and we add the line. There are several tests that launch another Java process that needs to call the `System.setSecurityManager()` method, and the system property is added to `ProcessBuilder`, `ProcessTools`, or the java command line (if the test is a shell test). 3 langtools tests are added into problem list due to [JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611). 2 SQL tests are moved because they need different options on the `@run` line but they are inside a directory that has a `TEST.properties`: rename test/jdk/java/sql/{testng/test/sql/othervm => permissionTests}/DriverManagerPermissionsTests.java (93%) rename test/jdk/javax/sql/{testng/test/rowset/spi => permissionTests}/SyncFactoryPermissionsTests.java (95%) ``` The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073. ------------- Commit messages: - test for awt - test for hotspot-gc - test for compiler - test for net - test for core-libs - test for beans - test for xml - test for nio - test for hotspot-runtime - test for security - ... and 7 more: https://git.openjdk.java.net/jdk/compare/79b39445...900c28c0 Changes: https://git.openjdk.java.net/jdk/pull/4071/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4071&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267184 Stats: 1024 lines in 852 files changed: 249 ins; 8 del; 767 mod Patch: https://git.openjdk.java.net/jdk/pull/4071.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4071/head:pull/4071 PR: https://git.openjdk.java.net/jdk/pull/4071

7 13

RFR: 8224158: assertion related to NPE at DynamicCallSiteDesc::withArgs should be reworded
by Vicente Romero 24 May '21

24 May '21

This is a very small patch that is just rewording the spec for DynamicCallSiteDesc::withArgs. Basically adding that NPE can also be thrown if the content of the argument is `null` TIA for the review ------------- Commit messages: - 8224158: assertion related to NPE at DynamicCallSiteDesc::withArgs should be reworded Changes: https://git.openjdk.java.net/jdk/pull/4067/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4067&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8224158 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4067.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4067/head:pull/4067 PR: https://git.openjdk.java.net/jdk/pull/4067

2 4