August 2021 - core-libs-dev

RFR: 8271599: Javadoc of floorDiv() and floorMod() families is inaccurate in some places
by Raffaello Giulietti 04 Aug '21

04 Aug '21

Hello, please review the changes in documentation of floorDiv() and floorMod() in all their variants. Some are clarifications, some are corrections. Here's an example of a confusing formulation in the current doc > "the / operator returns the integer closest to zero" Literally, the integer closest to zero is zero! The doc of floorMod(int, int) also states: > "If the signs of arguments are unknown and a positive modulus > is needed it can be computed as (floorMod(x, y) + abs(y)) % abs(y)." That's a questionable advice, as the sum in parentheses can lead to irrecoverable overflow (beside requiring two divisions instead of one). E.g., setting x = Integer.MAX_VALUE - 1, y = Integer.MAX_VALUE leads to the quite incorrect result -3, which is not even positive! Greetings Raffaello ------------- Commit messages: - 8271599: Javadoc of floorDiv() and floorMod() families is inaccurate in some places Changes: https://git.openjdk.java.net/jdk/pull/4963/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4963&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271599 Stats: 31 lines in 1 file changed: 4 ins; 7 del; 20 mod Patch: https://git.openjdk.java.net/jdk/pull/4963.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4963/head:pull/4963 PR: https://git.openjdk.java.net/jdk/pull/4963

3 10

RFR: JDK-8256844: Make NMT late-initializable
by Thomas Stuefe 04 Aug '21

04 Aug '21

