September 2021 - core-libs-dev

RFR: 8274522: java/lang/management/ManagementFactory/MXBeanException.java test fails with Shenandoah
by Aleksey Shipilev 29 Sep '21

29 Sep '21

Fails like this: $ CONF=linux-x86_64-server-fastdebug make run-test TEST=java/lang/management/ManagementFactory/MXBeanException.java TEST_VM_OPTS="-XX:+UseShenandoahGC" ... java.lang.NullPointerException: Cannot invoke "java.lang.management.MemoryPoolMXBean.setUsageThreshold(long)" because "MXBeanException.pool" is null This test tries to find the pool that is `!p.isUsageThresholdSupported()`, and for Shenandoah there is no such pool. Likewise with ZGC, which is already filtered. Additional testing: - [x] Affected test is now skipped for Shenandoah - [x] Affected test is still skipped for Z - [x] Affected test is still passing for G1 ------------- Commit messages: - Fix Changes: https://git.openjdk.java.net/jdk/pull/5757/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5757&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274522 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5757.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5757/head:pull/5757 PR: https://git.openjdk.java.net/jdk/pull/5757

1 0

RFR: 8274242: Implement fast-path for ASCII-compatible CharsetEncoders on x86
by Claes Redestad 29 Sep '21

29 Sep '21

This patch extends the `ISO_8859_1.implEncodeISOArray` intrinsic on x86 to work also for ASCII encoding, which makes for example the `UTF_8$Encoder` perform on par with (or outperform) similarly getting charset encoded bytes from a String. The former took a small performance hit in JDK 9, and the latter improved greatly in the same release. Extending the `EncodeIsoArray` intrinsics on other platforms should be possible, but I'm unfamiliar with the macro assembler in general and unlike the x86 intrinsic they don't use a simple vectorized mask to implement the latin-1 check. For example aarch64 seem to filter out the low bytes and then check if there's any bits set in the high bytes. Clever, but very different to the 0xFF80 2-byte mask that an ASCII test wants. ------------- Commit messages: - Freshen up CharsetEncodeDecode micro (still only tests ASCII), optimize ASCII-compatible SingleByte (e.g. ISO-8859-15) - Add @bug id to test - Move and adapt defunct Test6896617 test to test more Charsets without using internal APIs; remove adhoc performance tests - Exclude encodeAsciiArray intrinsic on all non-X86 platforms - Fix indentation - Remove EncodeAsciiArray node and instead extend EncodeISOArray with an is_ascii predicate - Merge MacroAssembler methods - Implement intrinsified ASCII fast-path by copying and adapting encodeISOArray intrinsic (currently x86 only) - Enhance UTF_8.Encoder by using StringUTF16.compress for ASCII Changes: https://git.openjdk.java.net/jdk/pull/5621/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5621&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274242 Stats: 763 lines in 20 files changed: 375 ins; 355 del; 33 mod Patch: https://git.openjdk.java.net/jdk/pull/5621.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5621/head:pull/5621 PR: https://git.openjdk.java.net/jdk/pull/5621

6 23

RFR: 8273656: Improve java.lang.invoke.MethodType.parameterList() and its usage
by Сергей Цыпанов 29 Sep '21

29 Sep '21

Currently the method is implemented like public List<Class<?>> parameterList() { return Collections.unmodifiableList(Arrays.asList(ptypes.clone())); } This seems to be excessive, as three objects are allocated here. Instead we can use `List.of(ptypes)` which doesn't allocate anything for empty array and for one of length 1 and 2 it allocates lightweight objects with 2 fields, still copying longer arrays. This is likely to be fruitful as most of methods have 0-2 parameters. Also there is a couple of cases when `MethodType.parameterLis()` is called to get its size, which is excessive either as we can use `MethodType.parameterCount()` instead. ------------- Commit messages: - 8273656: Improve java.lang.invoke.MethodType.parameterList() and its usage Changes: https://git.openjdk.java.net/jdk/pull/5489/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5489&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273656 Stats: 7 lines in 2 files changed: 0 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/5489.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5489/head:pull/5489 PR: https://git.openjdk.java.net/jdk/pull/5489

7 13

