Re: RFR: 8305457: Implement java.io.IO [v10]
On Tue, 21 May 2024 21:16:52 GMT, Pavel Rappo wrote:
>> src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java line 78:
>>
>>> 76: String line = null;
>>> 77: synchronized (writeLock) {
>>> 78: synchronized(readLock) {
>>
>> Suggestion:
>>
>> synchronized (readLock) {
>
> Sorry, but no. It would be inconsistent with the rest of the file, which for
> whatever reason (copy-paste?) does not use whitespace between `synchronized`
> and `(readLock)`.
It looks strange to have space in one case, and don't have it in another. I
would'nt call it consistent.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19112#discussion_r1609287823
RFR: 8332490: JMH org.openjdk.bench.java.util.zip.InflaterInputStreams.inflaterInputStreamRead OOM
Can I please get a review of this test-only change for addressing https://bugs.openjdk.org/browse/JDK-8332490? The jmh test opens a `InflaterInputStream`, reads the stream contents, but then doesn't close the stream. This can lead to resource leak which can then cause OOM as noted in that JBS issue. The commit in this PR closes the `InflaterInputStream` when the reads complete. make test TEST=micro:org.openjdk.bench.java.util.zip.InflaterInputStreams and ./build/macosx-aarch64/images/jdk/bin/java -jar ./build/macosx-aarch64/images/test/micro/benchmarks.jar org.openjdk.bench.java.util.zip.InflaterInputStreams.inflaterInputStreamRead -t max -f 1 -wi 2 -foe true pass after this change. - Commit messages: - 8332490: JMH org.openjdk.bench.java.util.zip.InflaterInputStreams.inflaterInputStreamRead OOM Changes: https://git.openjdk.org/jdk/pull/19340/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19340=00 Issue: https://bugs.openjdk.org/browse/JDK-8332490 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19340.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19340/head:pull/19340 PR: https://git.openjdk.org/jdk/pull/19340
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v12]
On Tue, 21 May 2024 17:41:46 GMT, Volodymyr Paprotski wrote: >> Performance. Before: >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt ScoreError Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt3 6443.934 ± 6.491 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt3 6152.979 ± 4.954 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt3 1895.410 ± 36.979 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt3 1878.955 ± 45.487 ops/s >> Benchmark(algorithm) >> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units >> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1357.810 ± 26.584 ops/s >> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1352.119 ± 23.547 ops/s >> Benchmark (isMontBench) Mode Cnt Score >> Error Units >> PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ± >> 10.970 ops/s >> >> Performance, no intrinsic: >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt3 6529.839 ± 42.420 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt3 6199.747 ± 133.566 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt3 1973.676 ± 54.071 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt3 1932.127 ± 35.920 ops/s >> Benchmark(algorithm) >> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units >> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1355.788 ± 29.858 ops/s >> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1346.523 ± 28.722 ops/s >> Benchmark (isMontBench) Mode Cnt Score >> Error Units >> PolynomialP256Bench.benchMultiply true thrpt3 1919.57... > > Volodymyr Paprotski 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 17 additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/master' into ecc-montgomery > - shenandoah verifier > - comments from Sandhya > - whitespace > - add message back > - whitespace > - Use AffinePoint to exit Montgomery domain > >Style notes: >Affine.equals() >- Mismatched fields only appear to be used from testing, perhaps > should be moved there instead >Affine.getX(boolean)|getY(boolean) >- "Passing flag is bad design" - cleanest/performant alternative to > several instanceof checks >- needed to convert Affine to Projective (need to stay in montgomery > domain) >ECOperations.PointMultiplier > - changes could probably be restored to original (since ProjectivePoint > handling no longer required) > - consider these changes an improvement? (fewer nested classes) > - was an inner-class but not using inner-class features (i.e. ecOps > variable should be converted) > - whitespace > - Comments from Tony and Jatin > - Comments from Jatin and Tony > - ... and 7 more: https://git.openjdk.org/jdk/compare/9ee91a9f...b1a33004 Thanks! I submitted testing and will report back once it passed. - PR Comment: https://git.openjdk.org/jdk/pull/18583#issuecomment-2123869579
stack overflow in regex engine
(Sorry about my previous "do I need to subscribe?" email; in retrospect that was needless noise.) The purpose of this email is twofold: first, inquire about the status of ticket filed a few years ago, and second to point out some non-obvious reasons why it might be slightly more serious than it seems. The ticket is this one https://bugs.openjdk.org/browse/JDK-8260866 (stack overflow in regex matching quantified alternation) The priority is listed as P4, which I guess means something like "medium" (more important than p5, but less than p3) It also has a specific person assigned, which seems vaguely encouraging, but no updates at all in the years since it's been created, which seems less encouraging. It was seemingly never once discussed on this mailing list, not even when it was first filed. As an outsider, I'm not quite sure how to interpret all these various omens and turn them into guesses about its eventual fate. Will it remain unfixed for another decade or two? Will it be fixed in a few months, but then never backported to old versions? Something else? No one knows? That concludes the status inquiry. Now on to the advocacy. Some bugs are annoying, but once you hit them, you can work around them by changing your code so it does not trigger the bug. Note the phrase "your code" above. This is much more awkward to do if the bug triggered by third-party code you got from maven central or something. At that point your options are to either ask the third party library to work around it, or else fork the dependency (which is not well supported by mainstream build tools (or maybe I'm just using them wrong)). In this case, regular expressions are so ubiquitous that the bug is quite plausibly more likely to be triggered by some third party dependency than by code you own. That was the case for me today: after spending hours trying to track down a stack overflow error I found the offending regex in a third party library. The good news is that for the kinds of inputs we need to handle, it is indeed easy to substitute a much simpler regex that would avoid the issue. The bad news is that it's not my code, so I can't. I could petition the maintainers of the library, but this is not great because: First, maybe the version I'm on is not longer even supported, and newer versions are not compatible, Second, it may take them a while to fix it, and third, it is wasteful (and inelegant) to have workarounds slowly percolate throughout the Java ecosystem instead of fixing the problem at the root. The other annoying thing here is that even when you have "enough" stack space to avoid crashing, using it may not be quite "free". For instance, project loom's foundational premise seems to be that "most threads have oversized stacks; we can have more threads if we start off with small stacks and grow them only when needed". This would be false when the thread in question uses a regex with quantified alternation. (Since many Loom threads will be based on the same Runnable, it's a pretty safe bet that if one of them uses this feature, many will, so you can't assume it will "average out".) There are other reasons besides loom to be low on stack space; maybe you're using some crazy framework(s) that like(s) to have call stacks that are crazy deep. Or maybe you're running with -Xss set pretty low. Or you passed a small value for stack space to the Thread constructor. Or maybe none of these things are true, but in most operating systems a thread stack costs "real" memory in proportion to its high-watermark, so even a SINGLE heavy regex in the lifetime of a thread is tantamount to a memory leak of hundreds of kilobytes. Practicalities aside, I don't like it when code consumes "surprising" types of resources, or surprising amounts of them. For instance, you wouldn't expect a sorting function to spawn threads behind your back, unless it was called "parallel sort" or something like that. You wouldn't expect it to allocate multi-gigabyte arrays, nor to perform I/O. Similarly, most functions need only O(1) stack space, so this tends to be the default assumption unless the docs explicitly call out "this thing might throw stack overflows at you so make sure you have plenty of stack space" Some need a bit more... for instance, I would not be surprised if a regex need stack space in proportion to the depth of the parse tree of the regex. But stack space in proportion to the length of the string being matched is the kind of thing that I'd hope gets called out in those @implNotes thingies, or better yet fixed. Even people who know that regex matching can sometimes take exponential time may naively assume that regex matching would not consume O(n) stack space, where n is the input length. What's worse, not only does it indeed consume stack space linear in the length of the input, but the constant hidden by the O() notation is itself pretty scary. For instance, consider the regex that caused my
Re: RFR: 8320448: Accelerate IndexOf using AVX2 [v20]
On Fri, 17 May 2024 23:47:45 GMT, Scott Gibbons wrote: >> Re-write the IndexOf code without the use of the pcmpestri instruction, only >> using AVX2 instructions. This change accelerates String.IndexOf on average >> 1.3x for AVX2. The benchmark numbers: >> >> >> Benchmark Score >> Latest >> StringIndexOf.advancedWithMediumSub 343.573317.934 >> 0.925375393x >> StringIndexOf.advancedWithShortSub11039.081 1053.96 >> 1.014319384x >> StringIndexOf.advancedWithShortSub255.828110.541 >> 1.980027943x >> StringIndexOf.constantPattern9.361 11.906 >> 1.271872663x >> StringIndexOf.searchCharLongSuccess 4.216 4.218 >> 1.000474383x >> StringIndexOf.searchCharMediumSuccess3.133 3.216 >> 1.02649218x >> StringIndexOf.searchCharShortSuccess 3.763.761 >> 1.000265957x >> StringIndexOf.success9.186 >> 9.713 1.057369911x >> StringIndexOf.successBig 14.34146.343 >> 3.231504079x >> StringIndexOfChar.latin1_AVX2_String 6220.918 12154.52 >> 1.953814533x >> StringIndexOfChar.latin1_AVX2_char 5503.556 5540.044 >> 1.006629895x >> StringIndexOfChar.latin1_SSE4_String 6978.854 6818.689 >> 0.977049957x >> StringIndexOfChar.latin1_SSE4_char 5657.499 5474.624 >> 0.967675646x >> StringIndexOfChar.latin1_Short_String 7132.541 >> 6863.3590.962260014x >> StringIndexOfChar.latin1_Short_char 16013.389 16162.437 >> 1.009307711x >> StringIndexOfChar.latin1_mixed_String 7386.12314771.622 >> 1.15517x >> StringIndexOfChar.latin1_mixed_char9901.671 9782.245 >> 0.987938803 > > Scott Gibbons has updated the pull request incrementally with one additional > commit since the last revision: > > Addressing lots of comments. Interim commit. Comment on behalf of @sviswa7 : Unclear whether `size` in `byte_compare_helper` is intended to be in bytes or in elements. Please check its consistency. - PR Comment: https://git.openjdk.org/jdk/pull/16753#issuecomment-2123736900
Re: RFR: 8320448: Accelerate IndexOf using AVX2 [v20]
On Tue, 21 May 2024 18:03:41 GMT, Sandhya Viswanathan wrote: >> Scott Gibbons has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Addressing lots of comments. Interim commit. > > src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4648: > >> 4646: vpxor(vec1, vec2); >> 4647: >> 4648: vptest(vec1, vec1); > > These should be only 128 bit: >pxor(vec1, vec2); >ptest(vec1, vec1); Fixed > src/hotspot/cpu/x86/stubGenerator_x86_64_string.cpp line 1351: > >> 1349: assert_different_registers(needle, needleVal); >> 1350: >> 1351: bool isLL = (ae == StrIntrinsicNode::LL); > > isLL not used in this function. Fixed - PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1609164643 PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1609164578
Re: RFR: 8320448: Accelerate IndexOf using AVX2 [v21]
> Re-write the IndexOf code without the use of the pcmpestri instruction, only > using AVX2 instructions. This change accelerates String.IndexOf on average > 1.3x for AVX2. The benchmark numbers: > > > BenchmarkScore > Latest > StringIndexOf.advancedWithMediumSub 343.573 317.934 > 0.925375393x > StringIndexOf.advancedWithShortSub1 1039.081 1053.96 > 1.014319384x > StringIndexOf.advancedWithShortSub2 55.828110.541 > 1.980027943x > StringIndexOf.constantPattern 9.361 11.906 > 1.271872663x > StringIndexOf.searchCharLongSuccess 4.216 4.218 > 1.000474383x > StringIndexOf.searchCharMediumSuccess 3.133 3.216 > 1.02649218x > StringIndexOf.searchCharShortSuccess 3.763.761 > 1.000265957x > StringIndexOf.success 9.186 9.713 > 1.057369911x > StringIndexOf.successBig14.34146.343 > 3.231504079x > StringIndexOfChar.latin1_AVX2_String6220.918 12154.52 > 1.953814533x > StringIndexOfChar.latin1_AVX2_char 5503.556 5540.044 > 1.006629895x > StringIndexOfChar.latin1_SSE4_String6978.854 6818.689 > 0.977049957x > StringIndexOfChar.latin1_SSE4_char 5657.499 5474.624 > 0.967675646x > StringIndexOfChar.latin1_Short_String 7132.541 6863.359 > 0.962260014x > StringIndexOfChar.latin1_Short_char 16013.389 16162.437 > 1.009307711x > StringIndexOfChar.latin1_mixed_String 7386.12314771.622 > 1.15517x > StringIndexOfChar.latin1_mixed_char 9901.671 9782.245 > 0.987938803 Scott Gibbons has updated the pull request incrementally with one additional commit since the last revision: Fixed CI compiles; re-factor UL processing - Changes: - all: https://git.openjdk.org/jdk/pull/16753/files - new: https://git.openjdk.org/jdk/pull/16753/files/9a861979..38868a35 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16753=20 - incr: https://webrevs.openjdk.org/?repo=jdk=16753=19-20 Stats: 570 lines in 2 files changed: 327 ins; 158 del; 85 mod Patch: https://git.openjdk.org/jdk/pull/16753.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16753/head:pull/16753 PR: https://git.openjdk.org/jdk/pull/16753
Re: RFR: 8332161: Test restoring echo in the Console implementation (java.base) [v2]
> This test intends to verify the behavior of JdkConsole for the java.base > module, wrt restoring the echo. The test assumes the internal methods that > sets/gets the echo status of the platform. Naoto Sato 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 five additional commits since the last revision: - leftover typo - get/setEcho() -> echo() - Merge branch 'JDK-8332084-restoreEcho-readLock' into JDK-8332161-restoreEcho-Test - Merge branch 'JDK-8332084-restoreEcho-readLock' into JDK-8332161-restoreEcho-Test - initial commit - Changes: - all: https://git.openjdk.org/jdk/pull/19315/files - new: https://git.openjdk.org/jdk/pull/19315/files/58151807..ad4a4e47 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19315=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19315=00-01 Stats: 34602 lines in 391 files changed: 29274 ins; 4096 del; 1232 mod Patch: https://git.openjdk.org/jdk/pull/19315.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19315/head:pull/19315 PR: https://git.openjdk.org/jdk/pull/19315
RFR: 8331342: Convert Base64 tests to JUnit
Please review this test-only clean up PR which converts the java.util.Base64 tests to run under JUnit. In general, this allows for the tests to run independently, separates the data providers from the tests, as well being able to utilize the built in JUnit utility test methods to reduce boilerplate testing code. - Commit messages: - Additional clarifying comments - convert TestBase64.java - convert TestBase64Golden.java - convert Base64GetEncoderTest.java Changes: https://git.openjdk.org/jdk/pull/19337/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19337=00 Issue: https://bugs.openjdk.org/browse/JDK-8331342 Stats: 823 lines in 3 files changed: 257 ins; 336 del; 230 mod Patch: https://git.openjdk.org/jdk/pull/19337.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19337/head:pull/19337 PR: https://git.openjdk.org/jdk/pull/19337
Re: RFR: 8332084: Ensure JdkConsoleImpl.restoreEcho visibility in a shutdown hook [v8]
On Tue, 21 May 2024 22:51:14 GMT, Naoto Sato wrote: >> Making sure `restoreEcho` correctly reflects the state in the shutdown >> thread, which differs from the application's thread that issues the >> `readPassword()` method. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Used a dedicate lock for restoreEcho Based on an internal discussion regarding Stuart's above comment, I changed the `restoreEcho` field back to the original. Instead of making it volatile, synchronizing it with a dedicated lock would ensure visibility and atomic status updates (no races). - PR Comment: https://git.openjdk.org/jdk/pull/19184#issuecomment-2123561717
Re: RFR: 8332084: Ensure JdkConsoleImpl.restoreEcho visibility in a shutdown hook [v8]
> Making sure `restoreEcho` correctly reflects the state in the shutdown > thread, which differs from the application's thread that issues the > `readPassword()` method. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Used a dedicate lock for restoreEcho - Changes: - all: https://git.openjdk.org/jdk/pull/19184/files - new: https://git.openjdk.org/jdk/pull/19184/files/e58fbdcd..f69f747a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19184=07 - incr: https://webrevs.openjdk.org/?repo=jdk=19184=06-07 Stats: 66 lines in 3 files changed: 15 ins; 35 del; 16 mod Patch: https://git.openjdk.org/jdk/pull/19184.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19184/head:pull/19184 PR: https://git.openjdk.org/jdk/pull/19184
Re: RFR: 8331876: JFR: Move file read and write events to java.base [v3]
On Fri, 17 May 2024 09:26:03 GMT, Alan Bateman wrote: >> My main concern here is the addition of clutter (checking two flags, and >> creating two levels of nested "impl" methods) at every call site. We may >> need to rearrange our internal API for JFR (jdk.internal.events) in order to >> clean up the call sites without loading additional classes unnecessarily. >> >> I think the main performance comparison to make is the impact on startup >> time of loading the additional FileReadEvent class. That is, to compare the >> startup time of the baseline with code that loads the FileReadEvent class, >> with JFR disabled in both cases. I don't know how to do this. Anyway, if >> loading of additional classes imposes unacceptable overhead, then that >> justifies doing more work to avoid it. That's separate from whether adding >> the `jfrTracing` flag as done in this PR is an effective way to do it. > > I think `if (jfrTracing && FileReadEvent.enabled())` would be more readable > as it would avoid calling going through the traceXXX methods when the flag is > enabled but the specific event is not enabled. Collapsing the extra layer of methods and combining the test into if (jfrTracing && FileReadEvent.enabled()) would indeed keep things a little neater. I'm still questioning the need for `jfrTracing` at all. There's the matter of how this boolean gets set and unset, and whether this is done in a way that comports with the memory model. Setting this aside, is the only benefit that it avoids loading an extra class at JVM startup time (without JFR)? In this case that would be the `FileReadEvent` class, which is the stub class in `jdk.internal.event`. Wouldn't this class be in the CDS archive, making it not worth the effort of bringing up this new `jfrTracing` mechanism simply to avoid loading it unnecessarily? I observe that in JDK 22 some (but not all) of the event classes in `jdk.internal.event` seem to be present in the CDS archive. These include various virtual thread events. - PR Review Comment: https://git.openjdk.org/jdk/pull/19129#discussion_r1609024695
do I need to subscribe to this list in order to post?
This message is for information purposes only. It is not a recommendation, advice, offer or solicitation to buy or sell a product or service, nor an official confirmation of any transaction. It is directed at persons who are professionals and is intended for the recipient(s) only. It is not directed at retail customers. This message is subject to the terms at: https://www.ib.barclays/disclosures/web-and-email-disclaimer.html. For important disclosures, please see: https://www.ib.barclays/disclosures/sales-and-trading-disclaimer.html regarding marketing commentary from Barclays Sales and/or Trading desks, who are active market participants; https://www.ib.barclays/disclosures/barclays-global-markets-disclosures.html regarding our standard terms for Barclays Investment Bank where we trade with you in principal-to-principal wholesale markets transactions; and in respect to Barclays Research, including disclosures relating to specific issuers, see: http://publicresearch.barclays.com. __ If you are incorporated or operating in Australia, read these important disclosures: https://www.ib.barclays/disclosures/important-disclosures-asia-pacific.html. __ For more details about how we use personal information, see our privacy notice: https://www.ib.barclays/disclosures/personal-information-use.html. __
Re: RFR: 8305457: Implement java.io.IO [v10]
On Tue, 21 May 2024 19:55:22 GMT, Andrey Turbanov wrote:
>> Pavel Rappo has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 20 commits:
>>
>> - Add diagnostic output
>> - Use "expect" that was found
>> - Merge branch 'master' into 8305457-Implement-java.io.IO
>>
>># Conflicts:
>># src/java.base/share/classes/java/io/ProxyingConsole.java
>># src/java.base/share/classes/jdk/internal/io/JdkConsole.java
>># src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java
>>#
>> src/jdk.internal.le/share/classes/jdk/internal/org/jline/JdkConsoleProviderImpl.java
>># src/jdk.jshell/share/classes/jdk/jshell/execution/impl/ConsoleImpl.java
>> - Escape prompt
>> - Merge branch 'master' into 8305457-Implement-java.io.IO
>> - Clarify input charset
>> - Make IO final
>> - Fix System.console().readln(null) in jshell
>>
>>Without it, jshell hangs on me. Will think of a test.
>> - Fix typo
>> - Merge branch 'master' into 8305457-Implement-java.io.IO
>> - ... and 10 more: https://git.openjdk.org/jdk/compare/b92bd671...809e98e0
>
> src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java line 78:
>
>> 76: String line = null;
>> 77: synchronized (writeLock) {
>> 78: synchronized(readLock) {
>
> Suggestion:
>
> synchronized (readLock) {
Sorry, but no. It would be inconsistent with the rest of the file, which for
whatever reason (copy-paste?) does not use whitespace between `synchronized`
and `(readLock)`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19112#discussion_r1608962852
Re: RFR: 8332340: Add JavacBench as a test case for CDS [v3]
On Mon, 20 May 2024 17:24:22 GMT, Ioi Lam wrote:
>> JavacBench is a test program that compiles 90 Java source files. It uses a
>> fair amount of invokedynamic callsites, so it's good for testing CDS support
>> for indy and lambda expressions.
>>
>> This test was first integrated into the
>> [leyden](https://github.com/openjdk/leyden/tree/premain) repo. Hence some of
>> the files have copyrights in 2023.
>
> Ioi Lam has updated the pull request incrementally with one additional commit
> since the last revision:
>
> @calvinccheung comments
LGTM, and thanks for the new utilities! That should make writing CDS tests a
lot easier. I have a few style considerations but you can take them or leave
them.
test/lib/jdk/test/lib/cds/CDSAppTester.java line 98:
> 96: }
> 97:
> 98: private final String name;
Could these fields and the constructor be moved to the top of the class?
test/lib/jdk/test/lib/cds/CDSAppTester.java line 147:
> 145: }
> 146:
> 147: private OutputAnalyzer dumpStaticArchive() throws Exception {
The code from 156 to 162 is repeated 3 times here, is it worth making another
function for this?
-
Marked as reviewed by matsaave (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/19256#pullrequestreview-2069620296
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1608953801
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1608949352
Re: RFR: 8332084: Ensure JdkConsoleImpl.restoreEcho visibility in a shutdown hook [v7]
On Mon, 20 May 2024 17:49:29 GMT, Naoto Sato wrote: >> Making sure `restoreEcho` correctly reflects the state in the shutdown >> thread, which differs from the application's thread that issues the >> `readPassword()` method. > > Naoto Sato 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 ten additional commits since > the last revision: > > - Merge branch 'master' into JDK-8332084-restoreEcho-readLock > - Merge branch 'master' into JDK-8332084-restoreEcho-readLock > - copyright year > - Merge branch 'master' into JDK-8332084-restoreEcho-readLock > - get/setEcho() > - Addresses a review comment > - Replaced another unused exception with _ > - Update src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java > >Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> > - initial commit OK, I went over all this again, and it looks pretty good, though there's a potential risk. In the old code, the `restoreEcho` variable was usually false, and it was only true while `readPassword` was running, which then set it back to false. The shutdown hook would therefore almost always see a value of false and decide to do nothing. In the new code, the `restoreEcho` variable stores the initial state of the echo status, which is usually true. We don't keep track of whether readPassword() has set the echo status, since that leads to mutable state and race conditions etc. as discussed previously. The only time the shutdown hook doesn't set the echo status is if the initial echo status of the terminal is false, which probably almost never occurs. Thus, the shutdown hook almost always sets the echo status to true regardless of whether it's necessary to do so. I'm not concerned about the amount of work the shutdown hook is doing. I'm wondering if there's a possibility that setting the terminal modes almost every time on exit is the right thing. It might block, potentially hanging the VM shutdown. ... Reading further, the case I was concerned about was if somebody runs a Java program from a shell and then puts it into the background. Then the JVM shuts down and the shutdown hook tries to enable echo. Does this work, or does it cause the process to be stopped with SIGTTIN or something? And if it works, would it restore echo immediately, even if say a foreground process were reading a password? - PR Comment: https://git.openjdk.org/jdk/pull/19184#issuecomment-2123385605
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a stricter default configuration [v12]
> Add two sample configuration files: > > jaxp-strict.properties: used to set strict configuration, stricter than > jaxp.properties in previous versions such as JDK 22 > >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Updated on 5/16/2024 > > Design change: > The design is changed to include in the JDK two configuration files that are > the default jaxp.properties and jaxp-strict.properties, instead of three, > dropping jaxp-compat.properties. > > Updated on 5/18/2024 > > Withdraw changes to jaxp.properties. The original idea was to match > jaxp-strict.properties with regard to the properties. However, that change > impact the configuration process, resulting in tests that verify the process > to fail. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: add a note to module-info - Changes: - all: https://git.openjdk.org/jdk/pull/18831/files - new: https://git.openjdk.org/jdk/pull/18831/files/55a86db3..dd7f6239 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18831=11 - incr: https://webrevs.openjdk.org/?repo=jdk=18831=10-11 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18831.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831 PR: https://git.openjdk.org/jdk/pull/18831
Re: RFR: 8332547: Unloaded signature classes in DirectMethodHandles
On Tue, 21 May 2024 18:02:45 GMT, Vladimir Ivanov wrote: > I can definitely name it differently (e.g, ensureTypeVisible), but making a > separate class loading pass across signature classes doesn't make much sense. Ok, in that case I suggest also renaming `MemberName::checkForTypeAlias`, maybe to `ensureTypeVisible` as well. - PR Comment: https://git.openjdk.org/jdk/pull/19319#issuecomment-2123368855
Re: RFR: 8305457: Implement java.io.IO [v10]
On Tue, 21 May 2024 15:44:18 GMT, Pavel Rappo wrote:
>> Please review this PR which introduces the `java.io.IO` top-level class and
>> three methods to `java.io.Console` for [Implicitly Declared Classes and
>> Instance Main Methods (Third Preview)].
>>
>> This PR has been obtained as `git merge --squash` of a now obsolete [draft
>> PR].
>>
>> [Implicitly Declared Classes and Instance Main Methods (Third Preview)]:
>> https://bugs.openjdk.org/browse/JDK-8323335
>> [draft PR]: https://github.com/openjdk/jdk/pull/18921
>
> Pavel Rappo has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 20 commits:
>
> - Add diagnostic output
> - Use "expect" that was found
> - Merge branch 'master' into 8305457-Implement-java.io.IO
>
># Conflicts:
># src/java.base/share/classes/java/io/ProxyingConsole.java
># src/java.base/share/classes/jdk/internal/io/JdkConsole.java
># src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java
>#
> src/jdk.internal.le/share/classes/jdk/internal/org/jline/JdkConsoleProviderImpl.java
># src/jdk.jshell/share/classes/jdk/jshell/execution/impl/ConsoleImpl.java
> - Escape prompt
> - Merge branch 'master' into 8305457-Implement-java.io.IO
> - Clarify input charset
> - Make IO final
> - Fix System.console().readln(null) in jshell
>
>Without it, jshell hangs on me. Will think of a test.
> - Fix typo
> - Merge branch 'master' into 8305457-Implement-java.io.IO
> - ... and 10 more: https://git.openjdk.org/jdk/compare/b92bd671...809e98e0
src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java line 78:
> 76: String line = null;
> 77: synchronized (writeLock) {
> 78: synchronized(readLock) {
Suggestion:
synchronized (readLock) {
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19112#discussion_r1608882590
Re: RFR: 8320448: Accelerate IndexOf using AVX2 [v20]
On Fri, 17 May 2024 23:47:45 GMT, Scott Gibbons wrote: >> Re-write the IndexOf code without the use of the pcmpestri instruction, only >> using AVX2 instructions. This change accelerates String.IndexOf on average >> 1.3x for AVX2. The benchmark numbers: >> >> >> Benchmark Score >> Latest >> StringIndexOf.advancedWithMediumSub 343.573317.934 >> 0.925375393x >> StringIndexOf.advancedWithShortSub11039.081 1053.96 >> 1.014319384x >> StringIndexOf.advancedWithShortSub255.828110.541 >> 1.980027943x >> StringIndexOf.constantPattern9.361 11.906 >> 1.271872663x >> StringIndexOf.searchCharLongSuccess 4.216 4.218 >> 1.000474383x >> StringIndexOf.searchCharMediumSuccess3.133 3.216 >> 1.02649218x >> StringIndexOf.searchCharShortSuccess 3.763.761 >> 1.000265957x >> StringIndexOf.success9.186 >> 9.713 1.057369911x >> StringIndexOf.successBig 14.34146.343 >> 3.231504079x >> StringIndexOfChar.latin1_AVX2_String 6220.918 12154.52 >> 1.953814533x >> StringIndexOfChar.latin1_AVX2_char 5503.556 5540.044 >> 1.006629895x >> StringIndexOfChar.latin1_SSE4_String 6978.854 6818.689 >> 0.977049957x >> StringIndexOfChar.latin1_SSE4_char 5657.499 5474.624 >> 0.967675646x >> StringIndexOfChar.latin1_Short_String 7132.541 >> 6863.3590.962260014x >> StringIndexOfChar.latin1_Short_char 16013.389 16162.437 >> 1.009307711x >> StringIndexOfChar.latin1_mixed_String 7386.12314771.622 >> 1.15517x >> StringIndexOfChar.latin1_mixed_char9901.671 9782.245 >> 0.987938803 > > Scott Gibbons has updated the pull request incrementally with one additional > commit since the last revision: > > Addressing lots of comments. Interim commit. src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4648: > 4646: vpxor(vec1, vec2); > 4647: > 4648: vptest(vec1, vec1); These should be only 128 bit: pxor(vec1, vec2); ptest(vec1, vec1); src/hotspot/cpu/x86/stubGenerator_x86_64_string.cpp line 1351: > 1349: assert_different_registers(needle, needleVal); > 1350: > 1351: bool isLL = (ae == StrIntrinsicNode::LL); isLL not used in this function. - PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1608732591 PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1605624430
Re: RFR: 8332547: Unloaded signature classes in DirectMethodHandles
On Mon, 20 May 2024 21:29:20 GMT, Vladimir Ivanov wrote: > JVM routinely installs loader constraints for unloaded signature classes when > method resolution takes place. MethodHandle resolution took a different route > and eagerly resolves signature classes instead (see > `java.lang.invoke.MemberName$Factory::resolve` and > `sun.invoke.util.VerifyAccess::isTypeVisible` for details). > > There's a micro-optimization which bypasses eager resolution for `java.*` > classes. The downside is that `java.*` signature classes can show up as > unloaded. It manifests as inlining failures during JIT-compilation and may > cause severe performance issues. > > Proposed fix removes the aforementioned special case logic during > `MethodHandle` resolution. > > In some cases it may slow down `MethodHandle` construction a bit (e.g., when > repeatedly constructing `DirectMethodHandle`s with lots of arguments), but > `MethodHandle` construction step is not performance critical. > > Testing: hs-tier1 - hs-tier4 Class loading triggered by `Class.forName()` call is at the core of `isTypeVisible`. (The rest is fast path checks.) It's what makes `isTypeVisible` query idempotent. I can definitely name it differently (e.g, `ensureTypeVisible`), but making a separate class loading pass across signature classes doesn't make much sense. - PR Comment: https://git.openjdk.org/jdk/pull/19319#issuecomment-2123160245
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v11]
On Tue, 21 May 2024 07:21:14 GMT, Tobias Hartmann wrote: >> Volodymyr Paprotski has updated the pull request incrementally with one >> additional commit since the last revision: >> >> shenandoah verifier > > I'm getting some conflicts when trying to apply this to master. Could you > please merge the PR? Hi @TobiHartmann , merged with no issues for me. Could you please run the tests again? (I think Tony did run them, but can't hurt verifying again). Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/18583#issuecomment-2123122468
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v12]
> Performance. Before: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt ScoreError Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6443.934 ± 6.491 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6152.979 ± 4.954 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1895.410 ± 36.979 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1878.955 ± 45.487 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1357.810 ± 26.584 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1352.119 ± 23.547 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ± > 10.970 ops/s > > Performance, no intrinsic: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt Score Error Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6529.839 ± 42.420 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6199.747 ± 133.566 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1973.676 ± 54.071 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1932.127 ± 35.920 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1355.788 ± 29.858 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1346.523 ± 28.722 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply true thrpt3 1919.574 ± > 10.591 ops/s > > Performance, **with intrinsics*... Volodymyr Paprotski 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 17 additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into ecc-montgomery - shenandoah verifier - comments from Sandhya - whitespace - add message back - whitespace - Use AffinePoint to exit Montgomery domain Style notes: Affine.equals() - Mismatched fields only appear to be used from testing, perhaps should be moved there instead Affine.getX(boolean)|getY(boolean) - "Passing flag is bad design" - cleanest/performant alternative to several instanceof checks - needed to convert Affine to Projective (need to stay in montgomery domain) ECOperations.PointMultiplier - changes could probably be restored to original (since ProjectivePoint handling no longer required) - consider these changes an improvement? (fewer nested classes) - was an inner-class but not using inner-class features (i.e. ecOps variable should be converted) - whitespace - Comments from Tony and Jatin - Comments from Jatin and Tony - ... and 7 more: https://git.openjdk.org/jdk/compare/12e8009b...b1a33004 - Changes: - all: https://git.openjdk.org/jdk/pull/18583/files - new: https://git.openjdk.org/jdk/pull/18583/files/df4fe6fa..b1a33004 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18583=11 - incr: https://webrevs.openjdk.org/?repo=jdk=18583=10-11 Stats: 190975 lines in 3949 files changed: 105304 ins; 64688 del; 20983 mod Patch: https://git.openjdk.org/jdk/pull/18583.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18583/head:pull/18583 PR: https://git.openjdk.org/jdk/pull/18583
Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref [v22]
On Wed, 17 Apr 2024 17:12:28 GMT, Brent Christian wrote: >> Brent Christian has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Another update to reachabilityFence() @apiNote > > AFAICT, all review feedback on this change has been addressed. I would love > to have some reviewers take another look. Thanks! > @bchristi-git Just checking in—we're waiting for CSR-approval here before > integrating? 樂 Indeed - can't move forward without a CSR. Also wouldn't mind more reviewer ✔️s. - PR Comment: https://git.openjdk.org/jdk/pull/16644#issuecomment-2123059255
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]
On Tue, 21 May 2024 08:44:47 GMT, Maurizio Cimadamore wrote: > These are all good suggestions. I have not looked into jpackage, but yes, I > would expect that the jpackage user would need to provide extra options when > packaging the application. It would be good to document how jpackage users packaging apps with native access will be affected by this change. Primarily that they need to pass `--illegal-native-access` parameter to affected jpackage app launchers. - PR Comment: https://git.openjdk.org/jdk/pull/19213#issuecomment-2123054154
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]
On Fri, 17 May 2024 13:38:25 GMT, Maurizio Cimadamore wrote: >> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting >> the use of JNI in the following ways: >> >> * `System::load` and `System::loadLibrary` are now restricted methods >> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods >> * binding a JNI `native` method declaration to a native implementation is >> now considered a restricted operation >> >> This PR slightly changes the way in which the JDK deals with restricted >> methods, even for FFM API calls. In Java 22, the single >> `--enable-native-access` was used both to specify a set of modules for which >> native access should be allowed *and* to specify whether illegal native >> access (that is, native access occurring from a module not specified by >> `--enable-native-access`) should be treated as an error or a warning. More >> specifically, an error is only issued if the `--enable-native-access flag` >> is used at least once. >> >> Here, a new flag is introduced, namely >> `illegal-native-access=allow/warn/deny`, which is used to specify what >> should happen when access to a restricted method and/or functionality is >> found outside the set of modules specified with `--enable-native-access`. >> The default policy is `warn`, but users can select `allow` to suppress the >> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This >> aligns the treatment of restricted methods with other mechanisms, such as >> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. >> >> Some changes were required in the package-info javadoc for >> `java.lang.foreign`, to reflect the changes in the command line flags >> described above. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments client parts look fine. - Marked as reviewed by prr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19213#pullrequestreview-2069134455
Re: RFR: 8332340: Add JavacBench as a test case for CDS [v3]
On Mon, 20 May 2024 17:24:22 GMT, Ioi Lam wrote: >> JavacBench is a test program that compiles 90 Java source files. It uses a >> fair amount of invokedynamic callsites, so it's good for testing CDS support >> for indy and lambda expressions. >> >> This test was first integrated into the >> [leyden](https://github.com/openjdk/leyden/tree/premain) repo. Hence some of >> the files have copyrights in 2023. > > Ioi Lam has updated the pull request incrementally with one additional commit > since the last revision: > > @calvinccheung comments Updates look good. Thanks! - Marked as reviewed by ccheung (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19256#pullrequestreview-2069075764
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]
On Fri, 17 May 2024 13:38:25 GMT, Maurizio Cimadamore wrote: >> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting >> the use of JNI in the following ways: >> >> * `System::load` and `System::loadLibrary` are now restricted methods >> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods >> * binding a JNI `native` method declaration to a native implementation is >> now considered a restricted operation >> >> This PR slightly changes the way in which the JDK deals with restricted >> methods, even for FFM API calls. In Java 22, the single >> `--enable-native-access` was used both to specify a set of modules for which >> native access should be allowed *and* to specify whether illegal native >> access (that is, native access occurring from a module not specified by >> `--enable-native-access`) should be treated as an error or a warning. More >> specifically, an error is only issued if the `--enable-native-access flag` >> is used at least once. >> >> Here, a new flag is introduced, namely >> `illegal-native-access=allow/warn/deny`, which is used to specify what >> should happen when access to a restricted method and/or functionality is >> found outside the set of modules specified with `--enable-native-access`. >> The default policy is `warn`, but users can select `allow` to suppress the >> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This >> aligns the treatment of restricted methods with other mechanisms, such as >> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. >> >> Some changes were required in the package-info javadoc for >> `java.lang.foreign`, to reflect the changes in the command line flags >> described above. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments `jdk.jpackage` changes look good - PR Comment: https://git.openjdk.org/jdk/pull/19213#issuecomment-2122942586
Re: RFR: 8331865: Consolidate size and alignment checks in LayoutPath [v3]
On Tue, 21 May 2024 10:20:27 GMT, Maurizio Cimadamore
wrote:
>> When creating a nested memory access var handle, we ensure that the segment
>> is accessed at the correct alignment for the root layout being accessed. But
>> we do not ensure that the segment has at least the size of the accessed root
>> layout. Example:
>>
>>
>> MemoryLayout LAYOUT = sequenceLayout(2, structLayout(JAVA_INT.withName("x"),
>> JAVA_INT.withName("y")));
>> VarHandle X_HANDLE = LAYOUT.varHandle(PathElement.sequenceElement(),
>> PathElement.groupElement("x"));
>>
>>
>> If I have a memory segment whose size is 12, I can successfully call
>> X_HANDLE on it with index 1, like so:
>>
>>
>> MemorySegment segment = Arena.ofAuto().allocate(12);
>> int x = (int)X_HANDLE.get(segment, 0, 1);
>>
>>
>> This seems incorrect: after all, the provided segment doesn't have enough
>> space to fit _two_ elements of the nested structs.
>>
>> This PR adds checks to detect this kind of scenario earlier, thus improving
>> usability. To achieve this we could, in principle, just tweak
>> `LayoutPath::checkEnclosingLayout` and add the required size check.
>>
>> But doing so will add yet another redundant check on top of the other
>> already added by [pull/19124](https://git.openjdk.org/jdk/pull/19124).
>> Instead, this PR rethinks how memory segment var handles are created, in
>> order to eliminate redundant checks.
>>
>> The main observation is that size and alignment checks depend on an
>> _enclosing_ layout. This layout *might* be the very same value layout being
>> accessed (this is the case when e.g. using `JAVA_INT.varHandle()`). But, in
>> the general case, the accessed value layout and the enclosing layout might
>> differ (e.g. think about accessing a struct field).
>>
>> Furthermore, the enclosing layout check depends on the _base_ offset at
>> which the segment is accessed, _prior_ to any index computation that occurs
>> if the accessed layout path has any open elements. It is important to notice
>> that this base offset is only available when looking at the var handle that
>> is returned to the user. For instance, an indexed var handle with
>> coordinates:
>>
>>
>> (MemorySegment, long /* base */, long /* index 1 */, long /* index 2 */,
>> long /* index 3 */)
>>
>>
>> Is obtained through adaptation, by taking a more basic var handle of the
>> form:
>>
>>
>> (MemorySegment, long /* offset */)
>>
>>
>> And then injecting the result of the index multiplication into `offset`. As
>> such, we can't add an enclosing layout check inside the var handle returned
>> by `VarHandles::memorySegmentViewHandle`, as doing so will end up seeing the
>> *wrong* off...
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix typo in javadoc
Marked as reviewed by psandoz (Reviewer).
-
PR Review: https://git.openjdk.org/jdk/pull/19251#pullrequestreview-2068996279
Re: RFR: 8305457: Implement java.io.IO [v10]
> Please review this PR which introduces the `java.io.IO` top-level class and > three methods to `java.io.Console` for [Implicitly Declared Classes and > Instance Main Methods (Third Preview)]. > > This PR has been obtained as `git merge --squash` of a now obsolete [draft > PR]. > > [Implicitly Declared Classes and Instance Main Methods (Third Preview)]: > https://bugs.openjdk.org/browse/JDK-8323335 > [draft PR]: https://github.com/openjdk/jdk/pull/18921 Pavel Rappo has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 20 commits: - Add diagnostic output - Use "expect" that was found - Merge branch 'master' into 8305457-Implement-java.io.IO # Conflicts: #src/java.base/share/classes/java/io/ProxyingConsole.java #src/java.base/share/classes/jdk/internal/io/JdkConsole.java #src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java # src/jdk.internal.le/share/classes/jdk/internal/org/jline/JdkConsoleProviderImpl.java #src/jdk.jshell/share/classes/jdk/jshell/execution/impl/ConsoleImpl.java - Escape prompt - Merge branch 'master' into 8305457-Implement-java.io.IO - Clarify input charset - Make IO final - Fix System.console().readln(null) in jshell Without it, jshell hangs on me. Will think of a test. - Fix typo - Merge branch 'master' into 8305457-Implement-java.io.IO - ... and 10 more: https://git.openjdk.org/jdk/compare/b92bd671...809e98e0 - Changes: https://git.openjdk.org/jdk/pull/19112/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19112=09 Stats: 692 lines in 13 files changed: 692 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19112.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19112/head:pull/19112 PR: https://git.openjdk.org/jdk/pull/19112
Re: RFR: 8331196: vector api: Remove unnecessary index check in Byte/ShortVector.fromArray/fromArray0Template
On Mon, 20 May 2024 19:21:44 GMT, Paul Sandoz wrote: >> Hi, >> Can you help to review this simple patch? >> Some index check in Byte/ShortVector.fromArray/fromArray0Template seems not >> necessary, could be removed. >> Thanks > > That does not look correct and will only check a prefix indexes. A > `ByteVector` with a shape of 256 bits has 32 lanes, whereas an `IntVector` of > the same shape has 8 lanes. The `mapOffset` array will hold 32 indexes that > need checking, so we need to loop through `mapOffset` array four times. > Thanks @PaulSandoz for comment. I just re-ran the vector api tests with this > patch on x64 and riscv64, but seems no failures triggered. Let me check > further, either I missed something or maybe there is some gap in test to be > filled. More likely a gap in the tests, not sufficiently checking for out of bounds access across the range in the mapOffset array. - PR Comment: https://git.openjdk.org/jdk/pull/18977#issuecomment-2122917109
Re: RFR: 8331189: Implementation of Scoped Values (Third Preview)
On Wed, 8 May 2024 09:40:38 GMT, Alan Bateman wrote: > JEP 481 proposes Scoped Values to continue to preview in JDK 23 with one > change. The type of the operation parameter of the callWhere method is > changed to a new functional interface to avoid having the API throw > Exception. With that change, the getWhere (and the corresponding method on > Carrier) are no longer needed. The functional interface is not proposed for > j.u.function at this time. The changes look OK to me. - Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19136#pullrequestreview-2068941921
Re: RFR: 8331189: Implementation of Scoped Values (Third Preview)
On Wed, 8 May 2024 09:40:38 GMT, Alan Bateman wrote: > JEP 481 proposes Scoped Values to continue to preview in JDK 23 with one > change. The type of the operation parameter of the callWhere method is > changed to a new functional interface to avoid having the API throw > Exception. With that change, the getWhere (and the corresponding method on > Carrier) are no longer needed. The functional interface is not proposed for > j.u.function at this time. src/java.base/share/classes/java/lang/ScopedValue.java line 494: > 492: > 493: /** > 494: * An operation that returns a result and may throw an exception. Nit - should it be "or may throw an exception"? - PR Review Comment: https://git.openjdk.org/jdk/pull/19136#discussion_r1608511614
RFR: 8332614: Type-checked ConstantPool.entryByIndex and ClassReader.readEntryOrNull
I propose to add type-checked ConstantPool.entryByIndex and ClassReader.readEntryOrNull taking an extra Class parameter, which throws ConstantPoolException instead of ClassCastException on type mismatch, which can happen to malformed ClassFiles. Requesting a review from @asotona. Thanks! Proposal on mailing list: https://mail.openjdk.org/pipermail/classfile-api-dev/2024-May/000512.html - Commit messages: - 8332614: Type-checked ConstantPool.entryByIndex and ClassReader.readEntryOrNull Changes: https://git.openjdk.org/jdk/pull/19330/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19330=00 Issue: https://bugs.openjdk.org/browse/JDK-8332614 Stats: 183 lines in 14 files changed: 103 ins; 30 del; 50 mod Patch: https://git.openjdk.org/jdk/pull/19330.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19330/head:pull/19330 PR: https://git.openjdk.org/jdk/pull/19330
RFR: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null
When building with ubsan enabled (--enable-uban) on Linux x86_64 and doing jtreg tests afterwards I run into this error : /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null #0 0x7fd95bec78d8 in spawnChild /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562 #1 0x7fd95bec78d8 in startChild /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:612 #2 0x7fd95bec78d8 in Java_java_lang_ProcessImpl_forkAndExec /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:712 #3 0x7fd93797a06d () this is the memcpy call getting an unexpected null pointer : memcpy(buf+offset, c->pdir, sp.dirlen); gets a second parameter null. Something similar was discussed and fixed here https://bugs.python.org/issue27570 for Python . Similar issue in OpenJDK _ https://bugs.openjdk.org/browse/JDK-8332473 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed as argument 1, which is declared to never be null - Commit messages: - JDK-8332589 Changes: https://git.openjdk.org/jdk/pull/19329/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19329=00 Issue: https://bugs.openjdk.org/browse/JDK-8332589 Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19329.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19329/head:pull/19329 PR: https://git.openjdk.org/jdk/pull/19329
Integrated: 8331224: ClassCastException in ObjectInputStream hides ClassNotFoundException
On Wed, 1 May 2024 18:43:21 GMT, Roger Riggs wrote: > The issue reported a ClassCastException "cannot assign instance of > java.util.CollSer to field of type java.util.Map" > while deserializing an object referring to an immutable Map that contained a > reference to a class that was not available. > Immutable Collections such as Map utilize a serialization proxy in their > serialized form. > During deserialization the serialization proxy (a private implementation > class) was attempted to be set in a field resulting in the > ClassCastException. The ClassCastException and bug hid the ClassCastException > that should have been thrown. > > When reading record fields or fields of a class, the results of > deserialization of individual fields are recorded as dependencies of the > object being constructed. > The apparent bug is that the summary of those dependencies is not checked > between reading the fields and invoking the constructor to create the record > or assigning the fields to an object being constructed. This pull request has now been integrated. Changeset: 8291c94b Author:Roger Riggs URL: https://git.openjdk.org/jdk/commit/8291c94bcdbb01beddc94f290f2749841404cc0c Stats: 199 lines in 2 files changed: 195 ins; 0 del; 4 mod 8331224: ClassCastException in ObjectInputStream hides ClassNotFoundException Reviewed-by: smarks - PR: https://git.openjdk.org/jdk/pull/19043
Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]
On Fri, 17 May 2024 09:31:33 GMT, Per Minborg wrote: >> # Stable Values & Collections (Internal) >> >> ## Summary >> This PR proposes to introduce an internal _Stable Values & Collections_ API, >> which provides immutable value holders where elements are initialized _at >> most once_. Stable Values & Collections offer the performance and safety >> benefits of final fields while offering greater flexibility as to the timing >> of initialization. >> >> ## Goals >> * Provide an easy and intuitive API to describe value holders that can >> change at most once. >> * Decouple declaration from initialization without significant footprint or >> performance penalties. >> * Reduce the amount of static initializer and/or field initialization code. >> * Uphold integrity and consistency, even in a multi-threaded environment. >> >> For more details, see the draft JEP: https://openjdk.org/jeps/8312611 >> >> ## Performance >> Performance compared to instance variables using two `AtomicReference` and >> two protected by double-checked locking under concurrent access by all >> threads: >> >> >> Benchmark Mode Cnt Score >> Error Units >> StableBenchmark.atomicthrpt 10259.478 ? >> 36.809 ops/us >> StableBenchmark.dcl thrpt 10225.710 ? >> 26.638 ops/us >> StableBenchmark.stablethrpt 10 4382.478 ? >> 1151.472 ops/us <- StableValue significantly faster >> >> >> Performance compared to static variables protected by `AtomicReference`, >> class-holder idiom holder, and double-checked locking (all threads): >> >> >> Benchmark Mode Cnt Score >> Error Units >> StableStaticBenchmark.atomic thrpt 10 6487.835 ? >> 385.639 ops/us >> StableStaticBenchmark.dcl thrpt 10 6605.239 ? >> 210.610 ops/us >> StableStaticBenchmark.stable thrpt 10 14338.239 ? >> 1426.874 ops/us >> StableStaticBenchmark.staticCHI thrpt 10 13780.341 ? >> 1839.651 ops/us >> >> >> Performance for stable lists (thread safe) in both instance and static >> contexts whereby we access a single value compared to `ArrayList` instances >> (which are not thread-safe) (all threads): >> >> >> Benchmark Mode Cnt Score >> Error Units >> StableListElementBenchmark.instanceArrayList thrpt 10 5812.992 ? >> 1169.730 ops/us >> StableListElementBenchmark.instanceList thrpt 10 4818.643 ? >> 704.893 ops/us >> StableListElementBenchmark... > > Per Minborg has updated the pull request incrementally with two additional > commits since the last revision: > > - Add benchmarks for memoized IntFunction and Function > - Add benchmark for memoized supplier Thanks for the insights. Also, I wonder what is a good amount of metadata you are considering, as original stable values only take one possible representation out of all to indicate a mutable state, much lighter in weight compared to the current implementation, which takes many bits; this might disencourage StableValue use. - PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2122714875
Re: RFR: 8332528: Generate code in SwitchBootstraps.generateTypeSwitch that require fewer adaptations [v3]
On Tue, 21 May 2024 09:01:32 GMT, Claes Redestad wrote: >> We can fold the call to `Objects.checkIndex` into the code generated in >> generateTypeSwitchSkeleton instead of doing so by filtering the MH argument. >> This loads 9 less classes (of which 8 generated LFs and Species classes) on >> a minimal test, while being neutral on a throughput sanity test: >> >> >> Name Cnt Base Error Test Error Unit Change >> SwitchSanity.switchSum 15 8,162 ± 0,117 8,152 ± 0,131 ns/op 1,00x (p = >> 0,800 ) >> * = significant >> ``` >> >> A few additional optimizations includes moving some seldom used `findStatic` >> calls to a holder. All in all this means a reduction by 22M cycles to >> bootstrap a trivial switch expression on my M1. > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Add type switch to HelloClasslist Marked as reviewed by liach (Author). - PR Review: https://git.openjdk.org/jdk/pull/19307#pullrequestreview-2068663088
Integrated: 8332086: Remove the usage of ServiceLoader in j.u.r.RandomGeneratorFactory
On Mon, 13 May 2024 08:47:50 GMT, Raffaello Giulietti wrote: > All random number generator algorithms are implemented in module `java.base`. > The usage of `ServiceLoader` in `j.u.r.RandomGeneratorFactory` is no longer > needed. This pull request has now been integrated. Changeset: 42e3c842 Author:Raffaello Giulietti URL: https://git.openjdk.org/jdk/commit/42e3c842ae2684265c794868fc76eb0ff2dea3d9 Stats: 610 lines in 19 files changed: 215 ins; 275 del; 120 mod 8332086: Remove the usage of ServiceLoader in j.u.r.RandomGeneratorFactory 8332476: j.u.r.RandomGeneratorFactor.create(long|byte[]) should throw rather than silently fallback to no-arg create() Reviewed-by: jpai - PR: https://git.openjdk.org/jdk/pull/19212
Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]
On Fri, 17 May 2024 09:31:33 GMT, Per Minborg wrote: >> # Stable Values & Collections (Internal) >> >> ## Summary >> This PR proposes to introduce an internal _Stable Values & Collections_ API, >> which provides immutable value holders where elements are initialized _at >> most once_. Stable Values & Collections offer the performance and safety >> benefits of final fields while offering greater flexibility as to the timing >> of initialization. >> >> ## Goals >> * Provide an easy and intuitive API to describe value holders that can >> change at most once. >> * Decouple declaration from initialization without significant footprint or >> performance penalties. >> * Reduce the amount of static initializer and/or field initialization code. >> * Uphold integrity and consistency, even in a multi-threaded environment. >> >> For more details, see the draft JEP: https://openjdk.org/jeps/8312611 >> >> ## Performance >> Performance compared to instance variables using two `AtomicReference` and >> two protected by double-checked locking under concurrent access by all >> threads: >> >> >> Benchmark Mode Cnt Score >> Error Units >> StableBenchmark.atomicthrpt 10259.478 ? >> 36.809 ops/us >> StableBenchmark.dcl thrpt 10225.710 ? >> 26.638 ops/us >> StableBenchmark.stablethrpt 10 4382.478 ? >> 1151.472 ops/us <- StableValue significantly faster >> >> >> Performance compared to static variables protected by `AtomicReference`, >> class-holder idiom holder, and double-checked locking (all threads): >> >> >> Benchmark Mode Cnt Score >> Error Units >> StableStaticBenchmark.atomic thrpt 10 6487.835 ? >> 385.639 ops/us >> StableStaticBenchmark.dcl thrpt 10 6605.239 ? >> 210.610 ops/us >> StableStaticBenchmark.stable thrpt 10 14338.239 ? >> 1426.874 ops/us >> StableStaticBenchmark.staticCHI thrpt 10 13780.341 ? >> 1839.651 ops/us >> >> >> Performance for stable lists (thread safe) in both instance and static >> contexts whereby we access a single value compared to `ArrayList` instances >> (which are not thread-safe) (all threads): >> >> >> Benchmark Mode Cnt Score >> Error Units >> StableListElementBenchmark.instanceArrayList thrpt 10 5812.992 ? >> 1169.730 ops/us >> StableListElementBenchmark.instanceList thrpt 10 4818.643 ? >> 704.893 ops/us >> StableListElementBenchmark... > > Per Minborg has updated the pull request incrementally with two additional > commits since the last revision: > > - Add benchmarks for memoized IntFunction and Function > - Add benchmark for memoized supplier We are considering another implementation with less complexity. So, for now, thank you for all the feedback so far. We will try to make sure to carry over them to a new PR. - PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2122569580
Re: RFR: 8332528: Generate code in SwitchBootstraps.generateTypeSwitch that require fewer adaptations [v3]
On Tue, 21 May 2024 09:01:32 GMT, Claes Redestad wrote: >> We can fold the call to `Objects.checkIndex` into the code generated in >> generateTypeSwitchSkeleton instead of doing so by filtering the MH argument. >> This loads 9 less classes (of which 8 generated LFs and Species classes) on >> a minimal test, while being neutral on a throughput sanity test: >> >> >> Name Cnt Base Error Test Error Unit Change >> SwitchSanity.switchSum 15 8,162 ± 0,117 8,152 ± 0,131 ns/op 1,00x (p = >> 0,800 ) >> * = significant >> ``` >> >> A few additional optimizations includes moving some seldom used `findStatic` >> calls to a holder. All in all this means a reduction by 22M cycles to >> bootstrap a trivial switch expression on my M1. > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Add type switch to HelloClasslist [f04d78e](https://github.com/openjdk/jdk/pull/19307/commits/f04d78ea53ed0074026311f82eb0d4eafee3438d) passed tier1-3 testing - PR Comment: https://git.openjdk.org/jdk/pull/19307#issuecomment-2122519106
Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref [v22]
On Wed, 17 Apr 2024 17:12:28 GMT, Brent Christian wrote: >> Brent Christian has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Another update to reachabilityFence() @apiNote > > AFAICT, all review feedback on this change has been addressed. I would love > to have some reviewers take another look. Thanks! @bchristi-git Just checking in—we're waiting for CSR-approval here before integrating? 樂 - PR Comment: https://git.openjdk.org/jdk/pull/16644#issuecomment-2122461878
Re: RFR: 8332154: Memory leak in SynchronousQueue
On Thu, 16 May 2024 21:36:03 GMT, Konstantin Nisht wrote: >> @AlanBateman @DougLea Reviews are most welcome :) > > @viktorklang-ora We managed to reproduce it quite reliably by running the > test from the bug report multiple times (in our case, it was 100). I > understand this does not make a 100% stable regression reproducer, but in > case of multiple runs the probability of false positive in quite low. @knisht @vprovodin Thanks for the thoughts, I decided to go for a blackbox test to make sure that this test does not rely on any internal impl details of SynchronousQueue. - PR Comment: https://git.openjdk.org/jdk/pull/19271#issuecomment-2122448833
Re: RFR: 8332597: Remove redundant methods from j.l.classfile.ClassReader API
On Tue, 21 May 2024 09:20:36 GMT, Adam Sotona wrote: > j.l.classfile.ClassReader instance is exposed in the Class-File API through > j.l.classfile.AttributeMapper::readAttribute method only. > ClassReader only purpose is to serve as a tool for reading content of a > custom attribute in a user-provided AttribtueMapper. > It contains useful set of low-level class reading methods for user to > implement a custom attribute content parser. > > However methods ClassReader::thisClassPos and > ClassReader::skipAttributeHolder are not necessary for a custom attribute > content parsing and so redundant in the API. > Class-File API implementation internally use these methods, however they > should not be exposed in the API. > > This patch removes the methods from the API. > > Please review. > > Thanks, > Adam Marked as reviewed by liach (Author). - PR Review: https://git.openjdk.org/jdk/pull/19323#pullrequestreview-2068383252
Re: RFR: 8307818: Convert Indify tool to Classfile API [v8]
On Mon, 20 May 2024 20:56:18 GMT, Oussama Louati wrote: >> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code >> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, >> MethodType, and CallSite constants. >> It currently uses ad-hoc code to process class files and intends to migrate >> to ASM; but since we have the Classfile API, we can migrate to Classfile API >> instead. > > Oussama Louati has updated the pull request incrementally with one additional > commit since the last revision: > > update: Added javadoc documentation The pull request is not buildable in the current state and I suggest to return it to draft. Minimal change to enable preview for build and run of the Indify tool is drafted here: https://github.com/openjdk/jdk/commit/7c2344b275da1dff99ade284192488eaa8ffba02 Indify invocation on Microbenchmarks as a part of the JDK build (after application of the above patch) still fails with: Failure on /Applications/XcodeJIB.app/dev/github/jdk/build/macosx-aarch64/support/test/micro/classes Exception in thread "main" java.lang.NullPointerException: Cannot invoke "java.lang.classfile.ClassModel.thisClass()" because "model" is null at java.base/java.lang.classfile.ClassFile.transform(ClassFile.java:445) at indify.Indify.transformToBytes(Indify.java:369) at indify.Indify.writeNewClassFile(Indify.java:360) at indify.Indify.indifyFile(Indify.java:356) at indify.Indify.indifyTree(Indify.java:391) at indify.Indify.indifyTree(Indify.java:393) at indify.Indify.indifyTree(Indify.java:393) at indify.Indify.indifyTree(Indify.java:393) at indify.Indify.indifyTree(Indify.java:393) at indify.Indify.indifyTree(Indify.java:393) at indify.Indify.indifyTree(Indify.java:393) at indify.Indify.indify(Indify.java:339) at indify.Indify.run(Indify.java:191) at indify.Indify.main(Indify.java:101) - Changes requested by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18841#pullrequestreview-2068375431 PR Comment: https://git.openjdk.org/jdk/pull/18841#issuecomment-2122432164 PR Comment: https://git.openjdk.org/jdk/pull/18841#issuecomment-2122435605
Re: RFR: 8331865: Consolidate size and alignment checks in LayoutPath [v3]
> When creating a nested memory access var handle, we ensure that the segment
> is accessed at the correct alignment for the root layout being accessed. But
> we do not ensure that the segment has at least the size of the accessed root
> layout. Example:
>
>
> MemoryLayout LAYOUT = sequenceLayout(2, structLayout(JAVA_INT.withName("x"),
> JAVA_INT.withName("y")));
> VarHandle X_HANDLE = LAYOUT.varHandle(PathElement.sequenceElement(),
> PathElement.groupElement("x"));
>
>
> If I have a memory segment whose size is 12, I can successfully call X_HANDLE
> on it with index 1, like so:
>
>
> MemorySegment segment = Arena.ofAuto().allocate(12);
> int x = (int)X_HANDLE.get(segment, 0, 1);
>
>
> This seems incorrect: after all, the provided segment doesn't have enough
> space to fit _two_ elements of the nested structs.
>
> This PR adds checks to detect this kind of scenario earlier, thus improving
> usability. To achieve this we could, in principle, just tweak
> `LayoutPath::checkEnclosingLayout` and add the required size check.
>
> But doing so will add yet another redundant check on top of the other already
> added by [pull/19124](https://git.openjdk.org/jdk/pull/19124). Instead, this
> PR rethinks how memory segment var handles are created, in order to eliminate
> redundant checks.
>
> The main observation is that size and alignment checks depend on an
> _enclosing_ layout. This layout *might* be the very same value layout being
> accessed (this is the case when e.g. using `JAVA_INT.varHandle()`). But, in
> the general case, the accessed value layout and the enclosing layout might
> differ (e.g. think about accessing a struct field).
>
> Furthermore, the enclosing layout check depends on the _base_ offset at which
> the segment is accessed, _prior_ to any index computation that occurs if the
> accessed layout path has any open elements. It is important to notice that
> this base offset is only available when looking at the var handle that is
> returned to the user. For instance, an indexed var handle with coordinates:
>
>
> (MemorySegment, long /* base */, long /* index 1 */, long /* index 2 */, long
> /* index 3 */)
>
>
> Is obtained through adaptation, by taking a more basic var handle of the form:
>
>
> (MemorySegment, long /* offset */)
>
>
> And then injecting the result of the index multiplication into `offset`. As
> such, we can't add an enclosing layout check inside the var handle returned
> by `VarHandles::memorySegmentViewHandle`, as doing so will end up seeing the
> *wrong* offset (e.g. an offset in which the index part has already been mixed
> in)...
Maurizio Cimadamore has updated the pull request incrementally with one
additional commit since the last revision:
Fix typo in javadoc
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19251/files
- new: https://git.openjdk.org/jdk/pull/19251/files/a7b09d9d..14f0b8b3
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19251=02
- incr: https://webrevs.openjdk.org/?repo=jdk=19251=01-02
Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/19251.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19251/head:pull/19251
PR: https://git.openjdk.org/jdk/pull/19251
Re: RFR: 8331196: vector api: Remove unnecessary index check in Byte/ShortVector.fromArray/fromArray0Template
On Mon, 20 May 2024 19:21:44 GMT, Paul Sandoz wrote: >> Hi, >> Can you help to review this simple patch? >> Some index check in Byte/ShortVector.fromArray/fromArray0Template seems not >> necessary, could be removed. >> Thanks > > That does not look correct and will only check a prefix indexes. A > `ByteVector` with a shape of 256 bits has 32 lanes, whereas an `IntVector` of > the same shape has 8 lanes. The `mapOffset` array will hold 32 indexes that > need checking, so we need to loop through `mapOffset` array four times. Thanks @PaulSandoz for comment. I just re-ran the vector api tests with this patch on x64 and riscv64, but seems no failures triggered. Let me check further, either I missed something or maybe there is some gap in test to be filled. - PR Comment: https://git.openjdk.org/jdk/pull/18977#issuecomment-2122240453
Re: RFR: 8332086: Remove the usage of ServiceLoader in j.u.r.RandomGeneratorFactory [v8]
On Tue, 21 May 2024 09:06:16 GMT, Raffaello Giulietti wrote: >> All random number generator algorithms are implemented in module >> `java.base`. The usage of `ServiceLoader` in `j.u.r.RandomGeneratorFactory` >> is no longer needed. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > Prefer 'linkplain' to 'a href'. Marked as reviewed by jpai (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19212#pullrequestreview-2068098589
RFR: 8332597: Remove redundant methods from j.l.classfile.ClassReader API
j.l.classfile.ClassReader instance is exposed in the Class-File API through j.l.classfile.AttributeMapper::readAttribute method only. ClassReader only purpose is to serve as a tool for reading content of a custom attribute in a user-provided AttribtueMapper. It contains useful set of low-level class reading methods for user to implement a custom attribute content parser. However methods ClassReader::thisClassPos and ClassReader::skipAttributeHolder are not necessary for a custom attribute content parsing and so redundant in the API. Class-File API implementation internally use these methods, however they should not be exposed in the API. This patch removes the methods from the API. Please review. Thanks, Adam - Commit messages: - 8332597: Remove redundant methods from j.l.classfile.ClassReader API Changes: https://git.openjdk.org/jdk/pull/19323/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19323=00 Issue: https://bugs.openjdk.org/browse/JDK-8332597 Stats: 26 lines in 5 files changed: 0 ins; 21 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/19323.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19323/head:pull/19323 PR: https://git.openjdk.org/jdk/pull/19323
Re: RFR: 8332528: Generate code in SwitchBootstraps.generateTypeSwitch that require fewer adaptations [v3]
On Tue, 21 May 2024 09:01:32 GMT, Claes Redestad wrote: >> We can fold the call to `Objects.checkIndex` into the code generated in >> generateTypeSwitchSkeleton instead of doing so by filtering the MH argument. >> This loads 9 less classes (of which 8 generated LFs and Species classes) on >> a minimal test, while being neutral on a throughput sanity test: >> >> >> Name Cnt Base Error Test Error Unit Change >> SwitchSanity.switchSum 15 8,162 ± 0,117 8,152 ± 0,131 ns/op 1,00x (p = >> 0,800 ) >> * = significant >> ``` >> >> A few additional optimizations includes moving some seldom used `findStatic` >> calls to a holder. All in all this means a reduction by 22M cycles to >> bootstrap a trivial switch expression on my M1. > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Add type switch to HelloClasslist By adding a trivial type switch to `HelloClasslist` we archive a set of classes and avoid some runtime class generation. This grows the default CDS archive by approximately 1.2Mb by pulling in a large number of Classfile API classes, which is bound to happen soon anyway. On the SwitchSanity test, bootstrap cost drops by another 35M cycles: Wall clock:51.0 ms/op .taskclock:63.5 ms/op .cycles: 220064872 average cycles elapsed .instructions: 544981276 average instructions retired - PR Comment: https://git.openjdk.org/jdk/pull/19307#issuecomment-2122136668
Re: RFR: 8332086: Remove the usage of ServiceLoader in j.u.r.RandomGeneratorFactory [v8]
> All random number generator algorithms are implemented in module `java.base`. > The usage of `ServiceLoader` in `j.u.r.RandomGeneratorFactory` is no longer > needed. Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: Prefer 'linkplain' to 'a href'. - Changes: - all: https://git.openjdk.org/jdk/pull/19212/files - new: https://git.openjdk.org/jdk/pull/19212/files/880138d7..87813996 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19212=07 - incr: https://webrevs.openjdk.org/?repo=jdk=19212=06-07 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19212.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19212/head:pull/19212 PR: https://git.openjdk.org/jdk/pull/19212
Re: RFR: 8332528: Generate code in SwitchBootstraps.generateTypeSwitch that require fewer adaptations [v3]
> We can fold the call to `Objects.checkIndex` into the code generated in > generateTypeSwitchSkeleton instead of doing so by filtering the MH argument. > This loads 9 less classes (of which 8 generated LFs and Species classes) on a > minimal test, while being neutral on a throughput sanity test: > > > Name Cnt Base Error Test Error Unit Change > SwitchSanity.switchSum 15 8,162 ± 0,117 8,152 ± 0,131 ns/op 1,00x (p = > 0,800 ) > * = significant > ``` > > A few additional optimizations includes moving some seldom used `findStatic` > calls to a holder. All in all this means a reduction by 22M cycles to > bootstrap a trivial switch expression on my M1. Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Add type switch to HelloClasslist - Changes: - all: https://git.openjdk.org/jdk/pull/19307/files - new: https://git.openjdk.org/jdk/pull/19307/files/c212a3d5..f04d78ea Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19307=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19307=01-02 Stats: 11 lines in 1 file changed: 11 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19307.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19307/head:pull/19307 PR: https://git.openjdk.org/jdk/pull/19307
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]
On Tue, 21 May 2024 07:20:05 GMT, Alan Bateman wrote: > > Have you looked into / thought about how this will work for jpackaged apps > > ? I suspect that both the existing FFM usage and this will be options the > > application packager will need to supply when building the jpackaged app - > > the end user cannot pass in command line VM options. Seems there should be > > some testing of this as some kind of native access could be a common case > > for jpackaged apps. > > I don't see any tests in test/jdk/tools/jpackage that creates an application > that uses JNI code. Seems like a good idea to add this via another PR and it > specify --java-options so that the application launcher enables native > access. It could test jpackage using jlink too. These are all good suggestions. I have not looked into jpackage, but yes, I would expect that the jpackage user would need to provide extra options when packaging the application. The same is true for creating JDK image jlink (which we use in the jextract build) - although, in that case the end user also has the possibility to pass options on the command line. - PR Comment: https://git.openjdk.org/jdk/pull/19213#issuecomment-2122095444
Re: RFR: 8332547: Unloaded signature classes in DirectMethodHandles
On Mon, 20 May 2024 21:29:20 GMT, Vladimir Ivanov wrote: > JVM routinely installs loader constraints for unloaded signature classes when > method resolution takes place. MethodHandle resolution took a different route > and eagerly resolves signature classes instead (see > `java.lang.invoke.MemberName$Factory::resolve` and > `sun.invoke.util.VerifyAccess::isTypeVisible` for details). > > There's a micro-optimization which bypasses eager resolution for `java.*` > classes. The downside is that `java.*` signature classes can show up as > unloaded. It manifests as inlining failures during JIT-compilation and may > cause severe performance issues. > > Proposed fix removes the aforementioned special case logic during > `MethodHandle` resolution. > > In some cases it may slow down `MethodHandle` construction a bit (e.g., when > repeatedly constructing `DirectMethodHandle`s with lots of arguments), but > `MethodHandle` construction step is not performance critical. > > Testing: hs-tier1 - hs-tier4 Loading classes seems like a side-effect of `isTypeVisible`. I suggest adding a separate pass that explicitly loads all the signature classes in `MemberName$Factory::resolve`. I think that would make the intent clearer. - PR Comment: https://git.openjdk.org/jdk/pull/19319#issuecomment-2122077296
Re: RFR: 8332528: Generate code in SwitchBootstraps.generateTypeSwitch that require fewer adaptations [v2]
> We can fold the call to `Objects.checkIndex` into the code generated in > generateTypeSwitchSkeleton instead of doing so by filtering the MH argument. > This loads 9 less classes (of which 8 generated LFs and Species classes) on a > minimal test, while being neutral on a throughput sanity test: > > > Name Cnt Base Error Test Error Unit Change > SwitchSanity.switchSum 15 8,162 ± 0,117 8,152 ± 0,131 ns/op 1,00x (p = > 0,800 ) > * = significant > ``` > > A few additional optimizations includes moving some seldom used `findStatic` > calls to a holder. All in all this means a reduction by 22M cycles to > bootstrap a trivial switch expression on my M1. Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Revert bf68b1d, tidy up - Changes: - all: https://git.openjdk.org/jdk/pull/19307/files - new: https://git.openjdk.org/jdk/pull/19307/files/bf68b1d2..c212a3d5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19307=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19307=00-01 Stats: 27 lines in 1 file changed: 15 ins; 9 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19307.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19307/head:pull/19307 PR: https://git.openjdk.org/jdk/pull/19307
Re: RFR: 8331189: Implementation of Scoped Values (Third Preview)
On Wed, 8 May 2024 09:40:38 GMT, Alan Bateman wrote:
> JEP 481 proposes Scoped Values to continue to preview in JDK 23 with one
> change. The type of the operation parameter of the callWhere method is
> changed to a new functional interface to avoid having the API throw
> Exception. With that change, the getWhere (and the corresponding method on
> Carrier) are no longer needed. The functional interface is not proposed for
> j.u.function at this time.
src/java.base/share/classes/java/lang/ScopedValue.java line 500:
> 498: @PreviewFeature(feature = PreviewFeature.Feature.SCOPED_VALUES)
> 499: @FunctionalInterface
> 500: public interface CallableOp {
I feel like that a functional interface such as this one should be promoted to
a top‑level type in some common package by the time this feature exits preview.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19136#discussion_r1596041552
Re: RFR: 8331189: Implementation of Scoped Values (Third Preview)
On Wed, 8 May 2024 09:40:38 GMT, Alan Bateman wrote: > JEP 481 proposes Scoped Values to continue to preview in JDK 23 with one > change. The type of the operation parameter of the callWhere method is > changed to a new functional interface to avoid having the API throw > Exception. With that change, the getWhere (and the corresponding method on > Carrier) are no longer needed. The functional interface is not proposed for > j.u.function at this time. Thanks. That all looks reasonable enough, and gets us to the status quo ante + `ScopedValue.CallableOp`, which is way better than what we have right now. - Marked as reviewed by aph (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19136#pullrequestreview-2066514531
RFR: 8331189: Implementation of Scoped Values (Third Preview)
JEP 481 proposes Scoped Values to continue to preview in JDK 23 with one change. The type of the operation parameter of the callWhere method is changed to a new functional interface to avoid having the API throw Exception. With that change, the getWhere (and the corresponding method on Carrier) are no longer needed. The functional interface is not proposed for j.u.function at this time. - Commit messages: - Merge - Merge - Set JEP number - Sync up from loom repo - Merge - Initial commit Changes: https://git.openjdk.org/jdk/pull/19136/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19136=00 Issue: https://bugs.openjdk.org/browse/JDK-8331189 Stats: 298 lines in 7 files changed: 31 ins; 203 del; 64 mod Patch: https://git.openjdk.org/jdk/pull/19136.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19136/head:pull/19136 PR: https://git.openjdk.org/jdk/pull/19136
Re: RFR: 8332528: Generate code in SwitchBootstraps.generateTypeSwitch that require fewer adaptations
On Tue, 21 May 2024 00:16:51 GMT, Chen Liang wrote:
>> We can fold the call to `Objects.checkIndex` into the code generated in
>> generateTypeSwitchSkeleton instead of doing so by filtering the MH argument.
>> This loads 9 less classes (of which 8 generated LFs and Species classes) on
>> a minimal test, while being neutral on a throughput sanity test:
>>
>>
>> Name Cnt Base Error Test Error Unit Change
>> SwitchSanity.switchSum 15 8,162 ± 0,117 8,152 ± 0,131 ns/op 1,00x (p =
>> 0,800 )
>> * = significant
>> ```
>>
>> A few additional optimizations includes generating the switch method using
>> the precise type (to avoid the need for an explicitCast adaptation), and
>> moving some seldom used `findStatic` calls to a holder. All in all this
>> means a reduction by 33-34M cycles to bootstrap a trivial switch expression
>> on my M1.
>
> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 630:
>
>> 628:.withMethodBody("typeSwitch",
>> 629:
>> MethodTypeDescImpl.ofValidated(ConstantDescs.CD_int,
>> 630:
>> ClassDesc.ofDescriptor(selectorType.descriptorString()),
>
> `selectorType` can be primitive, therefore verification errors arise at
> `aload` in the generated code. This is why tests fail on GitHub actions.
Dang. I had focused on making the `java/lang/runtime` tests pass locally,
thinking that'd be sufficient. Luckily bf68b1d had only a minor benefit.
Backing it out and tidying up a bit gives me these numbers:
Wall clock:65.0 ms/op
.taskclock:75.0 ms/op
.cycles: 255323158 average cycles elapsed
.instructions: 626086160 average instructions retired
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19307#discussion_r1607874407
Integrated: 8311175: Move BufWriter::asByteBuffer to BufWriterImpl
On Fri, 30 Jun 2023 14:43:36 GMT, Chen Liang wrote: > As discussed on the mailing list > https://mail.openjdk.org/pipermail/classfile-api-dev/2023-June/000381.html, > BufWriter::asByteBuffer has a behavior not suitable for API and is only used > by internal StackMapGenerator/StackCounter, so it will be converted to an > internal API. > > Somehow the ByteBuffer needs to be sliced, or StackMapGenerator encounters > IOOBE. Not sure what the exact cause was. This pull request has now been integrated. Changeset: 414a7fdc Author:Chen Liang Committer: Adam Sotona URL: https://git.openjdk.org/jdk/commit/414a7fdc5e4aae4cec25b0847bb7c163f271b4e0 Stats: 11 lines in 4 files changed: 0 ins; 7 del; 4 mod 8311175: Move BufWriter::asByteBuffer to BufWriterImpl Reviewed-by: asotona - PR: https://git.openjdk.org/jdk/pull/14736
Integrated: 8332486: ClassFile API ArrayIndexOutOfBoundsException with label metadata
On Mon, 20 May 2024 08:37:49 GMT, Adam Sotona wrote: > Parsing of a specifically corrupted class file cause unexpected > `ArrayIndexOutOfBoundsException` during label inflation. > This patch checks the valid range and throws expected > `IllegalArgumentException` instead. > Relevant test is added. > > Please review. > > Thanks, > Adam This pull request has now been integrated. Changeset: 451cc239 Author:Adam Sotona URL: https://git.openjdk.org/jdk/commit/451cc239050f097060be927171fe0e46962f3356 Stats: 19 lines in 2 files changed: 18 ins; 0 del; 1 mod 8332486: ClassFile API ArrayIndexOutOfBoundsException with label metadata Reviewed-by: psandoz - PR: https://git.openjdk.org/jdk/pull/19304
Re: RFR: 8331670: Deprecate the Memory-Access Methods in sun.misc.Unsafe for Removal [v3]
> This is the implementation changes for JEP 471. > > The methods in sun.misc.Unsafe for on-heap and off-heap access are deprecated > for removal. This means a removal warning at compile time. No methods have > been removed. A deprecated message is added to each of the methods but > unlikely to be seen as the JDK does not generate or publish the API docs for > this class. > > A new command line option --sun-misc-unsafe-memory-access=$value is > introduced to allow or deny access to these methods. The default proposed for > JDK 23 is "allow" so no change in behavior compared to JDK 22 or previous > releases. > > A new test is added to test the command line option settings. The existing > micros for FFM that use Unsafe are updated to suppress the removal warning at > compile time. A new micro is introduced with a small sample of methods to > ensure the changes doesn't cause any perf regressions ([sample > results](https://cr.openjdk.org/~alanb/8331670-results.txt)). > > For now, the changes include the update to the man page for the "java" > command. It might be that this has to be separated out so that it goes with > other updates in the release. Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits: - Merge - Add removal to DISABLED_WARNINGS Fail at startup if bad value specified to option - Merge - Update man page - Update man page - Fix comment - Merge - Merge - Whitespace - Initial commit - Changes: https://git.openjdk.org/jdk/pull/19174/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19174=02 Stats: 1234 lines in 8 files changed: 1150 ins; 5 del; 79 mod Patch: https://git.openjdk.org/jdk/pull/19174.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19174/head:pull/19174 PR: https://git.openjdk.org/jdk/pull/19174
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v11]
On Fri, 17 May 2024 21:16:47 GMT, Volodymyr Paprotski wrote: >> Performance. Before: >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt ScoreError Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt3 6443.934 ± 6.491 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt3 6152.979 ± 4.954 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt3 1895.410 ± 36.979 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt3 1878.955 ± 45.487 ops/s >> Benchmark(algorithm) >> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units >> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1357.810 ± 26.584 ops/s >> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1352.119 ± 23.547 ops/s >> Benchmark (isMontBench) Mode Cnt Score >> Error Units >> PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ± >> 10.970 ops/s >> >> Performance, no intrinsic: >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt3 6529.839 ± 42.420 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt3 6199.747 ± 133.566 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt3 1973.676 ± 54.071 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt3 1932.127 ± 35.920 ops/s >> Benchmark(algorithm) >> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units >> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1355.788 ± 29.858 ops/s >> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1346.523 ± 28.722 ops/s >> Benchmark (isMontBench) Mode Cnt Score >> Error Units >> PolynomialP256Bench.benchMultiply true thrpt3 1919.57... > > Volodymyr Paprotski has updated the pull request incrementally with one > additional commit since the last revision: > > shenandoah verifier I'm getting some conflicts when trying to apply this to master. Could you please merge the PR? - PR Comment: https://git.openjdk.org/jdk/pull/18583#issuecomment-2121929550
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]
On Mon, 20 May 2024 18:47:35 GMT, Phil Race wrote: > Have you looked into / thought about how this will work for jpackaged apps ? > I suspect that both the existing FFM usage and this will be options the > application packager will need to supply when building the jpackaged app - > the end user cannot pass in command line VM options. Seems there should be > some testing of this as some kind of native access could be a common case for > jpackaged apps. I don't see any tests in test/jdk/tools/jpackage that creates an application that uses JNI code. Seems like a good idea to add this via another PR and it specify --java-options so that the application launcher enables native access. It could test jpackage using jlink too. - PR Comment: https://git.openjdk.org/jdk/pull/19213#issuecomment-2121927727
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v11]
On Fri, 17 May 2024 21:16:47 GMT, Volodymyr Paprotski wrote: >> Performance. Before: >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt ScoreError Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt3 6443.934 ± 6.491 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt3 6152.979 ± 4.954 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt3 1895.410 ± 36.979 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt3 1878.955 ± 45.487 ops/s >> Benchmark(algorithm) >> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units >> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1357.810 ± 26.584 ops/s >> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1352.119 ± 23.547 ops/s >> Benchmark (isMontBench) Mode Cnt Score >> Error Units >> PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ± >> 10.970 ops/s >> >> Performance, no intrinsic: >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt3 6529.839 ± 42.420 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt3 6199.747 ± 133.566 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt3 1973.676 ± 54.071 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt3 1932.127 ± 35.920 ops/s >> Benchmark(algorithm) >> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units >> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1355.788 ± 29.858 ops/s >> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1346.523 ± 28.722 ops/s >> Benchmark (isMontBench) Mode Cnt Score >> Error Units >> PolynomialP256Bench.benchMultiply true thrpt3 1919.57... > > Volodymyr Paprotski has updated the pull request incrementally with one > additional commit since the last revision: > > shenandoah verifier I'll send this through our testing and will report back once it passed. - PR Comment: https://git.openjdk.org/jdk/pull/18583#issuecomment-2121914071
Integrated: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr
On Mon, 20 May 2024 06:41:45 GMT, Axel Boldt-Christmas wrote: > Improve `java/util/zip/EntryCount64k.java` stderr parsing by ignoring > deprecated warnings. Testing non-generational ZGC requires the use of a > deprecated option. This pull request has now been integrated. Changeset: f5ab7dff Author:Axel Boldt-Christmas URL: https://git.openjdk.org/jdk/commit/f5ab7dff402a3152f5d5736cc6521b4be617eccf Stats: 21 lines in 2 files changed: 18 ins; 0 del; 3 mod 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr Reviewed-by: jpai, stefank, dholmes - PR: https://git.openjdk.org/jdk/pull/19297
Re: RFR: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr [v2]
On Mon, 20 May 2024 07:27:19 GMT, Axel Boldt-Christmas wrote: >> Improve `java/util/zip/EntryCount64k.java` stderr parsing by ignoring >> deprecated warnings. Testing non-generational ZGC requires the use of a >> deprecated option. > > Axel Boldt-Christmas has updated the pull request incrementally with one > additional commit since the last revision: > > Update copyright year Thanks for the reviews. - PR Comment: https://git.openjdk.org/jdk/pull/19297#issuecomment-2121826396
Integrated: 8332495: java/util/logging/LoggingDeadlock2.java fails with AssertionError: Some tests failed
On Mon, 20 May 2024 06:42:19 GMT, Axel Boldt-Christmas wrote: > Improve ` java/util/logging/LoggingDeadlock2.java` stderr parsing by ignoring > deprecated warnings. Testing non-generational ZGC requires the use of a > deprecated option. This pull request has now been integrated. Changeset: 9f777930 Author:Axel Boldt-Christmas URL: https://git.openjdk.org/jdk/commit/9f7779305c4ccbb86bb0e6d0ed8bc92a4b8f3b9d Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8332495: java/util/logging/LoggingDeadlock2.java fails with AssertionError: Some tests failed Reviewed-by: jpai, stefank - PR: https://git.openjdk.org/jdk/pull/19298
Re: RFR: 8332495: java/util/logging/LoggingDeadlock2.java fails with AssertionError: Some tests failed [v2]
On Mon, 20 May 2024 07:25:12 GMT, Axel Boldt-Christmas wrote: >> Improve ` java/util/logging/LoggingDeadlock2.java` stderr parsing by >> ignoring deprecated warnings. Testing non-generational ZGC requires the use >> of a deprecated option. > > Axel Boldt-Christmas has updated the pull request incrementally with one > additional commit since the last revision: > > Update copyright year Thanks for the reviews. - PR Comment: https://git.openjdk.org/jdk/pull/19298#issuecomment-2121825778