Short: this patch makes NMT available in custom-launcher scenarios and during gtests. It simplifies NMT initialization. It adds a lot of NMT-specific testing. --------- NMT continues to be an extremely useful tool for SAP to tackle memory problems in the JVM. However, NMT is of limited use due to the following restrictions: - NMT cannot be used if the hotspot is embedded into a custom launcher unless the launcher actively cooperates. Just creating and invoking the JVM is not enough, it needs to do some steps prior to loading the hotspot. This limitation is not well known (nor, do I believe, documented). Many products don't do this, e.g., you cannot use NMT with IntelliJ. For us at SAP this problem limits NMT usefulness greatly since our VMs are often embedded into custom launchers and modifying every launcher is impossible. - Worse, if that custom launcher links the libjvm *statically* there is just no way to activate NMT at all. This is the reason NMT cannot be used in the `gtestlauncher`. - Related to that is that we cannot pass NMT options via `JAVA_TOOL_OPTIONS` and `-XX:Flags=<file>`. - The fact that NMT cannot be used in gtests is really a pity since it would allow us to both test NMT itself more rigorously and check for memory leaks while testing other stuff. The reason for all this is that NMT initialization happens very early, on the first call to `os::malloc()`. And those calls happen already during dynamic C++ initialization - a long time before the VM gets around parsing arguments. So, regular VM argument parsing is too late to parse NMT arguments. The current solution is to pass NMT arguments via a specially prepared environment variable: `NMT_LEVEL_<PID>=<NMT arguments>`. That environment variable has to be set by the embedding launcher, before it loads the libjvm. Since its name contains the PID, we cannot even set that variable in the shell before starting the launcher. All that means that every launcher needs to especially parse and process the NMT arguments given at the command line (or via whatever method) and prepare the environment variable. `java` itself does this. This only works before the libjvm.so is loaded, before its dynamic C++ initialization. For that reason, it does not work if the launcher links statically against the hotspot, since in that case C++ initialization of the launcher and hotspot are folded into one phase with no possibility of executing code beforehand. And since it bypasses argument handling in the VM, it bypasses a number of argument processing ways, e.g., `JAVA_TOOL_OPTIONS`. ------ This patch fixes these shortcomings by making NMT late-initializable: it can now be initialized after normal VM argument parsing, like all other parts of the VM. This greatly simplifies NMT initialization and makes it work automagically for every third party launcher, as well as within our gtests. The glaring problem with late-initializing NMT is the NMT malloc headers. If we rule out just always having them (unacceptable in terms of memory overhead), there is no safe way to determine, in os::free(), if an allocation came from before or after NMT initialization ran, and therefore what to do with its malloc headers. For a more extensive explanation, please see the comment block `nmtPreInit.hpp` and the discussion with @kimbarrett and @zhengyu123 in the JBS comment section. The heart of this patch is a new way to track early, pre-NMT-init allocations. These are tracked via a lookup table. This was a suggestion by Kim and it worked out well. Changes in detail: - pre-NMT-init handling: - the new files `nmtPreInit.hpp/cpp` take case of NMT pre-init handling. They contain a small global lookup table managing C-heap blocks allocated in the pre-NMT-init phase. - `os::malloc()/os::realloc()/os::free()` defer to this code before doing anything else. - Please see the extensive comment block at the start of `nmtPreinit.hpp` explaining the details. - Changes to NMT: - Before, NMT initialization was spread over two phases, `initialize()` and `late_initialize()`. Those were merged into one and simplified - there is only one initialization now which happens after argument parsing. - Minor changes were needed for the `NMT_TrackingLevel` enum - to simplify code, I changed NMT_unknown to be numerically 0. A new comment block in `nmtCommon.hpp` now clearly specifies what's what, including allowed level state transitions. - New utility functions to translate tracking level from/to strings added to `NMTUtil` - NMT has never been able to handle virtual memory allocations before initialization, which is fine since os::reserve_memory() is not called before VM parses arguments. We now assert that. - All code outside the VM handling NMT initialization (eg. libjli) has been removed, as has the code testing it. - Gtests: - Some existing gtests had to be modified: before, they all changed global state (turning NMT on/off) before testing. This is not allowed anymore, to keep NMT simple. Also, this pattern disturbed other tests. - The new way to test is to passively check whether NMT has been switched on or off, and do tests accordingly: if on, full tests, if off, test just what makes sense in off-state. That does not disturb neighboring tests, gives us actually better coverage all around. - It is now possible to start the gtestlauncher with NMT on! Which additionally gives us good coverage. - To actually do gtests with NMT - since it's disabled by default - we now run NMT-enabled gtests as part of the hotspot jtreg NMT wrapper. This pattern we have done for a number of other facitilites, see all the tests in test/hotspot/jtreg/gtest.. . It works very well. - Finally, a new gtest has been written to test the NMT preinit lookup map in isolation, placed in `gtest/nmt/test_nmtpreinitmap.cpp`. - jtreg: - A new test has been added, `runtime/NMT/NMTInitializationTest.java`, testing NMT initialization in the face of many many VM arguments. ------------- Tests: - ran manually all new tests on 64-bit and 32-bit Linux - GHAs - The patch has been active in SAPs test systems for a while now. ------------- Commit messages: - NMT late init, hashmap variant Changes: https://git.openjdk.java.net/jdk/pull/4874/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4874&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8256844 Stats: 1616 lines in 22 files changed: 1227 ins; 346 del; 43 mod Patch: https://git.openjdk.java.net/jdk/pull/4874.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4874/head:pull/4874 PR: https://git.openjdk.java.net/jdk/pull/4874

6 32

RFR: 8260710: Inline and simplify String*::{coder, value, isLatin1} methods
by Aleksey Shipilev 04 Aug '21

04 Aug '21