RFR: 8274391: Suppress more warnings on non-serializable non-transient instance fields in java.util.concurrent
by Joe Darcy 28 Sep '21

28 Sep '21

Follow-up change to JDK-8232230, augmentations to javac's Xlint:serial checking are out for review (https://github.com/openjdk/jdk/pull/5709) and java.util.concurrent would need some changes to pass under the expanded checks. The changes are to suppress warnings where non-transient fields in serializable types are not declared with a type statically known to be serializable. That isn't necessarily a correctness issues, but it does merit further scrutiny. In ForkJoinTask, the SuppressWarnings annotation previously applied in JDK-8232230, seems to have been misplaced to a different field; this change corrects it. ------------- Commit messages: - 8274391: Suppress more warnings on non-serializable non-transient instance fields in java.util.concurrent Changes: https://git.openjdk.java.net/jdk/pull/5718/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5718&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274391 Stats: 3 lines in 2 files changed: 2 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5718.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5718/head:pull/5718 PR: https://git.openjdk.java.net/jdk/pull/5718

6 7

RFR: 8274415: Suppress warnings on non-serializable non-transient instance fields in java.xml
by Joe Darcy 28 Sep '21

28 Sep '21

Augmentations to javac's Xlint:serial checking are out for review (#5709) and the java.xml module would need some changes to pass under the expanded checks. The changes are to suppress warnings where non-transient fields in serializable types are not declared with a type statically known to be serializable. That isn't necessarily a correctness issues, but it does merit further scrutiny. I'll run a script to update the copyright years before pushing. ------------- Commit messages: - 8274415: Suppress warnings on non-serializable non-transient instance fields in java.xml Changes: https://git.openjdk.java.net/jdk/pull/5730/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5730&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274415 Stats: 45 lines in 27 files changed: 45 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5730.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5730/head:pull/5730 PR: https://git.openjdk.java.net/jdk/pull/5730

2 4

RFR: 8274394: Use Optional.isEmpty instead of !Optional.isPresent in jdk.jlink
by Andrey Turbanov 28 Sep '21

28 Sep '21

I propose to replace usages of !Optional.isPresent() with Optional.isEmpty method. It's makes code a bit easier to read. Noticing negation before long chain of method calls is hard. ------------- Commit messages: - [PATCH] Use Optional.isEmpty instead of !Optional.isPresent in jdk.jlink Changes: https://git.openjdk.java.net/jdk/pull/5707/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5707&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274394 Stats: 8 lines in 5 files changed: 0 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/5707.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5707/head:pull/5707 PR: https://git.openjdk.java.net/jdk/pull/5707

3 3

RFR: 8274276: Cache normalizedBase URL in URLClassPath.FileLoader
by Сергей Цыпанов 28 Sep '21

28 Sep '21

