Re: RFR: 8231640: (prop) Canonical property storage [v6]
> 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 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.html > [2] https://reproducible-builds.org/specs/source-date-epoch/ Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: update javadoc to match latest changes - Changes: - all: https://git.openjdk.java.net/jdk/pull/5372/files - new: https://git.openjdk.java.net/jdk/pull/5372/files/7736a8f2..c9d3cb8f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5372=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5372=04-05 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 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
How can I trigger a @Serial warning?
I am trying to give an example of the utility of the @Serial annotation.
But the JDK 17 javac doesn't seem to do anything with it. I tried:
@Serial private void readObject(java.io.ObjectInputStream stream, int
shouldNotHaveThisParam) throws IOException, ClassNotFoundException
@Serial private static final String serialVersionUID = "Fred";
@Serial public String getName() { ... }
Is there some flag that I need to use? I tried -Xlint:serial, but it
didn't make a difference.
Thanks,
Cay
--
Cay S. Horstmann | http://horstmann.com | mailto:c...@horstmann.com
Re: RFR: 8231640: (prop) Canonical property storage [v5]
> 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 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.html > [2] https://reproducible-builds.org/specs/source-date-epoch/ Jaikiran Pai has updated the pull request incrementally with three additional commits since the last revision: - Implement Stuart's suggestion to use Collections instead of Arrays for ordering property keys - update the new tests to match the new date format expectations and also print out some log messages for better debuggability of the tests - Implement Stuart's suggestion on date format: - use the same format for both when writing current date as well as when SOURCE_DATE_EPOCH is set - the format matches the one used in java.util.Date.toString() to preserve backward compatibility - when SOURCE_DATE_EPOCH is set, although the format is the same, the timezone is fixed to UTC and locale is set to ROOT to provide reproducibility of the generated comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/5372/files - new: https://git.openjdk.java.net/jdk/pull/5372/files/867ec999..7736a8f2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5372=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5372=03-04 Stats: 30 lines in 3 files changed: 9 ins; 5 del; 16 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
RFR: 8273514: java/util/DoubleStreamSums/CompensatedSums.java failure
Relaxing some assertion bounds to allow for small error values that still show improvement over previous summation method. - Commit messages: - Dropping unnecessary equals case - Dropping equals zero assert Changes: https://git.openjdk.java.net/jdk/pull/5430/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5430=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273514 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5430.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5430/head:pull/5430 PR: https://git.openjdk.java.net/jdk/pull/5430
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v7]
On Mon, 30 Aug 2021 06:26:01 GMT, Wang Huang wrote: >> Dear all, >> Can you do me a favor to review this patch. This patch use `ldp` to >> implement String.compareTo. >> >> * We add a JMH test case >> * Here is the result of this test case >> >> Benchmark |(size)| Mode| Cnt|Score | Error |Units >> -|--|-||--||- >> StringCompare.compareLL | 64 | avgt| 5 |7.992 | ± 0.005|us/op >> StringCompare.compareLL | 72 | avgt| 5 |15.029| ± 0.006|us/op >> StringCompare.compareLL | 80 | avgt| 5 |14.655| ± 0.011|us/op >> StringCompare.compareLL | 91 | avgt| 5 |16.363| ± 0.12 |us/op >> StringCompare.compareLL | 101 | avgt| 5 |16.966| ± 0.007|us/op >> StringCompare.compareLL | 121 | avgt| 5 |19.276| ± 0.006|us/op >> StringCompare.compareLL | 181 | avgt| 5 |19.002| ± 0.417|us/op >> StringCompare.compareLL | 256 | avgt| 5 |24.707| ± 0.041|us/op >> StringCompare.compareLLWithLdp| 64 | avgt| 5 |8.001 | ± >> 0.121|us/op >> StringCompare.compareLLWithLdp| 72 | avgt| 5 |11.573| ± 0.003|us/op >> StringCompare.compareLLWithLdp| 80 | avgt| 5 |6.861 | ± 0.004|us/op >> StringCompare.compareLLWithLdp| 91 | avgt| 5 |12.774| ± 0.201|us/op >> StringCompare.compareLLWithLdp| 101 | avgt| 5 |8.691 | ± 0.004|us/op >> StringCompare.compareLLWithLdp| 121 | avgt| 5 |11.091| ± 1.342|us/op >> StringCompare.compareLLWithLdp| 181 | avgt| 5 |14.64 | ± 0.581|us/op >> StringCompare.compareLLWithLdp| 256 | avgt| 5 |25.879| ± 1.775|us/op >> StringCompare.compareUU | 64 | avgt| 5 |13.476| ± 0.01 |us/op >> StringCompare.compareUU | 72 | avgt| 5 |15.078| ± 0.006|us/op >> StringCompare.compareUU | 80 | avgt| 5 |23.512| ± 0.011|us/op >> StringCompare.compareUU | 91 | avgt| 5 |24.284| ± 0.008|us/op >> StringCompare.compareUU | 101 | avgt| 5 |20.707| ± 0.017|us/op >> StringCompare.compareUU | 121 | avgt| 5 |29.302| ± 0.011|us/op >> StringCompare.compareUU | 181 | avgt| 5 |39.31 | ± >> 0.016|us/op >> StringCompare.compareUU | 256 | avgt| 5 |54.592| ± 0.392|us/op >> StringCompare.compareUUWithLdp| 64 | avgt| 5 |16.389| ± 0.008|us/op >> StringCompare.compareUUWithLdp| 72 | avgt| 5 |10.71 | ± 0.158|us/op >> StringCompare.compareUUWithLdp| 80 | avgt| 5 |11.488| ± 0.024|us/op >> StringCompare.compareUUWithLdp| 91 | avgt| 5 |13.412| ± 0.006|us/op >> StringCompare.compareUUWithLdp| 101 | avgt| 5 |16.245| ± 0.434|us/op >> StringCompare.compareUUWithLdp| 121 | avgt| 5 |16.597| ± 0.016|us/op >> StringCompare.compareUUWithLdp| 181 | avgt| 5 |27.373| ± 0.017|us/op >> StringCompare.compareUUWithLdp| 256 | avgt| 5 |41.74 | ± 3.5 |us/op >> >> From this table, we can see that in most cases, our patch is better than old >> one. >> >> Thank you for your review. Any suggestions are welcome. > > Wang Huang has updated the pull request incrementally with one additional > commit since the last revision: > > fix windows build failed Marked as reviewed by ngasson (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4722
Re: RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v7]
On Tue, 7 Sep 2021 01:38:02 GMT, Nick Gasson wrote: > Please check the Windows tier1 failure: > https://github.com/Wanghuang-Huawei/jdk/runs/3459332995 > > Seems unlikely that it's anything to do with this patch so you may just want > to re-run it or merge from master. OK, The rerun of presubmit test show that it passed all tests. The result is here: https://github.com/Wanghuang-Huawei/jdk/actions/runs/1181122290 - PR: https://git.openjdk.java.net/jdk/pull/4722
Re: RFR: 8273513: Make java.io.FilterInputStream specification more precise about overrides
On Wed, 8 Sep 2021 22:00:53 GMT, Brian Burkhalter wrote: > Modify the class level specification of `java.io.FilterInputStream` to be > more exact about `java.io.InputStream` methods that it overrides. CSR filed: [JDK-8273517](https://bugs.openjdk.java.net/browse/JDK-8273517). - PR: https://git.openjdk.java.net/jdk/pull/5426
Re: RFR: 8273188: java/lang/instrument/BootClassPath/BootClassPathTest.sh fails with "FATAL ERROR in native method: processing of -javaagent failed, processJavaStart failed"
On Wed, 8 Sep 2021 23:38:38 GMT, David Holmes wrote: > Pre-existing: The initialization logic in this code is quite odd for the case > when no conversion is necessary (we call `utfInitialize` on every call to > `convertUtf8ToPlatformString`!), but I assume we do not call > `appendBootClassPath` and `appendToClassLoaderSearch` very often? Indeed. I assume those methods are only used on agent startup. - PR: https://git.openjdk.java.net/jdk/pull/5427
Re: RFR: 8273188: java/lang/instrument/BootClassPath/BootClassPathTest.sh fails with "FATAL ERROR in native method: processing of -javaagent failed, processJavaStart failed"
On Wed, 8 Sep 2021 22:15:12 GMT, Naoto Sato wrote: > The gist of the issue is that the test case now always creates the boot > classpath with non-ASCII chars appended, because the default encoding is > fixed to UTF-8 with the fix to JDK-8260265. > > On macOS, javaagent tries to load the class with US-ASCII determined by > nl_langinfo() (in unix/.../EncodingSupport_md.c). The file system encoding is > always UTF-8 on mac, so no need to use the determined encoding by > nl_langinfo(). > > On Linux, the "defaultEncoding" in Setup.java used to be US-ASCII in "C" > locale (which matches the result from nl_langinfo()), so no non-ASCII suffix > was appended to the `boot` path. But now the "defaultEncoding" is always > UTF-8, the setup code appends the non-ASCII suffix, which fails to read in > the native code using iconv with US-ASCII. The setup code should use the > encoding from "sun.jnu.encoding" instead. Actually, the code there was copied > from UnicodeTest.java which was modified with JDK-8260265, so the same fix > needs to be applied. > > Tier5 run succeeds with one unrelated failure. Also, fixed some typos, e.g. > "Uft8" -> "Utf8" Hi Naoto, Based on what you've said the core functional change seems okay. Good catch on the uft typo! Pre-existing: The initialization logic in this code is quite odd for the case when no conversion is necessary (we call `utfInitialize` on every call to `convertUtf8ToPlatformString`!), but I assume we do not call `appendBootClassPath` and `appendToClassLoaderSearch` very often? Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5427
Re: RFR: 8273487: Zero: Handle "zero" variant in runtime tests [v2]
On Wed, 8 Sep 2021 13:25:27 GMT, Aleksey Shipilev wrote: >> JDK-8179317 rewritten runtime shell tests to Java. There is a little >> omission in VM variant selection, which makes Zero fail some of the tier1 >> tests, like: >> >> >> $ CONF=linux-x86_64-zero-fastdebug make exploded-test >> TEST=runtime/StackGap/TestStackGap.java >> >> STDERR: >> java.lang.Error: TESTBUG: unsupported vm variant >> at jdk.test.lib.Platform.variant(Platform.java:368) >> at jdk.test.lib.Platform.jvmLibDir(Platform.java:357) >> at jdk.test.lib.process.ProcessTools.addJvmLib(ProcessTools.java:585) >> at >> jdk.test.lib.process.ProcessTools.createNativeTestProcessBuilder(ProcessTools.java:575) >> >> >> >> Additional testing: >> - [x] Linux x86_64 Zero affected tests (StackGap, StackGuardPages, TestTLS) >> now run > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Build system awkwardness ensues Marked as reviewed by dholmes (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5413
Re: RFR: 8273188: java/lang/instrument/BootClassPath/BootClassPathTest.sh fails with "FATAL ERROR in native method: processing of -javaagent failed, processJavaStart failed"
On Wed, 8 Sep 2021 22:15:12 GMT, Naoto Sato wrote: > The gist of the issue is that the test case now always creates the boot > classpath with non-ASCII chars appended, because the default encoding is > fixed to UTF-8 with the fix to JDK-8260265. > > On macOS, javaagent tries to load the class with US-ASCII determined by > nl_langinfo() (in unix/.../EncodingSupport_md.c). The file system encoding is > always UTF-8 on mac, so no need to use the determined encoding by > nl_langinfo(). > > On Linux, the "defaultEncoding" in Setup.java used to be US-ASCII in "C" > locale (which matches the result from nl_langinfo()), so no non-ASCII suffix > was appended to the `boot` path. But now the "defaultEncoding" is always > UTF-8, the setup code appends the non-ASCII suffix, which fails to read in > the native code using iconv with US-ASCII. The setup code should use the > encoding from "sun.jnu.encoding" instead. Actually, the code there was copied > from UnicodeTest.java which was modified with JDK-8260265, so the same fix > needs to be applied. > > Tier5 run succeeds with one unrelated failure. Also, fixed some typos, e.g. > "Uft8" -> "Utf8" Hi David, > Doesn't it depend on the OS version and which file system is being used? HFS+ > seems to use UTF-16 Right. I was not clear enough as to `file system encoding`, what I meant was the `sun.jnu.encoding`, which is always `UTF-8` with this change: https://bugs.openjdk.java.net/browse/JDK-8003228 - PR: https://git.openjdk.java.net/jdk/pull/5427
Re: RFR: 8273188: java/lang/instrument/BootClassPath/BootClassPathTest.sh fails with "FATAL ERROR in native method: processing of -javaagent failed, processJavaStart failed"
On Wed, 8 Sep 2021 22:15:12 GMT, Naoto Sato wrote: > The gist of the issue is that the test case now always creates the boot > classpath with non-ASCII chars appended, because the default encoding is > fixed to UTF-8 with the fix to JDK-8260265. > > On macOS, javaagent tries to load the class with US-ASCII determined by > nl_langinfo() (in unix/.../EncodingSupport_md.c). The file system encoding is > always UTF-8 on mac, so no need to use the determined encoding by > nl_langinfo(). > > On Linux, the "defaultEncoding" in Setup.java used to be US-ASCII in "C" > locale (which matches the result from nl_langinfo()), so no non-ASCII suffix > was appended to the `boot` path. But now the "defaultEncoding" is always > UTF-8, the setup code appends the non-ASCII suffix, which fails to read in > the native code using iconv with US-ASCII. The setup code should use the > encoding from "sun.jnu.encoding" instead. Actually, the code there was copied > from UnicodeTest.java which was modified with JDK-8260265, so the same fix > needs to be applied. > > Tier5 run succeeds with one unrelated failure. Also, fixed some typos, e.g. > "Uft8" -> "Utf8" Hi Naoto, > The file system encoding is always UTF-8 on mac, so no need to use the > determined encoding by nl_langinfo(). Doesn't it depend on the OS version and which file system is being used? HFS+ seems to use UTF-16 Thanks, David - PR: https://git.openjdk.java.net/jdk/pull/5427
RFR: 8273188: java/lang/instrument/BootClassPath/BootClassPathTest.sh fails with "FATAL ERROR in native method: processing of -javaagent failed, processJavaStart failed"
The gist of the issue is that the test case now always creates the boot classpath with non-ASCII chars appended, because the default encoding is fixed to UTF-8 with the fix to JDK-8260265. On macOS, javaagent tries to load the class with US-ASCII determined by nl_langinfo() (in unix/.../EncodingSupport_md.c). The file system encoding is always UTF-8 on mac, so no need to use the determined encoding by nl_langinfo(). On Linux, the "defaultEncoding" in Setup.java used to be US-ASCII in "C" locale (which matches the result from nl_langinfo()), so no non-ASCII suffix was appended to the `boot` path. But now the "defaultEncoding" is always UTF-8, the setup code appends the non-ASCII suffix, which fails to read in the native code using iconv with US-ASCII. The setup code should use the encoding from "sun.jnu.encoding" instead. Actually, the code there was copied from UnicodeTest.java which was modified with JDK-8260265, so the same fix needs to be applied. Tier5 run succeeds with one unrelated failure. Also, fixed some typos, e.g. "Uft8" -> "Utf8" - Commit messages: - Merge branch 'master' into JDK-8273188 - 8273188: java/lang/instrument/BootClassPath/BootClassPathTest.sh fails with "FATAL ERROR in native method: processing of -javaagent failed, processJavaStart failed" Changes: https://git.openjdk.java.net/jdk/pull/5427/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5427=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273188 Stats: 36 lines in 8 files changed: 5 ins; 16 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/5427.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5427/head:pull/5427 PR: https://git.openjdk.java.net/jdk/pull/5427
Re: RFR: 8273513: Make java.io.FilterInputStream specification more precise about overrides
On Wed, 8 Sep 2021 22:00:53 GMT, Brian Burkhalter wrote: > Modify the class level specification of `java.io.FilterInputStream` to be > more exact about `java.io.InputStream` methods that it overrides. Some other incidental modifications are made in passing, principally adding `@Override` annotations and putting statements like `This method does X` in `@implSpec` blocks. - PR: https://git.openjdk.java.net/jdk/pull/5426
RFR: 8273513: Make java.io.FilterInputStream specification more precise about overrides
Modify the class level specification of `java.io.FilterInputStream` to be more exact about `java.io.InputStream` methods that it overrides. - Commit messages: - 8273513: Make java.io.FilterInputStream specification more precise about overrides Changes: https://git.openjdk.java.net/jdk/pull/5426/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5426=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273513 Stats: 53 lines in 1 file changed: 21 ins; 9 del; 23 mod Patch: https://git.openjdk.java.net/jdk/pull/5426.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5426/head:pull/5426 PR: https://git.openjdk.java.net/jdk/pull/5426
Re: RFR: 8266936: Add a finalization JFR event [v10]
On Fri, 27 Aug 2021 15:23:35 GMT, Markus Grönlund wrote:
>> Greetings,
>>
>> Object.finalize() was deprecated in JDK9. There is an ongoing effort to
>> replace and mitigate Object.finalize() uses in the JDK libraries; please see
>> https://bugs.openjdk.java.net/browse/JDK-8253568 for more information.
>>
>> We also like to assist users in replacing and mitigating uses in non-JDK
>> code.
>>
>> Hence, this changeset adds a periodic JFR event to help identify which
>> classes are overriding Object.finalize().
>>
>> Thanks
>> Markus
>
> Markus Grönlund has updated the pull request incrementally with three
> additional commits since the last revision:
>
> - localize
> - cleanup
> - FinalizerStatistics
I have some general comments and questions about this code.
src/hotspot/share/services/classLoadingService.cpp line 80:
> 78:
> 79: size_t ClassLoadingService::compute_class_size(InstanceKlass* k) {
> 80: // lifted from ClassStatistics.do_class(Klass* k)
Can you remove this line? I think that function is gone now.
src/hotspot/share/services/finalizerService.cpp line 44:
> 42: _ik(ik),
> 43: _objects_on_heap(0),
> 44: _total_finalizers_run(0) {}
Is this hashtable for every InstanceKlass that defines a finalizer? How many
are there generally? Why couldn't this be a simple hashtable like
ResourceHashtable (soon to be renamed) which you can write in only a few lines
of code?
src/hotspot/share/services/finalizerService.cpp line 114:
> 112:
> 113: static inline void added() {
> 114: set_atomic(&_count);
Why isn't Atomic::inc() good enough here? It's used everywhere else.
src/hotspot/share/services/finalizerService.cpp line 123:
> 121: static inline uintx hash_function(const InstanceKlass* ik) {
> 122: assert(ik != nullptr, "invariant");
> 123: return primitive_hash(ik);
If the hash function is primitive_hash, this hash is good enough to not need
rehashing. The only reason for the rehashing for symbol and string table was
that the char* had a dumb hash that was faster but could be hacked, so it
needed to become a different hashcode. This doesn't need rehashing.
src/hotspot/share/services/finalizerService.cpp line 485:
> 483: void FinalizerService::purge_unloaded() {
> 484: assert_locked_or_safepoint(ClassLoaderDataGraph_lock);
> 485: ClassLoaderDataGraph::classes_unloading_do(_unloading);
Since you remove entries on unloading, I don't see any reason to have any
concurrent cleanup.
You do however need in the lookup code, some code that doesn't find the
InstanceKlass if !ik->is_loader_alive()
-
PR: https://git.openjdk.java.net/jdk/pull/4731
Integrated: 8273242: (test) Refactor to use TestNG for RuntimeTests ExecCommand tests
On Wed, 1 Sep 2021 16:37:24 GMT, Roger Riggs wrote: > The ExecCommand test of Runtime.exec is difficult to maintain; the parallel > arrays are hard to keep in sync. > This cleanup converts to use TestNG DataProviders and other improvements. This pull request has now been integrated. Changeset: 7fd6b0bf Author:Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/7fd6b0bfd8ab3c64b374c71010bdfa369f0c67e8 Stats: 288 lines in 1 file changed: 149 ins; 105 del; 34 mod 8273242: (test) Refactor to use TestNG for RuntimeTests ExecCommand tests Reviewed-by: naoto, lancea - PR: https://git.openjdk.java.net/jdk/pull/5335
Withdrawn: 8255878: FilterInputStream is missing implementations of Java 9 InputStream methods
On Fri, 3 Sep 2021 22:29:19 GMT, Brian Burkhalter wrote: > This request proposes to modify `java.io.FilterInputStream` to override > `readAllBytes()`, `readNBytes(int)`, `skipNBytes(long)`, and > `transferTo(OutputStream)` in order to leverage any performance advantage > that the wrapped stream might have over the `java.io.InputStream` > implementations of these methods. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/5367
Re: RFR: 8255878: FilterInputStream is missing implementations of Java 9 InputStream methods [v2]
On Fri, 3 Sep 2021 23:19:22 GMT, Brian Burkhalter wrote: >> This request proposes to modify `java.io.FilterInputStream` to override >> `readAllBytes()`, `readNBytes(int)`, `skipNBytes(long)`, and >> `transferTo(OutputStream)` in order to leverage any performance advantage >> that the wrapped stream might have over the `java.io.InputStream` >> implementations of these methods. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8255878: Add @implSpec where appropriate Created [JDK-8273513](https://bugs.openjdk.java.net/browse/JDK-8273513). Closing this ill-conceived PR as being of too little value compared to the risk of subclass breakage. - PR: https://git.openjdk.java.net/jdk/pull/5367
Re: RFR: 8273430: Suspicious duplicate condition in java.util.regex.Grapheme#isExcludedSpacingMark
On Wed, 8 Sep 2021 20:24:31 GMT, Ian Graves wrote: > The duplicate condition in this chain of expressions needs to be shrunk to > drop a couple of character that are not excluded spacing marks. The copyright year in Grapheme.java should be 2021, otherwise looks good. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5425
RFR: 8273430: Suspicious duplicate condition in java.util.regex.Grapheme#isExcludedSpacingMark
The duplicate condition in this chain of expressions needs to be shrunk to drop a couple of character that are not excluded spacing marks. - Commit messages: - 8273430: Suspicious duplicate condition in java.util.regex.Grapheme#isExcludedSpacingMark Changes: https://git.openjdk.java.net/jdk/pull/5425/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5425=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273430 Stats: 10 lines in 2 files changed: 7 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/5425.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5425/head:pull/5425 PR: https://git.openjdk.java.net/jdk/pull/5425
Confusing javadoc on a method java.lang.StringBuilder#readObject
Hello. I found a confusing javadoc of the method java.lang.StringBuilder#readObject: "readObject is called to restore the state of the StringBuffer from a stream." I believe there should be "StringBuilder" instead of "StringBuffer". Andrey Turbanov
Re: RFR: 8273242: (test) Refactor to use TestNG for RuntimeTests ExecCommand tests [v2]
On Wed, 8 Sep 2021 19:31:46 GMT, Roger Riggs wrote: >> The ExecCommand test of Runtime.exec is difficult to maintain; the parallel >> arrays are hard to keep in sync. >> This cleanup converts to use TestNG DataProviders and other improvements. > > Roger Riggs has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Simplify file deletion >Add enum to document test modes. > - Merge branch 'master' into 8273242-execcommand-refactor > - 8273242: Refactored ExecCommand to use TestNG DataProvider for cases Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5335
Re: RFR: 8273242: (test) Refactor to use TestNG for RuntimeTests ExecCommand tests [v2]
On Wed, 8 Sep 2021 19:31:46 GMT, Roger Riggs wrote: >> The ExecCommand test of Runtime.exec is difficult to maintain; the parallel >> arrays are hard to keep in sync. >> This cleanup converts to use TestNG DataProviders and other improvements. > > Roger Riggs has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Simplify file deletion >Add enum to document test modes. > - Merge branch 'master' into 8273242-execcommand-refactor > - 8273242: Refactored ExecCommand to use TestNG DataProvider for cases Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5335
Re: RFR: 8273242: (test) Refactor to use TestNG for RuntimeTests ExecCommand tests [v2]
On Tue, 7 Sep 2021 22:01:54 GMT, Lance Andersen wrote:
>> Roger Riggs has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains three additional
>> commits since the last revision:
>>
>> - Simplify file deletion
>>Add enum to document test modes.
>> - Merge branch 'master' into 8273242-execcommand-refactor
>> - 8273242: Refactored ExecCommand to use TestNG DataProvider for cases
>
> test/jdk/java/lang/RuntimeTests/exec/ExecCommand.java line 107:
>
>> 105: private static void deleteOut(String path) {
>> 106: try {
>> 107: Files.delete(FileSystems.getDefault().getPath(path));
>
> More of a curious question, is there a reason you couldn't use
> Files::deleteIfExists or Path.of() instead of FileSystems.getDefault
That code is pre-existing. Your suggestion can completely replace the
deleteOut method. Tnx
-
PR: https://git.openjdk.java.net/jdk/pull/5335
Re: RFR: 8273242: (test) Refactor to use TestNG for RuntimeTests ExecCommand tests [v2]
> The ExecCommand test of Runtime.exec is difficult to maintain; the parallel > arrays are hard to keep in sync. > This cleanup converts to use TestNG DataProviders and other improvements. Roger Riggs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Simplify file deletion Add enum to document test modes. - Merge branch 'master' into 8273242-execcommand-refactor - 8273242: Refactored ExecCommand to use TestNG DataProvider for cases - Changes: - all: https://git.openjdk.java.net/jdk/pull/5335/files - new: https://git.openjdk.java.net/jdk/pull/5335/files/d374449b..8dbd021e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5335=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5335=00-01 Stats: 35582 lines in 1179 files changed: 24923 ins; 5611 del; 5048 mod Patch: https://git.openjdk.java.net/jdk/pull/5335.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5335/head:pull/5335 PR: https://git.openjdk.java.net/jdk/pull/5335
Re: RFR: 8231640: (prop) Canonical property storage [v3]
On Wed, 8 Sep 2021 09:32:55 GMT, Jaikiran Pai wrote:
>> Jaikiran Pai has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - adjust testcases to verify the new semantics
>> - implement review suggestions:
>> - Use doPriveleged instead of explicit permission checks, to reduce
>> complexity
>> - Use DateTimeFormatter and ZonedDateTime instead of Date.toString()
>> - Use DateTimeFormatter.RFC_1123_DATE_TIME for formatting
>> SOURCE_DATE_EPOCH dates
>> - Use Arrays.sort instead of a TreeMap for ordering property keys
>
> src/java.base/share/classes/java/util/Properties.java line 963:
>
>> 961: synchronized (this) {
>> 962: var entries = map.entrySet().toArray(new Map.Entry> ?>[0]);
>> 963: Arrays.sort(entries, new Comparator>() {
>
> This part here, intentionally doesn't use a lambda, since from what I
> remember seeing in some mail discussion, it was suggested that using lambda
> in core parts which get used very early during JVM boostrap, should be
> avoided. If that's not a concern here, do let me know and I can change it to
> a lambda.
This is a fair concern, but writing out a properties file is a pretty
high-level operation that doesn't seem likely to be used during bootstrap.
Also, I observe that the `doPrivileged` block above uses a lambda. So I think
we're probably ok to use a lambda here. But if you get an inexplicable error at
build time or at startup time, this would be the reason why.
Assuming we're ok with lambdas, it might be easier to use collections instead
of arrays in order to preserve generic types. Unfortunately the map is
`Map` so we have to do some fancy casting to get the right
type. But then we can use `Map.Entry.comparingByKey()` as the comparator. (Note
that this uses lambda internally.)
Something like this might work:
@SuppressWarnings("unchecked")
var entries = new ArrayList<>(((Map)(Map)map).entrySet());
entries.sort(Map.Entry.comparingByKey());
-
PR: https://git.openjdk.java.net/jdk/pull/5372
Re: [External] : Re: RFR: 8231640: (prop) Canonical property storage
Unless there's an overriding reason, it might be nice to have the output format match the format used in the Debian patch that adds SOURCE_DATE_EPOCH: https://salsa.debian.org/openjdk-team/openjdk/-/blob/master/debian/patches/reproducible-properties-timestamp.diff So the current patch implementation uses the format "d MMM HH:mm:ss 'GMT'", with a Locale.ROOT (for locale neutral formatting). I chose this format since that was the one that the (deprecated) java.util.Date#toGMTString() was using. Roger's suggestion is to use DateTimeFormatter#RFC_1123_DATE_TIME date format which is "dow, d MMM HH:mm:ss GMT" (where dow == day of week) IMO, either of these formats are "well known", since they are/were used within the JDK, especially the DateTimeFormatter#RFC_1123_DATE_TIME which Roger suggested, since that's even a public spec. The one in the debian patch is "-MM-dd HH:mm:ss z" which although is fine to use, it however feels a bit "less known". I was leaning towards Roger's suggestion to use the RFC_1123_DATE_TIME in my upcoming patch update. Is there a reason why the one in debian's patch is preferable compared to a spec backed format? My point in bringing this is up is to consider interoperability. I don't have a strong preference over the particular date format. As far as I can see, there are currently two behaviors "in the wild": 1) Baseline OpenJDK 17 behavior: dow mon dd hh:mm:ss zzz This is the behavior provided by "new Date().toString()" and has likely not changed in many years. Of course, the actual values reflect the current time and locale, which hurts reproducibility, but the format itself hasn't changed. 2) Debian's OpenJDK with SOURCE_DATE_EPOCH set: -MM-dd HH:mm:ss z The question is, what format should the JDK-8231640 use? I had said earlier that it might be a good idea to match the Debian format. But thinking about this further, I think sticking with the original JDK format would be preferable. The Debian change is after all an outlier. So the more specific question is, should we try to continue with the original JDK format or choose a format that's "better" in some sense? It seems to me that if there's something out there that parses the date from a properties file, we'd want to avoid breaking this code if the environment variable is set. So maybe stick with the original format in all cases. But of course for reproducibility use the epoch value from the environment and set the locale and zone offset to known values. s'marks
Re: RFR: 8273278: Support XSLT on GraalVM Native Image--deterministic bytecode generation in XSLT
On Wed, 1 Sep 2021 13:28:34 GMT, Jovan Stevanovic wrote: > GraalVM Native Image supports loading classes at runtime if they are known > during image build (class predefinition). This is achieved by the JVMTI agent > that registers dynamically generated classes in a regular HotSpot run. The > Native Image build uses these registered classes to embed them into the > produced binary so they can be loaded at runtime. Loading at runtime is > achieved by matching the unique hash of generated classes. > > If the generated bytecode is unstable across runs, the generated native image > can not match the hash of the runtime-generated bytecode to the pre-defined > classes. The execution failure happens here: > > https://github.com/oracle/graal/blob/master/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/PredefinedClassesSupport.java#L127-L131. > > In XSLT the produced bytecode is unstable for the following reasons: > > - Methods like ` HashMap#values` and `HashMap#keySet` result in different > traversal orders of its elements yielding a different order of methods and > fields. > > - The default `Object#toString` includes the current memory reference of > `com.sun.org.apache.xalan.internal.xsltc.compiler.Number` in the generated > class. I'm not involved in any older releases. You'll need to find the right channel for a particular backport you want to make and ask there. As for merging the change in the current release, please update the copyright header and the LastModified date for each of the classes. The github interface unfortunately won't allow me to add comments directly to the unchanged code area. Once that's done, you'll need to issue an integrate command. I'll sponsor your change. - PR: https://git.openjdk.java.net/jdk/pull/5331
Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v6]
On Wed, 8 Sep 2021 17:40:29 GMT, Naoto Sato wrote: >> Please review the fix to the issue. Avoiding overflow by not calling >> nanosUntil() directly, which will overflow beyond Long.MAX_VALUE difference >> in nano unit. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Corrected the calculation for MILLIS as well. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5396
Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v6]
On Wed, 8 Sep 2021 17:40:29 GMT, Naoto Sato wrote: >> Please review the fix to the issue. Avoiding overflow by not calling >> nanosUntil() directly, which will overflow beyond Long.MAX_VALUE difference >> in nano unit. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Corrected the calculation for MILLIS as well. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5396
Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]
On Wed, 8 Sep 2021 15:35:27 GMT, Stephen Colebourne wrote: > `toEpochMilli()` overflows at large/small values. Thus any attempt to > calculate the difference between two very large instants would fail. Thanks. Fixed. - PR: https://git.openjdk.java.net/jdk/pull/5396
Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v6]
> Please review the fix to the issue. Avoiding overflow by not calling > nanosUntil() directly, which will overflow beyond Long.MAX_VALUE difference > in nano unit. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Corrected the calculation for MILLIS as well. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5396/files - new: https://git.openjdk.java.net/jdk/pull/5396/files/ccf73bf7..1f6fd470 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5396=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5396=04-05 Stats: 24 lines in 3 files changed: 21 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/5396.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5396/head:pull/5396 PR: https://git.openjdk.java.net/jdk/pull/5396
Integrated: 8273450: Fix the copyright header of SVML files
On Tue, 7 Sep 2021 20:25:25 GMT, Sandhya Viswanathan wrote: > Fix the copyright header of SVML files to match others. > > This was brought up on jdk-dev mailing list: > https://mail.openjdk.java.net/pipermail/jdk-dev/2021-September/005992.html This pull request has now been integrated. Changeset: d7efd0e8 Author:Sandhya Viswanathan URL: https://git.openjdk.java.net/jdk/commit/d7efd0e8cf14c732427d2c1363b60278bebdc06a Stats: 288 lines in 72 files changed: 144 ins; 0 del; 144 mod 8273450: Fix the copyright header of SVML files Reviewed-by: dholmes, psandoz - PR: https://git.openjdk.java.net/jdk/pull/5399
Re: RFR: 8273450: Fix the copyright header of SVML files
On Wed, 8 Sep 2021 02:03:12 GMT, Paul Sandoz wrote: >> Fix the copyright header of SVML files to match others. >> >> This was brought up on jdk-dev mailing list: >> https://mail.openjdk.java.net/pipermail/jdk-dev/2021-September/005992.html > > Marked as reviewed by psandoz (Reviewer). Thanks a lot @PaulSandoz for the review. - PR: https://git.openjdk.java.net/jdk/pull/5399
Integrated: 8273329: Remove redundant null check from String.getBytes(String charsetName)
On Fri, 3 Sep 2021 13:22:54 GMT, Сергей Цыпанов
wrote:
> Current implementation looks like this:
>
> public byte[] getBytes(String charsetName)
> throws UnsupportedEncodingException {
> if (charsetName == null) throw new NullPointerException();
> return encode(lookupCharset(charsetName), coder(), value);
> }
>
> Null check seems to be redundant here because the same check of `charsetName`
> is done within `String.lookupCharset(String)`:
>
> private static Charset lookupCharset(String csn) throws
> UnsupportedEncodingException {
> Objects.requireNonNull(csn);
> try {
> return Charset.forName(csn);
> } catch (UnsupportedCharsetException | IllegalCharsetNameException x) {
> throw new UnsupportedEncodingException(csn);
> }
> }
This pull request has now been integrated.
Changeset: e5f298a7
Author:Sergey Tsypanov
Committer: Naoto Sato
URL:
https://git.openjdk.java.net/jdk/commit/e5f298a7f1f3106b72e43c152c090af1657485f0
Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
8273329: Remove redundant null check from String.getBytes(String charsetName)
Reviewed-by: rriggs, iris, naoto
-
PR: https://git.openjdk.java.net/jdk/pull/5361
Integrated: 8269373: some tests in jdk/tools/launcher/ fails on localized Windows platform
On Fri, 25 Jun 2021 12:10:18 GMT, Masanori Yano wrote: > Hi all, > > Could you please review the 8269373 bug fixes? > > These tests call java.lang.ProcessBuilder in direct, so not used jtreg > command option. To run non-localized tests, -Duser.language=en and > -Duser.country=US options should be added in ProcessBuilder. This pull request has now been integrated. Changeset: cb112aff Author:Masanori Yano Committer: Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/cb112affd6061e8ace6dad4e92c7b394a413e37f Stats: 14 lines in 2 files changed: 12 ins; 0 del; 2 mod 8269373: some tests in jdk/tools/launcher/ fails on localized Windows platform Reviewed-by: naoto - PR: https://git.openjdk.java.net/jdk/pull/4594
Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]
On Wed, 8 Sep 2021 00:36:29 GMT, Naoto Sato wrote: >> Please review the fix to the issue. Avoiding overflow by not calling >> nanosUntil() directly, which will overflow beyond Long.MAX_VALUE difference >> in nano unit. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Added comments for the test. `toEpochMilli()` overflows at large/small values. Thus any attempt to calculate the difference between two very large instants would fail. - PR: https://git.openjdk.java.net/jdk/pull/5396
Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]
On Wed, 8 Sep 2021 13:58:59 GMT, Stephen Colebourne wrote: > This change looks fine, but I think you also need a `millisUntil` private > method to fix the identical overflow problem with millis (which might as well > be fixed now). `until()` for millis simply subtracts its `toEpochMilli()` from the end Instant's, so I am not sure that would cause the same false `ArithmeticException`. Can you please elaborate more? - PR: https://git.openjdk.java.net/jdk/pull/5396
Re: RFR: 8273315: Parallelize and increase timeouts for java/foreign/TestMatrix.java test [v2]
On Wed, 8 Sep 2021 09:17:31 GMT, Aleksey Shipilev wrote: >> This test runs a lot of configurations, and spends a lot of time serially. >> This is especially pronounced when run in prospective tier4 runs >> (JDK-8273314). There are reports of multi-hour runs (see JDK-8271613). We >> can parallelize the test configurations for this test to make it hurt less. >> Also, timeouts need to be increased for `TestUpcall` test configs, because >> some of them are very slow in fastdebug mode. >> >> Sample run: >> >> >> $ time CONF=linux-x86_64-server-fastdebug make run-test >> TEST=java/foreign/TestMatrix.java | ts -s >> 00:00:00 Building target 'run-test' in configuration >> 'linux-x86_64-server-fastdebug' >> 00:00:03 Test selection 'java/foreign/TestMatrix.java', will run: >> 00:00:03 * jtreg:test/jdk/java/foreign/TestMatrix.java >> 00:00:03 >> 00:00:03 Running test 'jtreg:test/jdk/java/foreign/TestMatrix.java' >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTF >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFFT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTTF >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTF >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTF >> 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFF >> 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFF >> 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTT >> 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFF >> 00:00:38 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFT >> 00:01:50 Passed: java/foreign/TestMatrix.java#Downcall-FF >> 00:02:27 Passed: java/foreign/TestMatrix.java#Downcall-TF >> 00:03:03 Passed: java/foreign/TestMatrix.java#Downcall-FT >> 00:03:47 Passed: java/foreign/TestMatrix.java#Downcall-TT >> 00:04:17 Passed: java/foreign/TestMatrix.java#Upcall-FTFF >> 00:04:23 Passed: java/foreign/TestMatrix.java#Upcall-TFFF >> 00:05:46 Passed: java/foreign/TestMatrix.java#Upcall-TTFF >> 00:06:03 Passed: java/foreign/TestMatrix.java#Upcall-TFFT >> 00:06:44 Passed: java/foreign/TestMatrix.java#Upcall-FTFT >> 00:08:24 Passed: java/foreign/TestMatrix.java#Upcall-TFTF >> 00:08:39 Passed: java/foreign/TestMatrix.java#Upcall-TTFT >> 00:09:16 Passed: java/foreign/TestMatrix.java#Upcall-FTTF >> 00:09:19 Passed: java/foreign/TestMatrix.java#Upcall-TFTT >> 00:10:01 Passed: java/foreign/TestMatrix.java#Upcall-FTTT >> 00:10:37 Passed: java/foreign/TestMatrix.java#Upcall-TTTF >> 00:12:16 Passed: java/foreign/TestMatrix.java#Upcall- >> 00:12:17 Test results: passed: 32 >> 00:12:21 >> 00:12:21 == >> 00:12:21 Test summary >> 00:12:21 == >> 00:12:21TEST TOTAL PASS >> FAIL ERROR >> 00:12:21jtreg:test/jdk/java/foreign/TestMatrix.java 3232 >> 0 0 >> 00:12:21 == >> >> real 12m20.538s >> user 131m54.043s >> sys 0m59.944s >> >> >> If we don't parallelize, then those 130 minutes are spent serially. > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Go "manual" I am fine with this solution - it solves my issue. - PR: https://git.openjdk.java.net/jdk/pull/5358
Re: RFR: 8273315: Parallelize and increase timeouts for java/foreign/TestMatrix.java test
On Wed, 8 Sep 2021 11:06:50 GMT, Arno Zeller wrote: > Probably it could be a better solution to add the stress keyword for slow > running or high load tests - like it is done in the hotspot part of the > tests. Then automated test run can exclude or select these test by keyword. Thought so too, but I think Maurizio wants this test to run only explicitly by those who want to run it. So there needs to be something that disallows automatic runs completely. "manual" seems to be good at this. Do you agree? If you do, I'll integrate. - PR: https://git.openjdk.java.net/jdk/pull/5358
Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]
On Wed, 8 Sep 2021 00:36:29 GMT, Naoto Sato wrote: >> Please review the fix to the issue. Avoiding overflow by not calling >> nanosUntil() directly, which will overflow beyond Long.MAX_VALUE difference >> in nano unit. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Added comments for the test. This change looks fine, but I think you also need a `millisUntil` private method to fix the identical overflow problem with millis (which might as well be fixed now). - PR: https://git.openjdk.java.net/jdk/pull/5396
Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]
On Wed, 8 Sep 2021 00:36:29 GMT, Naoto Sato wrote: >> Please review the fix to the issue. Avoiding overflow by not calling >> nanosUntil() directly, which will overflow beyond Long.MAX_VALUE difference >> in nano unit. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Added comments for the test. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5396
Re: RFR: 8273487: Zero: Handle "zero" variant in runtime tests
On Wed, 8 Sep 2021 10:57:33 GMT, Aleksey Shipilev wrote: > JDK-8179317 rewritten runtime shell tests to Java. There is a little omission > in VM variant selection, which makes Zero fail some of the tier1 tests, like: > > > $ CONF=linux-x86_64-zero-fastdebug make exploded-test > TEST=runtime/StackGap/TestStackGap.java > > STDERR: > java.lang.Error: TESTBUG: unsupported vm variant > at jdk.test.lib.Platform.variant(Platform.java:368) > at jdk.test.lib.Platform.jvmLibDir(Platform.java:357) > at jdk.test.lib.process.ProcessTools.addJvmLib(ProcessTools.java:585) > at > jdk.test.lib.process.ProcessTools.createNativeTestProcessBuilder(ProcessTools.java:575) > > > > Additional testing: > - [x] Linux x86_64 Zero affected tests (StackGap, StackGuardPages, TestTLS) > now run Huh, it turns out there is the awkwardness in the build system that puts product `libjvm.so` to `server` for Zero. See [JDK-8273494](https://bugs.openjdk.java.net/browse/JDK-8273494). This was not the cause for gtest `libjvm.so`, which is in `zero` folder for Zero. So I had to make it more awkward until the build system fix is here. See new commit. - PR: https://git.openjdk.java.net/jdk/pull/5413
Re: RFR: 8273487: Zero: Handle "zero" variant in runtime tests [v2]
> JDK-8179317 rewritten runtime shell tests to Java. There is a little omission > in VM variant selection, which makes Zero fail some of the tier1 tests, like: > > > $ CONF=linux-x86_64-zero-fastdebug make exploded-test > TEST=runtime/StackGap/TestStackGap.java > > STDERR: > java.lang.Error: TESTBUG: unsupported vm variant > at jdk.test.lib.Platform.variant(Platform.java:368) > at jdk.test.lib.Platform.jvmLibDir(Platform.java:357) > at jdk.test.lib.process.ProcessTools.addJvmLib(ProcessTools.java:585) > at > jdk.test.lib.process.ProcessTools.createNativeTestProcessBuilder(ProcessTools.java:575) > > > > Additional testing: > - [x] Linux x86_64 Zero affected tests (StackGap, StackGuardPages, TestTLS) > now run Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision: Build system awkwardness ensues - Changes: - all: https://git.openjdk.java.net/jdk/pull/5413/files - new: https://git.openjdk.java.net/jdk/pull/5413/files/141d8184..40998df5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5413=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5413=00-01 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5413.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5413/head:pull/5413 PR: https://git.openjdk.java.net/jdk/pull/5413
Re: RFR: 8273487: Zero: Handle "zero" variant in runtime tests
On Wed, 8 Sep 2021 10:57:33 GMT, Aleksey Shipilev wrote: > JDK-8179317 rewritten runtime shell tests to Java. There is a little omission > in VM variant selection, which makes Zero fail some of the tier1 tests, like: > > > $ CONF=linux-x86_64-zero-fastdebug make exploded-test > TEST=runtime/StackGap/TestStackGap.java > > STDERR: > java.lang.Error: TESTBUG: unsupported vm variant > at jdk.test.lib.Platform.variant(Platform.java:368) > at jdk.test.lib.Platform.jvmLibDir(Platform.java:357) > at jdk.test.lib.process.ProcessTools.addJvmLib(ProcessTools.java:585) > at > jdk.test.lib.process.ProcessTools.createNativeTestProcessBuilder(ProcessTools.java:575) > > > > Additional testing: > - [ ] Linux x86_64 Zero affected tests (StackGap, StackGuardPages, TestTLS) > now run Marked as reviewed by dholmes (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5413
Re: RFR: 8231640: (prop) Canonical property storage [v4]
On Wed, 8 Sep 2021 09:54:33 GMT, Jaikiran Pai wrote: >> 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 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.html >> [2] https://reproducible-builds.org/specs/source-date-epoch/ > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > update javadoc @implNote to match latest changes Am I the only one thinking there should also be a way for developers to explicitly disable timestamps from the API? I think the current iteration looks okay (but the core-libs guys of course has the say in this), but I still think we need a new method (or overload) to allow for a timestamp-free to be generated, always, independent of environment variables. - PR: https://git.openjdk.java.net/jdk/pull/5372
Re: RFR: 8273278: Support XSLT on GraalVM Native Image--deterministic bytecode generation in XSLT
On Tue, 7 Sep 2021 23:48:08 GMT, Joe Wang wrote: >> GraalVM Native Image supports loading classes at runtime if they are known >> during image build (class predefinition). This is achieved by the JVMTI >> agent that registers dynamically generated classes in a regular HotSpot run. >> The Native Image build uses these registered classes to embed them into the >> produced binary so they can be loaded at runtime. Loading at runtime is >> achieved by matching the unique hash of generated classes. >> >> If the generated bytecode is unstable across runs, the generated native >> image can not match the hash of the runtime-generated bytecode to the >> pre-defined classes. The execution failure happens here: >> >> https://github.com/oracle/graal/blob/master/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/PredefinedClassesSupport.java#L127-L131. >> >> In XSLT the produced bytecode is unstable for the following reasons: >> >> - Methods like ` HashMap#values` and `HashMap#keySet` result in different >> traversal orders of its elements yielding a different order of methods and >> fields. >> >> - The default `Object#toString` includes the current memory reference of >> `com.sun.org.apache.xalan.internal.xsltc.compiler.Number` in the generated >> class. > > Yeah, there hasn't been any major activities (i.e. a minor release) for 7 > years. It's also true that the JDK has diverged. @JoeWang-Java We need to backport this to JDK 11 as well. How can we do that? Also, is there anything else I should do to merge these changes after approval? - PR: https://git.openjdk.java.net/jdk/pull/5331
Re: RFR: 8273315: Parallelize and increase timeouts for java/foreign/TestMatrix.java test [v2]
On Wed, 8 Sep 2021 09:17:31 GMT, Aleksey Shipilev wrote: >> This test runs a lot of configurations, and spends a lot of time serially. >> This is especially pronounced when run in prospective tier4 runs >> (JDK-8273314). There are reports of multi-hour runs (see JDK-8271613). We >> can parallelize the test configurations for this test to make it hurt less. >> Also, timeouts need to be increased for `TestUpcall` test configs, because >> some of them are very slow in fastdebug mode. >> >> Sample run: >> >> >> $ time CONF=linux-x86_64-server-fastdebug make run-test >> TEST=java/foreign/TestMatrix.java | ts -s >> 00:00:00 Building target 'run-test' in configuration >> 'linux-x86_64-server-fastdebug' >> 00:00:03 Test selection 'java/foreign/TestMatrix.java', will run: >> 00:00:03 * jtreg:test/jdk/java/foreign/TestMatrix.java >> 00:00:03 >> 00:00:03 Running test 'jtreg:test/jdk/java/foreign/TestMatrix.java' >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTF >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFFT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTTF >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTF >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTF >> 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFF >> 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFF >> 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTT >> 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFF >> 00:00:38 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFT >> 00:01:50 Passed: java/foreign/TestMatrix.java#Downcall-FF >> 00:02:27 Passed: java/foreign/TestMatrix.java#Downcall-TF >> 00:03:03 Passed: java/foreign/TestMatrix.java#Downcall-FT >> 00:03:47 Passed: java/foreign/TestMatrix.java#Downcall-TT >> 00:04:17 Passed: java/foreign/TestMatrix.java#Upcall-FTFF >> 00:04:23 Passed: java/foreign/TestMatrix.java#Upcall-TFFF >> 00:05:46 Passed: java/foreign/TestMatrix.java#Upcall-TTFF >> 00:06:03 Passed: java/foreign/TestMatrix.java#Upcall-TFFT >> 00:06:44 Passed: java/foreign/TestMatrix.java#Upcall-FTFT >> 00:08:24 Passed: java/foreign/TestMatrix.java#Upcall-TFTF >> 00:08:39 Passed: java/foreign/TestMatrix.java#Upcall-TTFT >> 00:09:16 Passed: java/foreign/TestMatrix.java#Upcall-FTTF >> 00:09:19 Passed: java/foreign/TestMatrix.java#Upcall-TFTT >> 00:10:01 Passed: java/foreign/TestMatrix.java#Upcall-FTTT >> 00:10:37 Passed: java/foreign/TestMatrix.java#Upcall-TTTF >> 00:12:16 Passed: java/foreign/TestMatrix.java#Upcall- >> 00:12:17 Test results: passed: 32 >> 00:12:21 >> 00:12:21 == >> 00:12:21 Test summary >> 00:12:21 == >> 00:12:21TEST TOTAL PASS >> FAIL ERROR >> 00:12:21jtreg:test/jdk/java/foreign/TestMatrix.java 3232 >> 0 0 >> 00:12:21 == >> >> real 12m20.538s >> user 131m54.043s >> sys 0m59.944s >> >> >> If we don't parallelize, then those 130 minutes are spent serially. > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Go "manual" Marked as reviewed by mcimadamore (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5358
Re: RFR: 8273315: Parallelize and increase timeouts for java/foreign/TestMatrix.java test [v2]
On Wed, 8 Sep 2021 11:34:10 GMT, Maurizio Cimadamore wrote: > Changes looks good. Whether we want to use `manual` or `@stress` I'm not > sure. I guess it depends a lot on which parameters are typically used by CI > to run those tests. I note that, for instance, the makefile `make/RunTests.gmk` does pass the `-automatic` flag to jtreg, but I don't see any keyword-based exclusion set up there - so I think `manual` would work well there. - PR: https://git.openjdk.java.net/jdk/pull/5358
Re: RFR: 8273315: Parallelize and increase timeouts for java/foreign/TestMatrix.java test [v2]
On Wed, 8 Sep 2021 09:17:31 GMT, Aleksey Shipilev wrote: >> This test runs a lot of configurations, and spends a lot of time serially. >> This is especially pronounced when run in prospective tier4 runs >> (JDK-8273314). There are reports of multi-hour runs (see JDK-8271613). We >> can parallelize the test configurations for this test to make it hurt less. >> Also, timeouts need to be increased for `TestUpcall` test configs, because >> some of them are very slow in fastdebug mode. >> >> Sample run: >> >> >> $ time CONF=linux-x86_64-server-fastdebug make run-test >> TEST=java/foreign/TestMatrix.java | ts -s >> 00:00:00 Building target 'run-test' in configuration >> 'linux-x86_64-server-fastdebug' >> 00:00:03 Test selection 'java/foreign/TestMatrix.java', will run: >> 00:00:03 * jtreg:test/jdk/java/foreign/TestMatrix.java >> 00:00:03 >> 00:00:03 Running test 'jtreg:test/jdk/java/foreign/TestMatrix.java' >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTF >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFFT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTTF >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTF >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFT >> 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTF >> 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFF >> 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFF >> 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTT >> 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFF >> 00:00:38 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFT >> 00:01:50 Passed: java/foreign/TestMatrix.java#Downcall-FF >> 00:02:27 Passed: java/foreign/TestMatrix.java#Downcall-TF >> 00:03:03 Passed: java/foreign/TestMatrix.java#Downcall-FT >> 00:03:47 Passed: java/foreign/TestMatrix.java#Downcall-TT >> 00:04:17 Passed: java/foreign/TestMatrix.java#Upcall-FTFF >> 00:04:23 Passed: java/foreign/TestMatrix.java#Upcall-TFFF >> 00:05:46 Passed: java/foreign/TestMatrix.java#Upcall-TTFF >> 00:06:03 Passed: java/foreign/TestMatrix.java#Upcall-TFFT >> 00:06:44 Passed: java/foreign/TestMatrix.java#Upcall-FTFT >> 00:08:24 Passed: java/foreign/TestMatrix.java#Upcall-TFTF >> 00:08:39 Passed: java/foreign/TestMatrix.java#Upcall-TTFT >> 00:09:16 Passed: java/foreign/TestMatrix.java#Upcall-FTTF >> 00:09:19 Passed: java/foreign/TestMatrix.java#Upcall-TFTT >> 00:10:01 Passed: java/foreign/TestMatrix.java#Upcall-FTTT >> 00:10:37 Passed: java/foreign/TestMatrix.java#Upcall-TTTF >> 00:12:16 Passed: java/foreign/TestMatrix.java#Upcall- >> 00:12:17 Test results: passed: 32 >> 00:12:21 >> 00:12:21 == >> 00:12:21 Test summary >> 00:12:21 == >> 00:12:21TEST TOTAL PASS >> FAIL ERROR >> 00:12:21jtreg:test/jdk/java/foreign/TestMatrix.java 3232 >> 0 0 >> 00:12:21 == >> >> real 12m20.538s >> user 131m54.043s >> sys 0m59.944s >> >> >> If we don't parallelize, then those 130 minutes are spent serially. > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Go "manual" Changes looks good. Whether we want to use `manual` or `@stress` I'm not sure. I guess it depends a lot on which parameters are typically used by CI to run those tests. - PR: https://git.openjdk.java.net/jdk/pull/5358
Integrated: 8273000: Remove WeakReference-based class initialisation barrier implementation
On Wed, 25 Aug 2021 22:05:24 GMT, Vladimir Ivanov wrote: > Get rid of WeakReference-based logic in > DirectMethodHandle::checkInitialized() and reimplement it with > `Unsafe::ensureClassInitialized()`/`shouldBeInitialized()`. > > The key observation is that `Unsafe::ensureClassInitialized()` does not block > the initializing thread. > > Also, removed `Unsafe::shouldBeInitialized()` in > `DMH::shouldBeInitialized(MemberName)` to save on calling into the VM. > `Unsafe::ensureClassInitialized()` already has a fast-path check which checks > whether the class is fully initialized or not. > > Testing: tier1 - tier6 This pull request has now been integrated. Changeset: faa942c8 Author:Vladimir Ivanov URL: https://git.openjdk.java.net/jdk/commit/faa942c8ba8ad778b6be20ff6d98a5040a9079e9 Stats: 43 lines in 2 files changed: 5 ins; 28 del; 10 mod 8273000: Remove WeakReference-based class initialisation barrier implementation Reviewed-by: psandoz, mchung - PR: https://git.openjdk.java.net/jdk/pull/5258
Re: RFR: 8273000: Remove WeakReference-based class initialisation barrier implementation [v3]
On Thu, 2 Sep 2021 11:45:01 GMT, Vladimir Ivanov wrote: >> Get rid of WeakReference-based logic in >> DirectMethodHandle::checkInitialized() and reimplement it with >> `Unsafe::ensureClassInitialized()`/`shouldBeInitialized()`. >> >> The key observation is that `Unsafe::ensureClassInitialized()` does not >> block the initializing thread. >> >> Also, removed `Unsafe::shouldBeInitialized()` in >> `DMH::shouldBeInitialized(MemberName)` to save on calling into the VM. >> `Unsafe::ensureClassInitialized()` already has a fast-path check which >> checks whether the class is fully initialized or not. >> >> Testing: tier1 - tier6 > > Vladimir Ivanov has updated the pull request incrementally with one > additional commit since the last revision: > > Update the comment Thanks for the reviews, Mandy, Paul, and David. - PR: https://git.openjdk.java.net/jdk/pull/5258
Re: RFR: 8078641: MethodHandle.asTypeCache can retain classes from unloading [v4]
On Fri, 3 Sep 2021 14:41:45 GMT, Vladimir Ivanov wrote: >> `MethodHandle.asTypeCache` keeps a strong reference to adapted >> `MethodHandle` and it can introduce a class loader leak through its >> `MethodType`. >> >> Proposed fix introduces a 2-level cache (1 element each) where 1st level can >> only contain `MethodHandle`s which are guaranteed to not introduce any >> dependencies on new class loaders compared to the original `MethodHandle`. >> 2nd level is backed by a `SoftReference` and is used as a backup when the >> result of `MethodHandle.asType()` conversion can't populate the higher level >> cache. >> >> The fix is based on [the >> work](http://cr.openjdk.java.net/~plevart/jdk9-dev/MethodHandle.asTypeCacheLeak/) >> made by Peter Levart @plevart back in 2015. >> >> Testing: tier1 - tier6 > > Vladimir Ivanov has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments Thanks for the reviews, Mandy, Paul, Peter, and Stuart. - PR: https://git.openjdk.java.net/jdk/pull/5246
Integrated: 8078641: MethodHandle.asTypeCache can retain classes from unloading
On Wed, 25 Aug 2021 09:31:51 GMT, Vladimir Ivanov wrote: > `MethodHandle.asTypeCache` keeps a strong reference to adapted `MethodHandle` > and it can introduce a class loader leak through its `MethodType`. > > Proposed fix introduces a 2-level cache (1 element each) where 1st level can > only contain `MethodHandle`s which are guaranteed to not introduce any > dependencies on new class loaders compared to the original `MethodHandle`. > 2nd level is backed by a `SoftReference` and is used as a backup when the > result of `MethodHandle.asType()` conversion can't populate the higher level > cache. > > The fix is based on [the > work](http://cr.openjdk.java.net/~plevart/jdk9-dev/MethodHandle.asTypeCacheLeak/) > made by Peter Levart @plevart back in 2015. > > Testing: tier1 - tier6 This pull request has now been integrated. Changeset: 21012f2b Author:Vladimir Ivanov URL: https://git.openjdk.java.net/jdk/commit/21012f2bbe214955d05f8bc583dcdceb0949b601 Stats: 109 lines in 2 files changed: 88 ins; 3 del; 18 mod 8078641: MethodHandle.asTypeCache can retain classes from unloading Co-authored-by: Peter Levart Co-authored-by: Vladimir Ivanov Reviewed-by: psandoz, mchung, plevart - PR: https://git.openjdk.java.net/jdk/pull/5246
Re: RFR: 8273487: Zero: Handle "zero" variant in runtime tests
On Wed, 8 Sep 2021 10:57:33 GMT, Aleksey Shipilev wrote: > JDK-8179317 rewritten runtime shell tests to Java. There is a little omission > in VM variant selection, which makes Zero fail some of the tier1 tests, like: > > > $ CONF=linux-x86_64-zero-fastdebug make exploded-test > TEST=runtime/StackGap/TestStackGap.java > > STDERR: > java.lang.Error: TESTBUG: unsupported vm variant > at jdk.test.lib.Platform.variant(Platform.java:368) > at jdk.test.lib.Platform.jvmLibDir(Platform.java:357) > at jdk.test.lib.process.ProcessTools.addJvmLib(ProcessTools.java:585) > at > jdk.test.lib.process.ProcessTools.createNativeTestProcessBuilder(ProcessTools.java:575) > > > > Additional testing: > - [ ] Linux x86_64 Zero affected tests (StackGap, StackGuardPages, TestTLS) > now run Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5413
Re: RFR: 8273315: Parallelize and increase timeouts for java/foreign/TestMatrix.java test
On Wed, 8 Sep 2021 09:13:20 GMT, Aleksey Shipilev wrote: > New commit: marked tests as `manual`, as per Maurizio's request. This forced > me to drop the `timeout=` clauses, as those are incompatible with `manual` > (jtreg plainly refuses to run these). Ok, I am too late, but just for the record: I let your old patch (including timeout adjustments) run in our test infrastructure and it fixed the timeout issue from [JDK-8271613](https://bugs.openjdk.java.net/browse/JDK-8271613). Probably it could be a better solution to add the stress keyword for slow running or high load tests - like it is done in the hotspot part of the tests. Then automated test run can exclude or select these test by keyword. - PR: https://git.openjdk.java.net/jdk/pull/5358
RFR: 8273487: Zero: Handle "zero" variant in runtime tests
JDK-8179317 rewritten runtime shell tests to Java. There is a little omission in VM variant selection, which makes Zero fail some of the tier1 tests, like: $ CONF=linux-x86_64-zero-fastdebug make exploded-test TEST=runtime/StackGap/TestStackGap.java STDERR: java.lang.Error: TESTBUG: unsupported vm variant at jdk.test.lib.Platform.variant(Platform.java:368) at jdk.test.lib.Platform.jvmLibDir(Platform.java:357) at jdk.test.lib.process.ProcessTools.addJvmLib(ProcessTools.java:585) at jdk.test.lib.process.ProcessTools.createNativeTestProcessBuilder(ProcessTools.java:575) Additional testing: - [ ] Linux x86_64 Zero affected tests (StackGap, StackGuardPages, TestTLS) now run - Commit messages: - Fix Changes: https://git.openjdk.java.net/jdk/pull/5413/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5413=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273487 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5413.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5413/head:pull/5413 PR: https://git.openjdk.java.net/jdk/pull/5413
RFR: 8273484: Cleanup unnecessary null comparison before instanceof check in java.naming
Update code checks both non-null and instance of a class in java.naming module classes. The checks and explicit casts could also be replaced with pattern matching for the instanceof operator. For example: The following code: return (obj != null && obj instanceof CompoundName && impl.equals(((CompoundName)obj).impl)); Can be simplified to: return (obj instanceof CompoundName other) && impl.equals(other.impl); See similar cleanup in java.base - https://bugs.openjdk.java.net/browse/JDK-8258422 - Commit messages: - [PATCH] Cleanup unnecessary null comparison before instanceof check in java.naming Changes: https://git.openjdk.java.net/jdk/pull/5374/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5374=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273484 Stats: 41 lines in 12 files changed: 0 ins; 11 del; 30 mod Patch: https://git.openjdk.java.net/jdk/pull/5374.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5374/head:pull/5374 PR: https://git.openjdk.java.net/jdk/pull/5374
Re: RFR: 8231640: (prop) Canonical property storage [v4]
> 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 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.html > [2] https://reproducible-builds.org/specs/source-date-epoch/ Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: update javadoc @implNote to match latest changes - Changes: - all: https://git.openjdk.java.net/jdk/pull/5372/files - new: https://git.openjdk.java.net/jdk/pull/5372/files/1ded17f9..867ec999 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5372=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5372=02-03 Stats: 5 lines in 1 file changed: 0 ins; 1 del; 4 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
Re: RFR: 8231640: (prop) Canonical property storage
Hello Andrey,
On 07/09/21 7:50 pm, Andrey Turbanov wrote:
On Sun, 5 Sep 2021 12:38:20 GMT, Jaikiran Pai wrote:
Do you mean that converting the keySet() of an
existing Map into an array and then sorting that array and then using
that sorted array to iterate and using these keys to do an additional
lookup for value against the original Map would be more efficient in
this case?
You can convert entrySet() to array. Not a keySet. In this case there is no
need to lookup for values.
I experimented this in a JMH test and the results matched your
expectations. So, I've updated the PR to use array sorting instead of
creating a TreeMap. For reference, here's the JMH benchmark code and the
results:
package org.myapp;
import org.openjdk.jmh.annotations.Benchmark;
import java.util.*;
import java.util.concurrent.*;
import org.openjdk.jmh.annotations.*;
public class MyBenchmark {
@State(Scope.Thread)
public static class TestData {
static final Map tenItems;
static final Map hundredItems;
static final Map thousandItems;
static {
tenItems = new ConcurrentHashMap<>(8);
hundredItems = new ConcurrentHashMap<>(8);
thousandItems = new ConcurrentHashMap<>(8);
for (int i = 0; i < 1000; i++) {
thousandItems.put("foo" + i, "bar");
if (i < 100) {
hundredItems.put("hello" + i, "world");
}
if (i < 10) {
tenItems.put("good" + i, "morning");
}
}
System.out.println("Test data created with " +
tenItems.size() + ", "
+ hundredItems.size() + " and " + thousandItems.size()
+ " Map keys");
}
}
@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
public void testTenItemsTreeMapSorting(TestData testData) {
final Map sorted = new TreeMap(testData.tenItems);
}
@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
public void testHundredItemsTreeMapSorting(TestData testData) {
final Map sorted = new
TreeMap(testData.hundredItems);
}
@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
public void testThousandItemsTreeMapSorting(TestData testData) {
final Map sorted = new
TreeMap(testData.thousandItems);
}
@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
public void testTenItemsArraySorting(TestData testData) {
var entries = testData.tenItems.entrySet().toArray(new
Map.Entry[0]);
Arrays.sort(entries, new Comparator>() {
@Override
public int compare(Map.Entry o1, Map.Entry o2) {
return ((String) o1.getKey()).compareTo((String)
o2.getKey());
}
});
}
@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
public void testHundredItemsArraySorting(TestData testData) {
var entries = testData.hundredItems.entrySet().toArray(new
Map.Entry[0]);
Arrays.sort(entries, new Comparator>() {
@Override
public int compare(Map.Entry o1, Map.Entry o2) {
return ((String) o1.getKey()).compareTo((String)
o2.getKey());
}
});
}
@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
public void testThousandItemsArraySorting(TestData testData) {
var entries = testData.thousandItems.entrySet().toArray(new
Map.Entry[0]);
Arrays.sort(entries, new Comparator>() {
@Override
public int compare(Map.Entry o1, Map.Entry o2) {
return ((String) o1.getKey()).compareTo((String)
o2.getKey());
}
});
}
}
Results:
Benchmark Mode Cnt Score Error Units
MyBenchmark.testHundredItemsArraySorting avgt 25 8.330 ± 0.147
us/op
MyBenchmark.testHundredItemsTreeMapSorting avgt 25 8.637 ± 0.333
us/op
MyBenchmark.testTenItemsArraySorting avgt 25 0.261 ± 0.006
us/op
MyBenchmark.testTenItemsTreeMapSorting avgt 25 0.422 ± 0.007
us/op
MyBenchmark.testThousandItemsArraySorting avgt 25 151.566 ± 1.660
us/op
MyBenchmark.testThousandItemsTreeMapSorting avgt 25 163.767 ± 1.911
us/op
-Jaikiran
Re: RFR: 8231640: (prop) Canonical property storage [v3]
On Wed, 8 Sep 2021 09:26:33 GMT, Jaikiran Pai wrote:
>> 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 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.html
>> [2] https://reproducible-builds.org/specs/source-date-epoch/
>
> Jaikiran Pai has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - adjust testcases to verify the new semantics
> - implement review suggestions:
> - Use doPriveleged instead of explicit permission checks, to reduce
> complexity
> - Use DateTimeFormatter and ZonedDateTime instead of Date.toString()
> - Use DateTimeFormatter.RFC_1123_DATE_TIME for formatting
> SOURCE_DATE_EPOCH dates
> - Use Arrays.sort instead of a TreeMap for ordering property keys
src/java.base/share/classes/java/util/Properties.java line 963:
> 961: synchronized (this) {
> 962: var entries = map.entrySet().toArray(new Map.Entry[0]);
> 963: Arrays.sort(entries, new Comparator>() {
This part here, intentionally doesn't use a lambda, since from what I remember
seeing in some mail discussion, it was suggested that using lambda in core
parts which get used very early during JVM boostrap, should be avoided. If
that's not a concern here, do let me know and I can change it to a lambda.
-
PR: https://git.openjdk.java.net/jdk/pull/5372
Re: RFR: 8231640: (prop) Canonical property storage
On 07/09/21 9:02 pm, Alan Bateman wrote: On 07/09/2021 16:05, Roger Riggs wrote: Hi, The value of SOURCE_DATE_EPOCH is not so sensitive that it needs the protections you are applying. The doPriv only exposes the value of that specific environment variable and in the usual case, it is undefined. The complexity in the specification and implementation seem unnecessary in this case. I agree. Given the complexity then it makes your suggestion/option to just drop the date from the comment somewhat tempting. -Alan I've now updated the PR to take into account the inputs that were provided so far. More specifically, the PR has been updated to: - remove the complexity around SecurityManager usage and now just uses a doPriveleged block to get the SOURCE_EPOCH_DATE. - use Arrays.sort(...) instead of a TreeMap to write out the sorted properties. This was a suggestion from Andrey and based on my JMH testing (which I will post separately), the Arrays.sort(...) did indeed perform better. - use DateTimeFormatter.RFC_1123_DATE_TIME while formatting and writing the reproducible SOURCE_DATE_EPOCH value. There isn't a general agreement yet on what format should be used. Stuart has suggested using a different format (the one in the debian patch). So this part of the change could still undergo further change. - use ZonedDateTime along with a DateTimeFormatter which matches the format used by java.util.Date.toString(), instead of using a java.util.Date() instance when writing out the current date. The new tests that have been introduced in this PR have been adjusted to verify these new expectations. The existing and these new tests continue to pass with these changes. -Jaikiran
Re: RFR: 8231640: (prop) Canonical property storage [v3]
> 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 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.html > [2] https://reproducible-builds.org/specs/source-date-epoch/ Jaikiran Pai has updated the pull request incrementally with two additional commits since the last revision: - adjust testcases to verify the new semantics - implement review suggestions: - Use doPriveleged instead of explicit permission checks, to reduce complexity - Use DateTimeFormatter and ZonedDateTime instead of Date.toString() - Use DateTimeFormatter.RFC_1123_DATE_TIME for formatting SOURCE_DATE_EPOCH dates - Use Arrays.sort instead of a TreeMap for ordering property keys - Changes: - all: https://git.openjdk.java.net/jdk/pull/5372/files - new: https://git.openjdk.java.net/jdk/pull/5372/files/641864e2..1ded17f9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5372=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5372=01-02 Stats: 225 lines in 4 files changed: 73 ins; 119 del; 33 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
Re: RFR: 8273315: Parallelize and increase timeouts for java/foreign/TestMatrix.java test [v2]
On Tue, 7 Sep 2021 16:54:11 GMT, Alan Bateman wrote: >> you are right about /manual being for GUI - but @AlanBateman pointed me at >> few tests with libzip using it for similar reasons (e.g. long running tests). > > I don't think /manual is strictly UI but its usage is discouraged as manual > tests are rarely run. The tests for the SmartCard I/O API come to mind as > they need special setup. There is one or two ZIP tests that take a long time > and have historically been manual tests as a result. There are also a few > tests dotted around that do not have the `@Test` tag, one needs to be working > in a specific area to know about these. Introducing a new test group or a > keyword for these tests could be an option. > > In passing, I should mention the jdk_sctp test group. These tests may require > kernel or other configuration and are deliberately not in any tier. That may > be a comment for the other PR that is proposing a jdk tier4. See new commit. - PR: https://git.openjdk.java.net/jdk/pull/5358
Re: RFR: 8273315: Parallelize and increase timeouts for java/foreign/TestMatrix.java test [v2]
> This test runs a lot of configurations, and spends a lot of time serially. > This is especially pronounced when run in prospective tier4 runs > (JDK-8273314). There are reports of multi-hour runs (see JDK-8271613). We can > parallelize the test configurations for this test to make it hurt less. Also, > timeouts need to be increased for `TestUpcall` test configs, because some of > them are very slow in fastdebug mode. > > Sample run: > > > $ time CONF=linux-x86_64-server-fastdebug make run-test > TEST=java/foreign/TestMatrix.java | ts -s > 00:00:00 Building target 'run-test' in configuration > 'linux-x86_64-server-fastdebug' > 00:00:03 Test selection 'java/foreign/TestMatrix.java', will run: > 00:00:03 * jtreg:test/jdk/java/foreign/TestMatrix.java > 00:00:03 > 00:00:03 Running test 'jtreg:test/jdk/java/foreign/TestMatrix.java' > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFT > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTF > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTT > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTT > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFFT > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTTF > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTF > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFT > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTF > 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFF > 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFF > 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTT > 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFF > 00:00:38 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFT > 00:01:50 Passed: java/foreign/TestMatrix.java#Downcall-FF > 00:02:27 Passed: java/foreign/TestMatrix.java#Downcall-TF > 00:03:03 Passed: java/foreign/TestMatrix.java#Downcall-FT > 00:03:47 Passed: java/foreign/TestMatrix.java#Downcall-TT > 00:04:17 Passed: java/foreign/TestMatrix.java#Upcall-FTFF > 00:04:23 Passed: java/foreign/TestMatrix.java#Upcall-TFFF > 00:05:46 Passed: java/foreign/TestMatrix.java#Upcall-TTFF > 00:06:03 Passed: java/foreign/TestMatrix.java#Upcall-TFFT > 00:06:44 Passed: java/foreign/TestMatrix.java#Upcall-FTFT > 00:08:24 Passed: java/foreign/TestMatrix.java#Upcall-TFTF > 00:08:39 Passed: java/foreign/TestMatrix.java#Upcall-TTFT > 00:09:16 Passed: java/foreign/TestMatrix.java#Upcall-FTTF > 00:09:19 Passed: java/foreign/TestMatrix.java#Upcall-TFTT > 00:10:01 Passed: java/foreign/TestMatrix.java#Upcall-FTTT > 00:10:37 Passed: java/foreign/TestMatrix.java#Upcall-TTTF > 00:12:16 Passed: java/foreign/TestMatrix.java#Upcall- > 00:12:17 Test results: passed: 32 > 00:12:21 > 00:12:21 == > 00:12:21 Test summary > 00:12:21 == > 00:12:21TEST TOTAL PASS > FAIL ERROR > 00:12:21jtreg:test/jdk/java/foreign/TestMatrix.java 3232 > 0 0 > 00:12:21 == > > real 12m20.538s > user 131m54.043s > sys 0m59.944s > > > If we don't parallelize, then those 130 minutes are spent serially. Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision: Go "manual" - Changes: - all: https://git.openjdk.java.net/jdk/pull/5358/files - new: https://git.openjdk.java.net/jdk/pull/5358/files/a31a14a5..77de8cbd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5358=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5358=00-01 Stats: 42 lines in 1 file changed: 10 ins; 0 del; 32 mod Patch: https://git.openjdk.java.net/jdk/pull/5358.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5358/head:pull/5358 PR: https://git.openjdk.java.net/jdk/pull/5358
Re: RFR: 8273315: Parallelize and increase timeouts for java/foreign/TestMatrix.java test
On Fri, 3 Sep 2021 09:53:53 GMT, Aleksey Shipilev wrote: > This test runs a lot of configurations, and spends a lot of time serially. > This is especially pronounced when run in prospective tier4 runs > (JDK-8273314). There are reports of multi-hour runs (see JDK-8271613). We can > parallelize the test configurations for this test to make it hurt less. Also, > timeouts need to be increased for `TestUpcall` test configs, because some of > them are very slow in fastdebug mode. > > Sample run: > > > $ time CONF=linux-x86_64-server-fastdebug make run-test > TEST=java/foreign/TestMatrix.java | ts -s > 00:00:00 Building target 'run-test' in configuration > 'linux-x86_64-server-fastdebug' > 00:00:03 Test selection 'java/foreign/TestMatrix.java', will run: > 00:00:03 * jtreg:test/jdk/java/foreign/TestMatrix.java > 00:00:03 > 00:00:03 Running test 'jtreg:test/jdk/java/foreign/TestMatrix.java' > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFT > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTF > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity- > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTT > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFTT > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FFFT > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTTF > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTTF > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFT > 00:00:31 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTF > 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFFF > 00:00:32 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFF > 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TFTT > 00:00:35 Passed: java/foreign/TestMatrix.java#UpcallHighArity-TTFF > 00:00:38 Passed: java/foreign/TestMatrix.java#UpcallHighArity-FTFT > 00:01:50 Passed: java/foreign/TestMatrix.java#Downcall-FF > 00:02:27 Passed: java/foreign/TestMatrix.java#Downcall-TF > 00:03:03 Passed: java/foreign/TestMatrix.java#Downcall-FT > 00:03:47 Passed: java/foreign/TestMatrix.java#Downcall-TT > 00:04:17 Passed: java/foreign/TestMatrix.java#Upcall-FTFF > 00:04:23 Passed: java/foreign/TestMatrix.java#Upcall-TFFF > 00:05:46 Passed: java/foreign/TestMatrix.java#Upcall-TTFF > 00:06:03 Passed: java/foreign/TestMatrix.java#Upcall-TFFT > 00:06:44 Passed: java/foreign/TestMatrix.java#Upcall-FTFT > 00:08:24 Passed: java/foreign/TestMatrix.java#Upcall-TFTF > 00:08:39 Passed: java/foreign/TestMatrix.java#Upcall-TTFT > 00:09:16 Passed: java/foreign/TestMatrix.java#Upcall-FTTF > 00:09:19 Passed: java/foreign/TestMatrix.java#Upcall-TFTT > 00:10:01 Passed: java/foreign/TestMatrix.java#Upcall-FTTT > 00:10:37 Passed: java/foreign/TestMatrix.java#Upcall-TTTF > 00:12:16 Passed: java/foreign/TestMatrix.java#Upcall- > 00:12:17 Test results: passed: 32 > 00:12:21 > 00:12:21 == > 00:12:21 Test summary > 00:12:21 == > 00:12:21TEST TOTAL PASS > FAIL ERROR > 00:12:21jtreg:test/jdk/java/foreign/TestMatrix.java 3232 > 0 0 > 00:12:21 == > > real 12m20.538s > user 131m54.043s > sys 0m59.944s > > > If we don't parallelize, then those 130 minutes are spent serially. New commit: marked tests as `manual`, as per Maurizio's request. This forced me to drop the `timeout=` clauses, as those are incompatible with `manual` (jtreg plainly refuses to run these). Since OpenJDK build always passes `-a` to tests, one would need to run the jtreg separately to get tests to run, which also includes setting up nativepath appropriately. I have put some instructions at the top of the file. $ time ~/Install/jtreg-6+1/bin/jtreg -jdk:/home/shade/trunks/jdk/build/linux-x86_64-server-fastdebug/images/jdk/ -nativepath:/home/shade/trunks/jdk/build/linux-x86_64-server-fastdebug/support/test/jdk/jtreg/native/lib/ -concurrency:auto ./test/jdk/java/foreign/TestMatrix.java Test results: passed: 32 Report written to /home/shade/trunks/jdk/JTreport/html/report.html Results written to /home/shade/trunks/jdk/JTwork real12m36.699s user127m9.995s sys 1m1.343s - PR: https://git.openjdk.java.net/jdk/pull/5358
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
On Wed, 8 Sep 2021 06:30:34 GMT, wxiang wrote: > Create CSR: https://bugs.openjdk.java.net/browse/JDK-8273473 Thanks. I've updated the CSR to make it clearer what this change is about. There is still some TDB for the JAR file spec. - PR: https://git.openjdk.java.net/jdk/pull/5383
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
On Wed, 8 Sep 2021 06:22:38 GMT, wxiang wrote: >> There is a bug for URLClassPath.findResources with JarIndex. >> With some discussions about the bug, the current priority is to remove the >> JAR index support in URLClassPath, >> and don’t need to do anything to the jar tool in the short term, except just >> to move JarIndex to the jdk.jartool module. >> >> The PR includes: >> 1. remove the JarIndex support in URLClassPath >> 2. move JarIndex into jdk.jartool module. > > wxiang has updated the pull request incrementally with one additional commit > since the last revision: > > Some minor changes > > Include: > 1. remove public for INDEX_NAME in JarFile > 2. recover the definition for INDEX_NAME in JarIndex > 3. mv the test case JarIndexMergeTest.java into test/jdk/java/util/jar Hi all, Just wondering why we remove JarIndex other than fixing it? Is there any strong motive that drives us to do this? Judging from our internal performance testing and user feedback, JarIndex can significantly reduce the time for class/resource lookup. Although JarIndex is an ancient technology, it is still helpful for many modern scenarios such as serverless applications. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/5383
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
On Wed, 8 Sep 2021 06:22:38 GMT, wxiang wrote: >> There is a bug for URLClassPath.findResources with JarIndex. >> With some discussions about the bug, the current priority is to remove the >> JAR index support in URLClassPath, >> and don’t need to do anything to the jar tool in the short term, except just >> to move JarIndex to the jdk.jartool module. >> >> The PR includes: >> 1. remove the JarIndex support in URLClassPath >> 2. move JarIndex into jdk.jartool module. > > wxiang has updated the pull request incrementally with one additional commit > since the last revision: > > Some minor changes > > Include: > 1. remove public for INDEX_NAME in JarFile > 2. recover the definition for INDEX_NAME in JarIndex > 3. mv the test case JarIndexMergeTest.java into test/jdk/java/util/jar Create CSR: https://bugs.openjdk.java.net/browse/JDK-8273473 - PR: https://git.openjdk.java.net/jdk/pull/5383
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
On Tue, 7 Sep 2021 17:39:20 GMT, Sean Mullan wrote:
>> src/java.base/share/classes/java/util/jar/JarVerifier.java line 147:
>>
>>> 145:
>>> 146: if (uname.equals(JarFile.MANIFEST_NAME) ||
>>> 147: uname.equals(JarFile.INDEX_NAME)) {
>>
>> It would be useful if someone from security-libs could comment on this. The
>> interaction between signed JAR and JAR index isn't very clear. The change
>> you have is safe but it might be that we can drop the checking for
>> INDEX.LIST here.
>
> I am thinking this line should not be removed for compatibility with existing
> JARs that have indexes.
still keep the code
-
PR: https://git.openjdk.java.net/jdk/pull/5383
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
> There is a bug for URLClassPath.findResources with JarIndex. > With some discussions about the bug, the current priority is to remove the > JAR index support in URLClassPath, > and don’t need to do anything to the jar tool in the short term, except just > to move JarIndex to the jdk.jartool module. > > The PR includes: > 1. remove the JarIndex support in URLClassPath > 2. move JarIndex into jdk.jartool module. wxiang has updated the pull request incrementally with one additional commit since the last revision: Some minor changes Include: 1. remove public for INDEX_NAME in JarFile 2. recover the definition for INDEX_NAME in JarIndex 3. mv the test case JarIndexMergeTest.java into test/jdk/java/util/jar - Changes: - all: https://git.openjdk.java.net/jdk/pull/5383/files - new: https://git.openjdk.java.net/jdk/pull/5383/files/7e11c607..cba9047d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5383=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5383=00-01 Stats: 12 lines in 4 files changed: 5 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/5383.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5383/head:pull/5383 PR: https://git.openjdk.java.net/jdk/pull/5383
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
On Tue, 7 Sep 2021 10:39:01 GMT, Lance Andersen wrote: >> src/java.base/share/classes/java/util/jar/JarFile.java line 220: >> >>> 218: * The index file name. >>> 219: */ >>> 220: public static final String INDEX_NAME = "META-INF/INDEX.LIST"; >> >> Adding this as a public field means it becomes part of the API, so it >> shouldn't be public here. > > Agree remove public，but recover the same definition in JarIndex - PR: https://git.openjdk.java.net/jdk/pull/5383