Since Compact Strings implementation, there are simple methods in String and StringBuilders: `coder()`, `value()`, `isLatin1()`. They are mostly there to capture `COMPACT_STRINGS` flag that would fold to "false" when compact strings are disabled with `-XX:-CompactStrings`. This was to guarantee the same performance when Compact String are unimplemented and/or have surprising performance behaviors. Quite some time had passed since JDK 9 release, and we can consider simplifying these. For example, ignoring `COMPACT_STRINGS` and inlining the methods simplifies the code. Some new code added to `String` after JDK 9 already has these methods inlined, checking `coder` directly. The major drawback is that supplying `-XX:-CompactStrings` would have to go through the same `coder == UTF16` paths the "normal" Compact Strings are going. It would not be folded to the most optimal form by the optimizer, because `coder` field value is generally opaque to optimizers. On the flip side, there is an advantage on paths that do not have optimizer ready (yet), for example at startup. Hello World startup improves for about 0.3 msec (or ~1.3%). The improvement is quite probably more visible on larger workloads, i.e. large application servers. (In fact, this is where I spotted this opportunity: OpenLiberty startup profiles). Interpreter-only "Hello World": Performance counter stats for 'taskset -c 13 build/baseline/bin/java -Xms128m -Xmx128m -Xint Hello' (5000 runs): 23.40 msec task-clock # 0.933 CPUs utilized ( +- 0.01% ) 86,133,190 cycles # 3.680 GHz ( +- 0.01% ) 79,854,588 instructions # 0.93 insn per cycle ( +- 0.00% ) 0.02507105 +- 0.00000343 seconds time elapsed ( +- 0.01% ) Performance counter stats for 'taskset -c 13 build/linux-x86_64-server-release/images/jdk/bin/java -Xms128m -Xmx128m -Xint Hello' (5000 runs): 23.07 msec task-clock # 0.935 CPUs utilized ( +- 0.01% ) 84,877,118 cycles # 3.679 GHz ( +- 0.01% ) 79,231,598 instructions # 0.93 insn per cycle ( +- 0.00% ) 0.02466467 +- 0.00000382 seconds time elapsed ( +- 0.02% ) OpenLiberty startup: Before: 2.296, 2.281, 2.291 (seconds) After: 2.254, 2.267, 2.272 (seconds) Additional tests: - [x] Linux `tier1` default (passes) - [x] Linux `tier1` `-XX:-CompactStrings` ------------- Commit messages: - Merge branch 'master' into JDK-8260710-string-inline - Merge branch 'master' into JDK-8260710-string-inline - Initial prototype Changes: https://git.openjdk.java.net/jdk/pull/2334/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2334&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8260710 Stats: 173 lines in 5 files changed: 5 ins; 48 del; 120 mod Patch: https://git.openjdk.java.net/jdk/pull/2334.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2334/head:pull/2334 PR: https://git.openjdk.java.net/jdk/pull/2334

5 5

jpackage MacOS Notarization
by Daniel Peintner 04 Aug '21

04 Aug '21