Currently on each invocation of `URLClassPath.FileLoader.getResource()` `normalizedBase` URL is recalculated using same final `baseURL` from parent class. This can be avoided giving significant improvement in memory consumption. Consider the benchmark: @State(Scope.Benchmark) @BenchmarkMode(Mode.AverageTime) @OutputTimeUnit(TimeUnit.NANOSECONDS) @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"}) public class URLClassPathBenchmark { private final ClassLoader classLoader = getClass().getClassLoader(); @Benchmark public URL getResource() { return classLoader.getResource("file:./config/application.properties"); } } it demonstrates improvement brought in by this patch: before Benchmark Mode Cnt Score Error Units URLClassPathBenchmark.getResource avgt 50 2840,746 ± 22,206 ns/op URLClassPathBenchmark.getResource:·gc.alloc.rate avgt 50 645,313 ± 8,072 MB/sec URLClassPathBenchmark.getResource:·gc.alloc.rate.norm avgt 50 2403,258 ± 17,639 B/op URLClassPathBenchmark.getResource:·gc.churn.G1_Eden_Space avgt 50 656,624 ± 116,090 MB/sec URLClassPathBenchmark.getResource:·gc.churn.G1_Eden_Space.norm avgt 50 2450,175 ± 440,011 B/op URLClassPathBenchmark.getResource:·gc.churn.G1_Survivor_Space avgt 50 0,123 ± 0,149 MB/sec URLClassPathBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm avgt 50 0,459 ± 0,556 B/op URLClassPathBenchmark.getResource:·gc.count avgt 50 67,000 counts URLClassPathBenchmark.getResource:·gc.time avgt 50 117,000 ms after Benchmark Mode Cnt Score Error Units URLClassPathBenchmark.getResource avgt 100 2596,719 ± 9,786 ns/op URLClassPathBenchmark.getResource:·gc.alloc.rate avgt 100 448,780 ± 1,684 MB/sec URLClassPathBenchmark.getResource:·gc.alloc.rate.norm avgt 100 1528,040 ± 0,005 B/op URLClassPathBenchmark.getResource:·gc.churn.G1_Eden_Space avgt 100 479,905 ± 23,369 MB/sec URLClassPathBenchmark.getResource:·gc.churn.G1_Eden_Space.norm avgt 100 1634,314 ± 79,821 B/op URLClassPathBenchmark.getResource:·gc.churn.G1_Survivor_Space avgt 100 0,101 ± 0,097 MB/sec URLClassPathBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm avgt 100 0,345 ± 0,329 B/op URLClassPathBenchmark.getResource:·gc.count avgt 100 98,000 counts URLClassPathBenchmark.getResource:·gc.time avgt 100 218,000 ms ------------- Commit messages: - 8274276: Cache normalizedBase URL in URLClassPath.FileLoader Changes: https://git.openjdk.java.net/jdk/pull/5677/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5677&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274276 Stats: 4 lines in 1 file changed: 2 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5677.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5677/head:pull/5677 PR: https://git.openjdk.java.net/jdk/pull/5677

2 5

RFR: 8231640: (prop) Canonical property storage
by Jaikiran Pai 28 Sep '21

28 Sep '21

The commit in this PR implements the proposal for enhancement that was discussed in the core-libs-dev mailing list recently[1], for https://bugs.openjdk.java.net/browse/JDK-8231640 At a high level - the `store()` APIs in `Properties` have been modified to now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env variable is set, then instead of writing out the current date time as a date comment, the `store()` APIs instead will use the value set for this env variable to parse it to a `Date` and write out the string form of such a date. The implementation here uses the `d MMM yyyy HH:mm:ss 'GMT'` date format and `Locale.ROOT` to format and write out such a date. This should provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. Furthermore, intentionally, no changes in the date format of the "current date" have been done. These modified `store()` APIs work in the presence of the `SecurityManager` too. The caller is expected to have a read permission on the `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that permission, then the implementation of these `store()` APIs will write out the "current date" and will ignore any value that has been set for the `SOURCE_DATE_EPOCH` env variable. This should allow for backward compatibility of existing applications, where, when they run under a `SecurityManager` and perhaps with an existing restrictive policy file, the presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the `store()` APIs. The modified `store()` APIs will also ignore any value for `SOURCE_DATE_EPOCH` that cannot be parsed to an `long` value. In such cases, the `store()` APIs will write out the "current date" and ignore the value set for this environment variable. No exceptions will be thrown for such invalid values. This is an additional backward compatibility precaution to prevent any rogue value for `SOURCE_DATE_EPOCH` from breaking applications. An additional change in the implementation of these `store()` APIs and unrelated to the date comment, is that these APIs will now write out the property keys in a deterministic order. The keys will be written out in the natural ordering as specified by `java.lang.String#compareTo()` API. The combination of the ordering of the property keys when written out and the usage of `SOURCE_DATE_EPOCH` environment value to determine the date comment should together allow for reproducibility of the output generated by these `store()` APIs. New jtreg test classes have been introduced to verify these changes. The primary focus of `PropertiesStoreTest` is the ordering aspects of the property keys that are written out. On the other hand `StoreReproducibilityTest` focuses on the reproducibility of the output generated by these APIs. The `StoreReproducibilityTest` runs these tests both in the presence and absence of `SecurityManager`. Plus, in the presence of SecurityManager, it tests both the scenarios where the caller is granted the requisite permission and in other case not granted that permission. These new tests and existing tests under `test/jdk/java/util/Properties/` pass with these changes. [1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.ht… [2] https://reproducible-builds.org/specs/source-date-epoch/ ------------- Commit messages: - 8231640: (prop) Canonical property storage Changes: https://git.openjdk.java.net/jdk/pull/5372/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5372&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8231640 Stats: 667 lines in 5 files changed: 663 ins; 1 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/5372.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5372/head:pull/5372 PR: https://git.openjdk.java.net/jdk/pull/5372

15 106

RFR: 8273790: Potential cyclic dependencies between Gregorian and CalendarSystem
by Jaikiran Pai 28 Sep '21

28 Sep '21

Can I please get a review for this change which proposes to fix the issue reported in https://bugs.openjdk.java.net/browse/JDK-8273790? As noted in that issue, trying to class load `sun.util.calendar.CalendarSystem` and `sun.util.calendar.Gregorian` concurrently in separate threads can lead to a deadlock because of the cyclic nature of the code in their static initialization. More specifically, consider this case: - Thread T1 initiates a classload on the `sun.util.calendar.CalendarSystem`. - This gives T1 the implicit class init lock on `CalendarSystem`. - Consider thread T2 which at the same time initiates a classload on `sun.util.calendar.Gregorian` class. - This gives T2 a implicit class init lock on `Gregorian`. - T1, still holding a lock on `CalendarSystem` attempts to load `Gregorian` since it wants to create a (singleton) instance of `Gregorian` and assign it to the `static final GREGORIAN_INSTANCE` member. Since T2 is holding a class init lock on `Gregorian`, T1 ends up waiting - T2 on the other hand is still loading the `Gregorian` class. `Gregorian` itself "is a" `CalendarSystem`, so during this loading of `Gregorian` class, T2 starts travelling up the class hierarchy and asks for a lock on `CalendarSystem`. However T1 is holding this lock and as a result T2 ends up waiting on T1 which is waiting on T2. That triggers this deadlock. The linked JBS issue has a thread dump which shows this in action. The commit here delays the instance creation of `Gregorian` by moving that instance creation logic from the static initialization of the `CalendarSystem` class, to the first call to `CalendarSystem#getGregorianCalendar()`. This prevents the `CalendarSystem` from needing a lock on `Gregorian` during its static init (of course, unless some code in this static init flow calls `CalendarSystem#getGregorianCalendar()`, in which case it is back to square one. I have verified, both manually and through the jtreg test, that the code in question doesn't have such calls) A new jtreg test has been introduced to reproduce the issue and verify the fix. The test in addition to loading these 2 classes in question, also additionally loads a few other classes concurrently. These classes have specific static initialization which leads the calls to `CalendarSystem#getGregorianCalendar()` or `CalendarSystem#forName()`. Including these classes in the tests ensures that this deadlock hasn't "moved" to a different location. I have run multiple runs (approximately 25) of this test with the fix and I haven't seen it deadlock anymore. ------------- Commit messages: - 8273790: Potential cyclic dependencies between Gregorian and CalendarSystem Changes: https://git.openjdk.java.net/jdk/pull/5683/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5683&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273790 Stats: 185 lines in 2 files changed: 181 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/5683.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5683/head:pull/5683 PR: https://git.openjdk.java.net/jdk/pull/5683

4 10

RFR: 8274367: Re-indent stack-trace examples for Throwable.printStackTrace
by Pavel Rappo 27 Sep '21

27 Sep '21

This PR fixes indentation in the examples in the doc comment for java.lang.Throwable#printStackTrace(). 1. Indentation in stack-trace output is significant. The cause of an exception is output on the same level of indentation as that of the exception. A suppressed exception is output at a deeper level of indentation than that of the containing exception. The last example for Throwable.printStackTrace violates this. 2. Indentation in examples for Throwable.printStackTrace that relate to suppressed exceptions is inconsistent with that of cause exceptions. ------------- Commit messages: - Initial commit Changes: https://git.openjdk.java.net/jdk/pull/5715/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5715&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274367 Stats: 21 lines in 1 file changed: 0 ins; 0 del; 21 mod Patch: https://git.openjdk.java.net/jdk/pull/5715.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5715/head:pull/5715 PR: https://git.openjdk.java.net/jdk/pull/5715

5 5