All, I am trying to notarize an app (built with jpackage) for MacOS. jpackage at first *seems* to properly sign all resources with the available --mac-sign options et cetera. Having said that, there are still remaining issues 1. The app cannot be properly installed (without hacks like xattr -d com.apple.quarantine /Applications/myAPP.app ). 2. I am also not able to properly notarize the app. According to online resources there seem to exist issues in *past* about notarization but according to [1, 2] the issues are fixed. As mentioned, I still have issues :-( Am I really the only one still having problems? Java Version: AdoptOpenJDK-16.0.1+9 (tried Oracle JDK also without success) The issue seems to boil down to 2 errors (attached the error log from Apple notarization process). * app Folder * libjli.dylib I thought I better ask first on the mailing list before creating an actual bug report. Note1: I used to use the *old* javapackager that worked with the same signature/credentials. Note2: running jpackage without --mac-sign options causes many more errors in notarization (Hence, jpackage signs most resources but fails with some) Any help / hint appreciated. Thanks, -- Daniel [1] https://bugs.openjdk.java.net/browse/JDK-8257488 [2] https://bugs.openjdk.java.net/browse/JDK-8251892 ## APPLE LOG ## { "logFormatVersion": 1, "jobId": "...", "status": "Invalid", "statusSummary": "Archive contains critical validation errors", "statusCode": 4000, "archiveFilename": "myAPP-21.07.28.dmg", "uploadDate": "2021-07-28T14:31:24Z", "sha256": "...", "ticketContents": null, "issues": [ { "severity": "error", "code": null, "path": "myAPP-21.07.28.dmg/myAPP.app/Contents/MacOS/myAPP", "message": "The signature of the binary is invalid.", "docUrl": null, "architecture": "x86_64" }, { "severity": "error", "code": null, "path": "myAPP-21.07.28.dmg/myAPP.app/Contents/runtime/Contents/MacOS/libjli.dylib", "message": "The signature of the binary is invalid.", "docUrl": null, "architecture": "x86_64" } ] }

3 10

RFR: 8271616: oddPart in MutableBigInteger::mutableModInverse contains info on final result
by Weijun Wang 04 Aug '21

04 Aug '21

oddPart contains a lot of info on the modInverse output, sometimes it's even the same. Clearing it in case the result is sensitive. No new regression test since it's difficult to access a temporary local variable in an internal class. Existing tier1-2 tests passed. ------------- Commit messages: - 8271616: oddPart in MutableBigInteger::mutableModInverse contains info on final result Changes: https://git.openjdk.java.net/jdk/pull/4973/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4973&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271616 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4973.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4973/head:pull/4973 PR: https://git.openjdk.java.net/jdk/pull/4973

4 4

RFR: JDK-8271344: Windows product version issue
by Andy Herrick 03 Aug '21

03 Aug '21

JDK-8271344: Windows product version issue ------------- Commit messages: - JDK-8271344: Windows product version issue Changes: https://git.openjdk.java.net/jdk/pull/4958/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4958&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271344 Stats: 21 lines in 5 files changed: 5 ins; 2 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/4958.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4958/head:pull/4958 PR: https://git.openjdk.java.net/jdk/pull/4958

3 3

Quick question on JDK-8198997 and normalization of user.dir
by Volker Simonis 03 Aug '21

03 Aug '21

Hi, I have a quick question on JDK-8198997 [1] (a follow-up of JDK-8194154 [2]) which introduced normalization for the cached "user.dir" property in order to avoid "inefficient, repeated normalization". However, from what I can see, WinNTFileSystem.getUserPath(), the only place where the cached and normalized "userDir" is used, still calls normalize() on each invocation: class WinNTFileSystem extends FileSystem { ... private final String userDir; ... public WinNTFileSystem() { ... userDir = normalize(props.getProperty("user.dir")); } ... private String getUserPath() { ... return normalize(userDir); } Is this just an oversight (in which case I'll fix it) or am I missing something? Thank you and best regards, Volker [1] https://bugs.openjdk.java.net/browse/JDK-8198997 [2] https://bugs.openjdk.java.net/browse/JDK-8194154

2 2

RFR: Merge jdk17
by Jesper Wilhelmsson 03 Aug '21

03 Aug '21

Forwardport JDK 17 -> JDK 18 ------------- Commit messages: - Merge - 8067223: [TESTBUG] Rename Whitebox API package - 8271150: Remove EA from JDK 17 version string starting with Initial RC promotion on Aug 5, 2021(B34) The webrevs contain the adjustments done while merging with regards to each parent branch: - master: https://webrevs.openjdk.java.net/?repo=jdk&pr=4964&range=00.0 - jdk17: https://webrevs.openjdk.java.net/?repo=jdk&pr=4964&range=00.1 Changes: https://git.openjdk.java.net/jdk/pull/4964/files Stats: 535 lines in 37 files changed: 449 ins; 1 del; 85 mod Patch: https://git.openjdk.java.net/jdk/pull/4964.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4964/head:pull/4964 PR: https://git.openjdk.java.net/jdk/pull/4964

2 3

RFR: 8271624: Avoid unnecessary ThreadGroup.checkAccess calls when creating Threads
by Claes Redestad 02 Aug '21

02 Aug '21

Trivially avoid some redundant calls when creating threads. (ThreadGroup.checkAccess is final, and all it does is call security.checkAccess(g) if there's a SM installed) ------------- Commit messages: - Merge branch 'master' into thread_access - Avoid calling ThreadGroup.checkAccess when no SecurityManager is installed Changes: https://git.openjdk.java.net/jdk/pull/4959/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4959&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271624 Stats: 8 lines in 1 file changed: 4 ins; 4 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4959.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4959/head:pull/4959 PR: https://git.openjdk.java.net/jdk/pull/4959

2 3

RFR: 8271627: Use local field access in favor of Class.getClassLoader0
by Claes Redestad 02 Aug '21

02 Aug '21

Trivial startup hack/cleanup to use the `classLoader` field directly rather than the `getClassLoader0` accessor. Before aa12c8fbf5b020d168cf29412643bf15514b0439 there was no `classLoader` field and `getClassLoader0` was a native method that retrieved the `ClassLoader` by calling into the runtime. However, now that `classLoader` is injected on mirror creation it makes less sense to use the accessor when not necessary. ------------- Commit messages: - Merge branch 'master' into startup_classLoader0 - Avoid getClassLoader0 calls from within Class - Reuse SecurityConstants.ACCESS_PERMISSION from MethodHandles Changes: https://git.openjdk.java.net/jdk/pull/4960/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4960&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271627 Stats: 10 lines in 1 file changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/4960.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4960/head:pull/4960 PR: https://git.openjdk.java.net/jdk/pull/4960

2 3