Re: RFR: 8320786: Remove ThreadGroup.stop [v2]
> ThreadGroup.stop was deprecated since JDK 1.2, deprecated for removal in Java > 18, and re-specified/degraded to throw UnsupportedOperationException > unconditionally in Java 20. Early in Java 23 seems a fine time to finally > remove this method. Corpus analysis of 176 million classes in 485k artifacts > found residual usages in 14 artifacts, so not many. > > It would be nice if we could remove Thread.stop at the same time. Sadly there > are still quite a few artifacts containing code (and in some cases tests) > that reference this method. We will have to come back to this in some future > 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 two commits: - Merge - Initial commit - Changes: https://git.openjdk.org/jdk/pull/16828/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16828&range=01 Stats: 17 lines in 2 files changed: 0 ins; 17 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16828.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16828/head:pull/16828 PR: https://git.openjdk.org/jdk/pull/16828
Integrated: 8320532: Remove Thread/ThreadGroup suspend/resume
On Thu, 23 Nov 2023 09:18:44 GMT, Alan Bateman wrote: > The deadlock prone Thread/ThreadGroup suspend/resume were deprecated since > JDK 1.2, deprecated for removal in Java 14, and re-specified/degraded to > throw UnsupportedOperationException unconditionally in Java 19/20. Early in > Java 23 seems a fine time to finally remove these methods. > > Corpus analysis of 176 million classes in 485k artifacts found no remaining > usages of ThreadGroup.suspend/resume beyond the artifacts that include a copy > of java.lang.ThreadGroup (!). It found 87 remaining uses of Thread.suspend > and 86 remaining usages of Thread.resume, some of these are tests. > > Thread.suspend/resume have always linked to the "Java Thread Primitive > Deprecation" page. This originally explained the reasons why suspend/resume > were deprecated. When these methods were degraded to throw UOE we changed the > text to explain why the ability to suspend or resume a thread was removed. > Now we must change it again. One choice is to re-word to explain why the Java > APIs were removed or why the Java APIs don't define a way to suspend/resume > threads, the other choice (which I prefer) is to remove the text. > > The method description of java.lang.management.ThreadInfo.isSuspended is > tweaked to link to JVMTI SuspendThread instead of Thread.suspend This pull request has now been integrated. Changeset: af5c4922 Author:Alan Bateman URL: https://git.openjdk.org/jdk/commit/af5c49226c3416a9c3bdde06cac2076acf9de5c3 Stats: 553 lines in 9 files changed: 116 ins; 430 del; 7 mod 8320532: Remove Thread/ThreadGroup suspend/resume Reviewed-by: dholmes, jpai, sspitsyn, smarks - PR: https://git.openjdk.org/jdk/pull/16789
Re: RFR: 8320532: Remove Thread/ThreadGroup suspend/resume [v2]
On Fri, 8 Dec 2023 06:39:59 GMT, Alan Bateman wrote: >> The deadlock prone Thread/ThreadGroup suspend/resume were deprecated since >> JDK 1.2, deprecated for removal in Java 14, and re-specified/degraded to >> throw UnsupportedOperationException unconditionally in Java 19/20. Early in >> Java 23 seems a fine time to finally remove these methods. >> >> Corpus analysis of 176 million classes in 485k artifacts found no remaining >> usages of ThreadGroup.suspend/resume beyond the artifacts that include a >> copy of java.lang.ThreadGroup (!). It found 87 remaining uses of >> Thread.suspend and 86 remaining usages of Thread.resume, some of these are >> tests. >> >> Thread.suspend/resume have always linked to the "Java Thread Primitive >> Deprecation" page. This originally explained the reasons why suspend/resume >> were deprecated. When these methods were degraded to throw UOE we changed >> the text to explain why the ability to suspend or resume a thread was >> removed. Now we must change it again. One choice is to re-word to explain >> why the Java APIs were removed or why the Java APIs don't define a way to >> suspend/resume threads, the other choice (which I prefer) is to remove the >> text. >> >> The method description of java.lang.management.ThreadInfo.isSuspended is >> tweaked to link to JVMTI SuspendThread instead of Thread.suspend > > Alan Bateman 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 four additional > commits since the last revision: > > - Update copyright year > - Merge > - Clarify ThreadInfo.isSuspended > - Initial commit Marked as reviewed by jpai (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/16789#pullrequestreview-1771631210
Re: RFR: 8320532: Remove Thread/ThreadGroup suspend/resume [v2]
> The deadlock prone Thread/ThreadGroup suspend/resume were deprecated since > JDK 1.2, deprecated for removal in Java 14, and re-specified/degraded to > throw UnsupportedOperationException unconditionally in Java 19/20. Early in > Java 23 seems a fine time to finally remove these methods. > > Corpus analysis of 176 million classes in 485k artifacts found no remaining > usages of ThreadGroup.suspend/resume beyond the artifacts that include a copy > of java.lang.ThreadGroup (!). It found 87 remaining uses of Thread.suspend > and 86 remaining usages of Thread.resume, some of these are tests. > > Thread.suspend/resume have always linked to the "Java Thread Primitive > Deprecation" page. This originally explained the reasons why suspend/resume > were deprecated. When these methods were degraded to throw UOE we changed the > text to explain why the ability to suspend or resume a thread was removed. > Now we must change it again. One choice is to re-word to explain why the Java > APIs were removed or why the Java APIs don't define a way to suspend/resume > threads, the other choice (which I prefer) is to remove the text. > > The method description of java.lang.management.ThreadInfo.isSuspended is > tweaked to link to JVMTI SuspendThread instead of Thread.suspend Alan Bateman 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 four additional commits since the last revision: - Update copyright year - Merge - Clarify ThreadInfo.isSuspended - Initial commit - Changes: - all: https://git.openjdk.org/jdk/pull/16789/files - new: https://git.openjdk.org/jdk/pull/16789/files/cc17ea42..517de309 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16789&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16789&range=00-01 Stats: 106040 lines in 2348 files changed: 55620 ins; 41750 del; 8670 mod Patch: https://git.openjdk.org/jdk/pull/16789.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16789/head:pull/16789 PR: https://git.openjdk.org/jdk/pull/16789
Re: RFR: 8310994: Add JFR event for selection operations
On Wed, 22 Nov 2023 12:08:08 GMT, Daniel Fuchs wrote: > It could also be interesting to provide the `timeout` that was given to the > selection operation. I've tried to work through issues, esp. around selector spinning, and being able to distinguish select from selectNow is important for all of them, so yes, the timeout is needed or else no emit when the timeout == 0 as that's the case you have to filter out when troubleshooting. - PR Review Comment: https://git.openjdk.org/jdk/pull/16710#discussion_r1419979779
Re: RFR: JDK-8316708: Augment WorstCaseTests with more cases
On Fri, 22 Sep 2023 05:36:02 GMT, Joe Darcy wrote: > A new paper > > "Accuracy of Mathematical Functions in Single, Double, Double Extended, and > Quadruple Precision" > by Brian Gladman, Vincenzo Innocente and Paul Zimmermann > https://members.loria.fr/PZimmermann/papers/accuracy.pdf > > details the inputs with generate the worst-case observed errors in different > math library implementations. The FDLIBM-related worst cases should be added > to the test suite. Still keep alive. - PR Comment: https://git.openjdk.org/jdk/pull/15879#issuecomment-1846597255
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v11]
On Thu, 7 Dec 2023 22:06:14 GMT, Vladimir Yaroslavskiy wrote: >> > xmlns:o="urn:schemas-microsoft-com:office:office" >> xmlns:x="urn:schemas-microsoft-com:office:excel" >> xmlns="http://www.w3.org/TR/REC-html40";> >> >> >> >> >> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip.htm"> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip_filelist.xml"> >> >> >> >> >> >> >> >> >> Benchmark (us/op) | Builder | (size) | Stock JDK (+ AVX512 sort) | >> DPQS_r01 (+ AVX512 sort) | Speedup >> -- | -- | -- | -- | -- | -- >> ArraysSort.Int.testSort | RANDOM | 600 | 2.256 | 1.713 | 1.32 >> ArraysSort.Int.testSort | RANDOM | 9000 | 41.457 | 38.316 | 1.08 >> ArraysSort.Int.testSort | RANDOM | 2 | 98.448 | 86.376 | 1.14 >> ArraysSort.Int.testSort | RANDOM | 40 | 2820.939 | 2792.333 | 1.01 >> ArraysSort.Int.testSort | RANDOM | 300 | 23426.411 | 23711.885 | 0.99 >> ArraysSort.Int.testSort | REPEATED | 600 | 1.032 | 0.859 | 1.20 >> ArraysSort.Int.testSort | REPEATED | 9000 | 5.114 | 5.014 | 1.02 >> ArraysSort.Int.testSort | REPEATED | 2 | 10.3 | 9.532 | 1.08 >> ArraysSort.Int.testSort | REPEATED | 40 | 210.742 | 235.281 | 0.90 >> ArraysSort.Int.testSort | REPEATED | 300 | 1948.589 | 1955.258 | 1.00 >> ArraysSort.Int.testSort | STAGGER | 600 | 2.125 | 2.157 | 0.99 >> ArraysSort.Int.testSort | STAGGER | 9000 | 29.86 | 29.931 | 1.00 >> ArraysSort.Int.testSort | STAGGER | 2 | 67.096 | 66.543 | 1.01 >> ArraysSort.Int.testSort | STAGGER | 40 | 1247.53 | 1224.999 | 1.02 >> ArraysSort.Int.testSort | STAGGER | 300 | 9435.404 | 9495.189 | 0.99 >> ArraysSort.Int.testSort | SHUFFLE | 600 | 2.701 | 1.64 | 1.65 >> ArraysSort.Int.testSort | SHUFFLE | 9000 | 38.976 | 34.201 | 1.14 >> ArraysSort.Int.testSort | SHUFFLE | 2 | 96.399 | 79.616 | 1.21 >> ArraysSort.Int.testSort | SHUFFLE | 40 | 2566.338 | 2436.271 | 1.05 >> ArraysSort.Int.testSort | SHUFFLE | 300 | 20835.935 | 20071.12 | 1.04 >> >> >> >> >> >> > > Hello Vamsi (@vamsi-parasa), > > Did you have a chance to run benchmarking? Hi Vladimir (@iaroslavski), Please see the data below. Thanks, Vamsi http://www.w3.org/TR/REC-html40";> Benchmark (us/op) | (builder) | (size) | Stock JDK | r_02 | r_03 | r_04 | r_05 | r_06 | r_07 | r_08 | r_98 | r_99 -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- ArraysSort.Int.testSort | RANDOM | 600 | 2.256 | 1.634 | 1.651 | 1.659 | 1.671 | 1.646 | 1.611 | 1.661 | 1.642 | 1.671 ArraysSort.Int.testSort | RANDOM | 9000 | 41.457 | 37.697 | 38.075 | 37.927 | 39.693 | 38.989 | 37.86 | 38.163 | 39.222 | 38.835 ArraysSort.Int.testSort | RANDOM | 2 | 98.448 | 91.494 | 89.683 | 87.971 | 90.231 | 90.141 | 90.515 | 90.415 | 89.571 | 90.308 ArraysSort.Int.testSort | RANDOM | 40 | 2820.939 | 2816.5 | 2811.334 | 2833.15 | 2802.958 | 2813.012 | 2815.24 | 2825.526 | 2801.497 | 2816.25 ArraysSort.Int.testSort | RANDOM | 300 | 23426.411 | 23661.09 | 23778.15 | 23748.91 | 23802.62 | 23746.3 | 23778.16 | 23631.1 | 23651.78 | 23859.91 ArraysSort.Int.testSort | REPEATED | 600 | 1.032 | 0.929 | 0.955 | 0.944 | 0.927 | 0.928 | 0.953 | 0.918 | 0.934 | 0.93 ArraysSort.Int.testSort | REPEATED | 9000 | 5.114 | 5.059 | 4.832 | 5.162 | 4.965 | 4.973 | 5.518 | 5.003 | 5.435 | 4.971 ArraysSort.Int.testSort | REPEATED | 2 | 10.3 | 12.238 | 12.474 | 12.482 | 12.351 | 12.338 | 12.372 | 12.394 | 12.688 | 13.477 ArraysSort.Int.testSort | REPEATED | 40 | 210.742 | 261.709 | 264.572 | 263.203 | 260.822 | 260.475 | 262.03 | 260.356 | 265.976 | 264.273 ArraysSort.Int.testSort | REPEATED | 300 | 1948.589 | 2062.235 | 2079.128 | 2065.445 | 2053.24 | 2076.278 | 2049.799 | 2059.1 | 2073.191 | 2075.65 ArraysSort.Int.testSort | STAGGER | 600 | 2.125 | 2.001 | 2.023 | 2.021 | 2.001 | 2.018 | 2.011 | 2.017 | 2.005 | 2.011 ArraysSort.Int.testSort | STAGGER | 9000 | 29.86 | 26.169 | 26.093 | 25.562 | 26.385 | 26.109 | 26.485 | 26.375 | 26.412 | 25.712 ArraysSort.Int.testSort | STAGGER | 2 | 67.096 | 77.157 | 63.636 | 64.479 | 58.697 | 59.728 | 58.913 | 59.482 | 58.633 | 76.904 ArraysSort.Int.testSort | STAGGER | 40 | 1247.53 | 1271.293 | 1236.158 | 1240.29 | 1261.469 | 1233.526 | 1153.822 | 1255.238 | 1224.071 | 1235.624 ArraysSort.Int.testSort | STAGGER | 300 | 9435.404 | 9612.98 | 9597.262 | 9590.393 | 9592.343 | 9616.005 | 9591.057 | 9637.881 | 9596.932 | 9570.482 ArraysSort.Int.testSort | SHUFFLE | 600 | 2.701 | 1.678 | 1.66 | 1.676 | 1.694 | 1.704 | 1.693 | 1.686 | 1.675 | 1.699 ArraysSort.Int.testSort | SHUFFLE | 9000 | 38.976 | 35.146 | 34.879 | 34.723 | 35.093 | 35.904 | 35.672 | 35.124 | 34.626 | 35.553 ArraysSort.Int.testSort | SHUFFLE | 2 | 96.399 | 81.651 | 83.113 | 81.186 | 80.802 | 82.464 | 81.473 | 83.511 | 82.289 | 81.794 ArraysSort.Int.testSort | SHUFFLE | 40 | 2566.338 | 2446.738 | 2424.526 | 2433.211 | 2459.019 | 2446.518 | 2450.989 | 2447.125 | 2449.441 | 2444.414
Re: RFR: 8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable [v2]
> This fix is for JDK 23 but the intention is to back port it to 22 in RDP-1 > time frame. > It is fixing a deadlock issue between `VirtualThread` class critical sections > with the `interruptLock` (in methods: `unpark()`, `interrupt()`, > `getAndClearInterrupt()`, `threadState()`, `toString()`), > `JvmtiVTMSTransitionDisabler` and JVMTI `Suspend/Resume` mechanisms. > The deadlocking scenario is well described by Patricio in a bug report > comment. > In simple words, a virtual thread should not be suspended during > 'interruptLock' critical sections. > > The fix is to record that a virtual thread is in a critical section > (`JavaThread`'s `_in_critical_section` bit) by notifying the VM/JVMTI about > begin/end of critical section. > This bit is used in `HandshakeState::get_op_for_self()` to filter out any > `HandshakeOperation` if a target `JavaThread` is in a critical section. > > Some of new notifications with `notifyJvmtiSync()` method is on a performance > critical path. It is why this method has been intrincified. > > New test was developed by Patricio: > `test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendWithInterruptLock` > The test is very nice as it reliably in 100% reproduces the deadlock without > the fix. > The test is never failing with this fix. > > Testing: > - tested with newly added test: > `test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendWithInterruptLock` > - tested with mach5 tiers 1-6 Serguei Spitsyn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits: - Resolved merge conflict in VirtualThread.java - added @summary to new test SuspendWithInterruptLock.java - add new test SuspendWithInterruptLock.java - 8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable - Changes: https://git.openjdk.org/jdk/pull/17011/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17011&range=01 Stats: 183 lines in 15 files changed: 178 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/17011.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17011/head:pull/17011 PR: https://git.openjdk.org/jdk/pull/17011
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
On Thu, 7 Dec 2023 09:30:01 GMT, Xiaohong Gong wrote: >> Currently the vector floating-point math APIs like >> `VectorOperators.SIN/COS/TAN...` are not intrinsified on AArch64 platform, >> which causes large performance gap on AArch64. Note that those APIs are >> optimized by C2 compiler on X86 platforms by calling Intel's SVML code [1]. >> To close the gap, we would like to optimize these APIs for AArch64 by >> calling a third-party vector library called libsleef [2], which are >> available in mainstream Linux distros (e.g. [3] [4]). >> >> SLEEF supports multiple accuracies. To match Vector API's requirement and >> implement the math ops on AArch64, we 1) call 1.0 ULP accuracy with FMA >> instructions used stubs in libsleef for most of the operations by default, >> and 2) add the vector calling convention to apply with the runtime calls to >> stub code in libsleef. Note that for those APIs that libsleef does not >> support 1.0 ULP, we choose 0.5 ULP instead. >> >> To help loading the expected libsleef library, this patch also adds an >> experimental JVM option (i.e. `-XX:UseSleefLib`) for AArch64 platforms. >> People can use it to denote the libsleef path/name explicitly. By default, >> it points to the system installed library. If the library does not exist or >> the dynamic loading of it in runtime fails, the math vector ops will >> fall-back to use the default scalar version without error. But a warning is >> printed out if people specifies a nonexistent library explicitly. >> >> Note that this is a part of the original proposed patch in panama-dev [5], >> just with some initial review comments addressed. And now we'd like to get >> some wider feedbacks from more hotspot experts. >> >> [1] https://github.com/openjdk/jdk/pull/3638 >> [2] https://sleef.org/ >> [3] https://packages.fedoraproject.org/pkgs/sleef/sleef/ >> [4] https://packages.debian.org/bookworm/libsleef3 >> [5] https://mail.openjdk.org/pipermail/panama-dev/2022-December/018172.html > > Xiaohong Gong has updated the pull request incrementally with one additional > commit since the last revision: > > Fix potential attribute issue > Build changes finally look good. Great, actually! Thanks for persisting, > despite the many rounds of review. > > You will still need the 2 hotspot reviews for the hotspot part of the patch. > > /reviewers 3 Thanks for the review and all the comments! - PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1846330893
Re: RFR: 8319577: x86_64 AVX2 intrinsics for Arrays.sort methods (int, float arrays) [v12]
On Fri, 8 Dec 2023 00:33:49 GMT, Srinivas Vamsi Parasa wrote: > > Testing have only one failure in closed tests and I need to fix it before > > this can be pushed. > > Thanks Vladimir for the update. Is the test failure because of this PR? Yes. One of our test, which checks integrity of built JDK, is confused by changes in libsimdsort.so. - PR Comment: https://git.openjdk.org/jdk/pull/16534#issuecomment-1846326542
Re: RFR: 8319577: x86_64 AVX2 intrinsics for Arrays.sort methods (int, float arrays) [v12]
On Wed, 6 Dec 2023 23:12:13 GMT, Srinivas Vamsi Parasa wrote: >> The goal is to develop faster sort routines for x86_64 CPUs by taking >> advantage of AVX2 instructions. This enhancement provides an order of >> magnitude speedup for Arrays.sort() using int, long, float and double arrays. >> >> For serial sort on random data, this PR shows upto ~7.5x improvement for >> 32-bit datatypes (int, float) on Intel TigerLake machine as shown in the >> performance data below. >> >> For parallel sort on random data, this PR shows upto ~3.4x for 32-bit >> datatypes (int, float) as shown below. >> >> **Note:** This PR also improves the performance of AVX512 sort by upto 35%. >> >> > xmlns:o="urn:schemas-microsoft-com:office:office" >> xmlns:x="urn:schemas-microsoft-com:office:excel" >> xmlns="http://www.w3.org/TR/REC-html40";> >> >> >> >> >> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip.htm"> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip_filelist.xml"> >> >> >> >> >> >> >> >> >> Benchmark (Serial Sort) | Size | Baseline (us/op) | AVX2 (us/op) | >> Speedup >> -- | -- | -- | -- | -- >> ArraysSort.intSort | 10 | 0.034 | 0.029 | 1.2 >> ArraysSort.intSort | 25 | 0.088 | 0.044 | 2.0 >> ArraysSort.intSort | 50 | 0.239 | 0.159 | 1.5 >> ArraysSort.intSort | 75 | 0.417 | 0.27 | 1.5 >> ArraysSort.intSort | 100 | 0.572 | 0.265 | 2.2 >> ArraysSort.intSort | 1000 | 10.098 | 4.282 | 2.4 >> ArraysSort.intSort | 1 | 330.065 | 43.383 | 7.6 >> ArraysSort.intSort | 10 | 4099.527 | 778.943 | 5.3 >> ArraysSort.intSort | 100 | 49150.16 | 9634.335 | 5.1 >> ArraysSort.floatSort | 10 | 0.045 | 0.043 | 1.0 >> ArraysSort.floatSort | 25 | 0.105 | 0.073 | 1.4 >> ArraysSort.floatSort | 50 | 0.278 | 0.216 | 1.3 >> ArraysSort.floatSort | 75 | 0.476 | 0.241 | 2.0 >> ArraysSort.floatSort | 100 | 0.583 | 0.313 | 1.9 >> ArraysSort.floatSort | 1000 | 10.182 | 4.329 | 2.4 >> ArraysSort.floatSort | 1 | 323.136 | 57.175 | 5.7 >> ArraysSort.floatSort | 10 | 4299.519 | 862.63 | 5.0 >> ArraysSort.floatSort | 100 | 50889.4 | 10972.19 | 4.6 >> >> >> >> >> >> > xmlns:o="urn:schemas-microsoft-com:office:office" >> xmlns:x="urn:schemas-microsoft-com:office:excel" >> xmlns="http://www.w3.org/TR/REC-html40";> >> >> >> >> > Srinivas Vamsi Parasa has updated the pull request incrementally with one > additional commit since the last revision: > > Revert "Change supported intrinsic check" > > This reverts commit 9621eb045c2958582f81ec06b237789a07481ddd. Testing have only one failure in closed tests and I need to fix it before this can be pushed. - PR Comment: https://git.openjdk.org/jdk/pull/16534#issuecomment-1846315767
Re: RFR: 8319577: x86_64 AVX2 intrinsics for Arrays.sort methods (int, float arrays) [v12]
On Fri, 8 Dec 2023 00:31:26 GMT, Vladimir Kozlov wrote: > Testing have only one failure in closed tests and I need to fix it before > this can be pushed. Thanks Vladimir for the update. Is the test failure because of this PR? - PR Comment: https://git.openjdk.org/jdk/pull/16534#issuecomment-1846317507
Re: RFR: 8321480: ISO 4217 Amendment 176 Update
On Thu, 7 Dec 2023 19:43:14 GMT, Justin Lu wrote:
> Please review this PR which incorporates the ISO 4217 Amendment 176 Update.
> As the replacement of `ANG` to `XCG` won't occur until 2025, this change does
> not need to go into JDK22. `HR` was also updated to remove the past cutover
> dates.
>
> An existing test in _ValidateISO4217.java_ checked that
> `Currency::getAvailableCurrencies` had all the expected currencies. This
> method returns all currencies, including ones to take place in the future
> (e.g. `XCG`). The expected currencies `Set` the method was test against had
> to be updated to also include future currencies as well.
>
> Additionally, this change also converted a parameterized test to a normal
> JUnit test, due to output overflow.
src/java.base/share/classes/sun/util/resources/CurrencyNames.properties line
497:
> 495: xbd=European Unit of Account (XBD)
> 496: xcd=East Caribbean Dollar
> 497: xcg=Caribbean Guilder
I think `XCG=XCG` is also needed for not throwing `MissingResourceException`
for `getSymbol()`
test/jdk/java/util/Currency/ValidateISO4217.java line 181:
> 179: // without updating ISO4217Codes
> 180: String futureCurr = tokens.nextToken();
> 181: testCurrencies.add(Currency.getInstance(futureCurr));
I'd not add the future currency, and fix it in the code not to add future
currency in available currencies.
test/jdk/java/util/Currency/ValidateISO4217.java line 289:
> 287: assertNull(Currency.getInstance(Locale.of("", country)),
> 288: "Error: Currency.getInstance() for this locale
> should return null: " + country);
> 289: }
What is this change for?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17023#discussion_r1419721244
PR Review Comment: https://git.openjdk.org/jdk/pull/17023#discussion_r1419722063
PR Review Comment: https://git.openjdk.org/jdk/pull/17023#discussion_r1419722636
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v11]
On Tue, 28 Nov 2023 00:35:48 GMT, Srinivas Vamsi Parasa wrote: >> Laurent Bourgès has updated the pull request incrementally with one >> additional commit since the last revision: >> >> add @SuppressWarnings (serial) > > xmlns:o="urn:schemas-microsoft-com:office:office" > xmlns:x="urn:schemas-microsoft-com:office:excel" > xmlns="http://www.w3.org/TR/REC-html40";> > > > > > > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip.htm"> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip_filelist.xml"> > > > > > > > > > Benchmark (us/op) | Builder | (size) | Stock JDK (+ AVX512 sort) | > DPQS_r01 (+ AVX512 sort) | Speedup > -- | -- | -- | -- | -- | -- > ArraysSort.Int.testSort | RANDOM | 600 | 2.256 | 1.713 | 1.32 > ArraysSort.Int.testSort | RANDOM | 9000 | 41.457 | 38.316 | 1.08 > ArraysSort.Int.testSort | RANDOM | 2 | 98.448 | 86.376 | 1.14 > ArraysSort.Int.testSort | RANDOM | 40 | 2820.939 | 2792.333 | 1.01 > ArraysSort.Int.testSort | RANDOM | 300 | 23426.411 | 23711.885 | 0.99 > ArraysSort.Int.testSort | REPEATED | 600 | 1.032 | 0.859 | 1.20 > ArraysSort.Int.testSort | REPEATED | 9000 | 5.114 | 5.014 | 1.02 > ArraysSort.Int.testSort | REPEATED | 2 | 10.3 | 9.532 | 1.08 > ArraysSort.Int.testSort | REPEATED | 40 | 210.742 | 235.281 | 0.90 > ArraysSort.Int.testSort | REPEATED | 300 | 1948.589 | 1955.258 | 1.00 > ArraysSort.Int.testSort | STAGGER | 600 | 2.125 | 2.157 | 0.99 > ArraysSort.Int.testSort | STAGGER | 9000 | 29.86 | 29.931 | 1.00 > ArraysSort.Int.testSort | STAGGER | 2 | 67.096 | 66.543 | 1.01 > ArraysSort.Int.testSort | STAGGER | 40 | 1247.53 | 1224.999 | 1.02 > ArraysSort.Int.testSort | STAGGER | 300 | 9435.404 | 9495.189 | 0.99 > ArraysSort.Int.testSort | SHUFFLE | 600 | 2.701 | 1.64 | 1.65 > ArraysSort.Int.testSort | SHUFFLE | 9000 | 38.976 | 34.201 | 1.14 > ArraysSort.Int.testSort | SHUFFLE | 2 | 96.399 | 79.616 | 1.21 > ArraysSort.Int.testSort | SHUFFLE | 40 | 2566.338 | 2436.271 | 1.05 > ArraysSort.Int.testSort | SHUFFLE | 300 | 20835.935 | 20071.12 | 1.04 > > > > > > Hello Vamsi (@vamsi-parasa), Did you have a chance to run benchmarking? - PR Comment: https://git.openjdk.org/jdk/pull/13568#issuecomment-1846182446
Re: RFR: 6230751: [Fmt-Ch] Recursive MessageFormats in ChoiceFormats ignore indicated subformats [v4]
> Please review this PR which updates an incorrect code example in
> _java/text/ChoiceFormat_.
>
> ChoiceFormat (and MessageFormat) provide an example of how to produce a
> pattern that supports singular and plural forms. The ChoiceFormat example is
> incorrect, as recursive MessageFormats produced by a ChoiceFormat subformat
> only recognize subformats defined through the MessageFormat pattern syntax,
> not through the subformats contained within the top level MessageFormat.
>
> In the original example,
> `Format[] testFormats = {fileform, null, NumberFormat.getInstance()};` could
> have been replaced with
> `Format[] testFormats = {fileform, null, new ChoiceFormat("0#BROKEN")};` and
> the original output would have been the same.
>
> This PR replaces the example with the one used in MessageFormat, which is
> correct.
>
> This PR also includes a drive-by fix to remove leftover ``s from
> a previous `@snippet` conversion.
Justin Lu has updated the pull request incrementally with one additional commit
since the last revision:
shorten wording
-
Changes:
- all: https://git.openjdk.org/jdk/pull/16891/files
- new: https://git.openjdk.org/jdk/pull/16891/files/47e91b3d..3c9a6324
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=16891&range=03
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=16891&range=02-03
Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod
Patch: https://git.openjdk.org/jdk/pull/16891.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/16891/head:pull/16891
PR: https://git.openjdk.org/jdk/pull/16891
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v11]
On Thu, 7 Dec 2023 22:06:14 GMT, Vladimir Yaroslavskiy wrote: >> > xmlns:o="urn:schemas-microsoft-com:office:office" >> xmlns:x="urn:schemas-microsoft-com:office:excel" >> xmlns="http://www.w3.org/TR/REC-html40";> >> >> >> >> >> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip.htm"> >> > href="file:///C:/Users/sparasa/AppData/Local/Temp/msohtmlclip1/01/clip_filelist.xml"> >> >> >> >> >> >> >> >> >> Benchmark (us/op) | Builder | (size) | Stock JDK (+ AVX512 sort) | >> DPQS_r01 (+ AVX512 sort) | Speedup >> -- | -- | -- | -- | -- | -- >> ArraysSort.Int.testSort | RANDOM | 600 | 2.256 | 1.713 | 1.32 >> ArraysSort.Int.testSort | RANDOM | 9000 | 41.457 | 38.316 | 1.08 >> ArraysSort.Int.testSort | RANDOM | 2 | 98.448 | 86.376 | 1.14 >> ArraysSort.Int.testSort | RANDOM | 40 | 2820.939 | 2792.333 | 1.01 >> ArraysSort.Int.testSort | RANDOM | 300 | 23426.411 | 23711.885 | 0.99 >> ArraysSort.Int.testSort | REPEATED | 600 | 1.032 | 0.859 | 1.20 >> ArraysSort.Int.testSort | REPEATED | 9000 | 5.114 | 5.014 | 1.02 >> ArraysSort.Int.testSort | REPEATED | 2 | 10.3 | 9.532 | 1.08 >> ArraysSort.Int.testSort | REPEATED | 40 | 210.742 | 235.281 | 0.90 >> ArraysSort.Int.testSort | REPEATED | 300 | 1948.589 | 1955.258 | 1.00 >> ArraysSort.Int.testSort | STAGGER | 600 | 2.125 | 2.157 | 0.99 >> ArraysSort.Int.testSort | STAGGER | 9000 | 29.86 | 29.931 | 1.00 >> ArraysSort.Int.testSort | STAGGER | 2 | 67.096 | 66.543 | 1.01 >> ArraysSort.Int.testSort | STAGGER | 40 | 1247.53 | 1224.999 | 1.02 >> ArraysSort.Int.testSort | STAGGER | 300 | 9435.404 | 9495.189 | 0.99 >> ArraysSort.Int.testSort | SHUFFLE | 600 | 2.701 | 1.64 | 1.65 >> ArraysSort.Int.testSort | SHUFFLE | 9000 | 38.976 | 34.201 | 1.14 >> ArraysSort.Int.testSort | SHUFFLE | 2 | 96.399 | 79.616 | 1.21 >> ArraysSort.Int.testSort | SHUFFLE | 40 | 2566.338 | 2436.271 | 1.05 >> ArraysSort.Int.testSort | SHUFFLE | 300 | 20835.935 | 20071.12 | 1.04 >> >> >> >> >> >> > > Hello Vamsi (@vamsi-parasa), > > Did you have a chance to run benchmarking? Hello Vladimir (@iaroslavski), Will provide the data by EOD Friday (US Pacific time). Had to wrap up some important things at work as I'll be going on a winter vacation for 4 weeks starting from Monday. Thanks for understanding! Thanks, Vamsi - PR Comment: https://git.openjdk.org/jdk/pull/13568#issuecomment-1846189152
RFR: 8321480: ISO 4217 Amendment 176 Update
Please review this PR which incorporates the ISO 4217 Amendment 176 Update. As the replacement of `ANG` to `XCG` won't occur until 2025, this change does not need to go into JDK22. `HR` was also updated to remove the past cutover dates. An existing test in _ValidateISO4217.java_ checked that `Currency::getAvailableCurrencies` had all the expected currencies. This method returns all currencies, including ones to take place in the future (e.g. `XCG`). The expected currencies `Set` the method was test against had to be updated to also include future currencies as well. Additionally, this change also converted a parameterized test to a normal JUnit test, due to output overflow. - Commit messages: - init Changes: https://git.openjdk.org/jdk/pull/17023/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17023&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8321480 Stats: 36 lines in 4 files changed: 7 ins; 0 del; 29 mod Patch: https://git.openjdk.org/jdk/pull/17023.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17023/head:pull/17023 PR: https://git.openjdk.org/jdk/pull/17023
Integrated: 8321409: Console read line with zero out should zero out underlying buffer in JLine (redux)
On Wed, 6 Dec 2023 21:12:40 GMT, Naoto Sato wrote: > This is an additional fix to JDK-8321131, where more clearing is required in > JLine. This pull request has now been integrated. Changeset: 4ed38f5a Author:Naoto Sato URL: https://git.openjdk.org/jdk/commit/4ed38f5ad5f822ab948257ed39717ea919fd32ed Stats: 12 lines in 3 files changed: 11 ins; 0 del; 1 mod 8321409: Console read line with zero out should zero out underlying buffer in JLine (redux) Reviewed-by: alanb - PR: https://git.openjdk.org/jdk/pull/17004
Re: RFR: 8321396: Retire test/jdk/java/util/zip/NoExtensionSignature.java
On Thu, 7 Dec 2023 18:14:59 GMT, Lance Andersen wrote: > Eirik, Could you add a reference to [PR > 12959](https://github.com/openjdk/jdk/pull/12959/) or to > [JDK-8303920](https://bugs.openjdk.org/browse/JDK-8303920) in the above Thanks, that makes sense! - PR Comment: https://git.openjdk.org/jdk/pull/16975#issuecomment-1845880582
Re: RFR: 8321396: Retire test/jdk/java/util/zip/NoExtensionSignature.java
On Tue, 5 Dec 2023 15:58:14 GMT, Eirik Bjorsnos wrote: > Please review this PR which suggests we retire the ZIP test > `NoExtensionSignature` along with its `test.jar` test vector. > > The concern of a missing data descriptor signature is covered by the recently > updated `DataDescriptorSignatureMissing` test. That test is more complete, > includes more documentation and uses a programmatically generated test vector. Eirik, Could you add a reference to [PR 12959](https://github.com/openjdk/jdk/pull/12959/) or to [JDK-8303920](https://bugs.openjdk.org/browse/JDK-8303920) in the above - PR Comment: https://git.openjdk.org/jdk/pull/16975#issuecomment-1845874650
Re: RFR: 8321396: Retire test/jdk/java/util/zip/NoExtensionSignature.java
On Tue, 5 Dec 2023 15:58:14 GMT, Eirik Bjorsnos wrote: > Please review this PR which suggests we retire the ZIP test > `NoExtensionSignature` along with its `test.jar` test vector. > > The concern of a missing data descriptor signature is covered by the recently > updated `DataDescriptorSignatureMissing` test. That test is more complete, > includes more documentation and uses a programmatically generated test vector. > > One might argue that 'more tests are always better', but in this case I think > the 21 year old `NoExtensionSignature` test with its binary test vector is > nebulous and requires extensive analysis to understand, more so to update. I > think it does not carry its weight and should be retired. > > Careful analysis of the deleted `test.jar` test vector revealed that it > contains a local header with non-zero `compressed size` and `uncompressed > size` fields for a streaming-mode entry. `APPNOTE.TXT` mandates that when bit > 3 of the general purpose bit flag is set, then these fields and the `crc` > field should all be set to zero. > > By injecting assertions into `ZipInputStream.readLOC` I was able to determine > that `NoExtensionSignature` is the only test currently parsing a ZIP file > with such non-zero fields in streaming mode. > > Because of this, and out of caution, this PR introduces a new test > `DataDescriptorIgnoreCrcAndSizeFields` which explicitly verifies that > `ZipInputStream` does not read any non-zero `crc`, `compressed size` and > `uncompressed size` field values from a local header when in streaming mode. > `ZipInputStream` should ignore these values and it would be good to have a > test which asserts that this invariant holds even when the fields are > non-zero. For completeness, here is a trace of the deleted `test.zip`: -- Local File Header -- 00 signature 0x04034b50 04 version20 06 flags 0x0008 08 method 8 Deflated 10 time 0x9a45 19:18:10 12 date 0x2c47 2002-02-07 14 crc0x 18 csize 2 22 size 2 26 nlen 8 28 elen 0 30 name 8 bytes'test.txt' -- File Data -- 38 data 4 bytes -- Data Descriptor -- 42 crc0xd8932aac 46 csize 4 50 size 2 -- Central Directory File Header -- 54 signature 0x02014b50 58 made by version788 60 extract version20 62 flags 0x0008 64 method 8 Deflated 66 time 0x9a45 19:18:10 68 date 0x2c47 2002-02-07 70 crc0xd8932aac 74 csize 4 78 size 2 82 diskstart 0 84 nlen 8 86 elen 0 88 clen 0 90 iattr 0x00 92 eattr 0x81b6 96 loc offset 0 000100 name 8 bytes'test.txt' -- End of Central Directory -- 000108 signature 0x06054b50 000112 this disk 0 000114 cen disk 0 000116 entries disk 1 000118 entries total 1 000120 cen size 54 000124 cen offset 54 000128 clen 0 - PR Comment: https://git.openjdk.org/jdk/pull/16975#issuecomment-1845780545
Re: RFR: 8320198: some reference processing tests don't wait long enough for reference processing to complete
On Thu, 7 Dec 2023 05:18:26 GMT, Jaikiran Pai wrote:
>> This slightly increases the wait for reference processing to complete to
>> accommodate long Xcomp compile times. Testing is underway.
>
> test/jdk/java/lang/Object/FinalizationOption.java line 89:
>
>> 87: static boolean checkFinalizerCalled(boolean expected) {
>> 88: create();
>> 89: for (int i = 0; i < 100; i++) {
>
> Hello Tom, I think this entire loop can be replaced by using the
> `jdk.test.lib.util.ForceGC` utility class available in the tests. That class
> has the necessary knowledge of using the jtreg timeout factor. I think you
> could replace this loop with something like (I haven't tried it):
>
>
> ForceGC.wait(() -> finalizerWasCalled);
Thanks for the tip. I'll take a look at. Even if there isn't something that's
exactly right I think having a general waitForReferenceProcessing helper would
be a good idea for any tests which do this.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16956#discussion_r1419305640
Integrated: JDK-8319413: Start of release updates for JDK 23
On Fri, 3 Nov 2023 23:42:03 GMT, Joe Darcy wrote: > Time to start making preparations for JDK 23. This pull request has now been integrated. Changeset: 519ecd35 Author:Joe Darcy Committer: Jesper Wilhelmsson URL: https://git.openjdk.org/jdk/commit/519ecd352a66633589f160db7390647d90e36b99 Stats: 4872 lines in 98 files changed: 4828 ins; 0 del; 44 mod 8319413: Start of release updates for JDK 23 8319414: Add SourceVersion.RELEASE_23 8319416: Add source 23 and target 23 to javac Reviewed-by: iris, erikj, alanb, vromero - PR: https://git.openjdk.org/jdk/pull/16505
RFR: 8321396: Retire test/jdk/java/util/zip/NoExtensionSignature.java
Please review this PR which suggests we retire the ZIP test `NoExtensionSignature` along with its `test.jar` test vector. The concern of a missing data descriptor signature is covered by the recently updated `DataDescriptorSignatureMissing` test. That test is more complete, includes more documentation and uses a programmatically generated test vector. One might argue that 'more tests are always better', but in this case I think the 21 year old `NoExtensionSignature` test with its binary test vector is nebulous and requires extensive analysis to understand, more so to update. I think it does not carry its weight and should be retired. Careful analysis of the deleted `test.jar` test vector revealed that it contains a local header with non-zero `compressed size` and `uncompressed size` fields for a streaming-mode entry. `APPNOTE.TXT` mandates that when bit 3 of the general purpose bit flag is set, then these fields and the `crc` field should all be set to zero. By injecting assertions into `ZipInputStream.readLOC` I was able to determine that `NoExtensionSignature` is the only test currently parsing a ZIP file with such non-zero fields in streaming mode. Because of this, and out of caution, this PR introduces a new test `DataDescriptorIgnoreCrcAndSizeFields` which explicitly verifies that `ZipInputStream` does not read any non-zero `crc`, `compressed size` and `uncompressed size` field values from a local header when in streaming mode. `ZipInputStream` should ignore these values and it would be good to have a test which asserts that this invariant holds even when the fields are non-zero. - Commit messages: - Rename 'nameAndContent' parameter to 'expected' - Retire the test NoExtensionSignature in favor of DataDescriptorSignatureMissing. Introduce the new test DataDescriptorIgnoreCrcAndSizeFields covering unrelated aspects implicitly tested by NoExtensionSignature. Changes: https://git.openjdk.org/jdk/pull/16975/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16975&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8321396 Stats: 222 lines in 3 files changed: 180 ins; 42 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16975.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16975/head:pull/16975 PR: https://git.openjdk.org/jdk/pull/16975
Integrated: 8321223: Implementation of Scoped Values (Second Preview)
On Sun, 3 Dec 2023 08:46:07 GMT, Alan Bateman wrote: > This API is sitting out JDK 22, meaning no API/implementation changes in this > PR. Some small API changes are likely for JDK 23. > > For now, we just need to bump JEP number/title that shows up in the preview > section of the javadoc. This pull request has now been integrated. Changeset: 58530f40 Author:Alan Bateman URL: https://git.openjdk.org/jdk/commit/58530f4098538f490cfea58f2382d0997841c171 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8321223: Implementation of Scoped Values (Second Preview) Reviewed-by: psandoz, mcimadamore - PR: https://git.openjdk.org/jdk/pull/16937
Integrated: 8321470: ThreadLocal.nextHashCode can be static final
On Wed, 6 Dec 2023 00:52:48 GMT, Brett Okken wrote: > The static AtomicInteger used for the nextHashCode should be final. This pull request has now been integrated. Changeset: c42535f1 Author:Brett Okken Committer: Aleksey Shipilev URL: https://git.openjdk.org/jdk/commit/c42535f1110d60d1472080ad4fcadb8acbeb4c4b Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8321470: ThreadLocal.nextHashCode can be static final Reviewed-by: shade, jpai - PR: https://git.openjdk.org/jdk/pull/16987
Integrated: 8321467: MemorySegment.setString(long, String, Charset) throws IAE(Misaligned access)
On Wed, 6 Dec 2023 16:49:30 GMT, Maurizio Cimadamore wrote: > This PR fixes a couple of aligned accesses when reading/writing strings. Such > aligned accesses crept in when we optimized string read/write operations to > work in bulk. As a result, depending on the maximum alignment constraints > supported by the heap segment, some string operations might fail. > > I've added some tests to make sure that all operations work as expected with > unaligned semantics. > > Note: I've considered inferring an alignment constraint from the provided > charset, and then use aligned operations (and document that behavior), but I > found that to be unsatisfactory: memory operations typically accept a layout, > which allow clients to opt out from alignment checks if needed. But if we > always infer an alignment constraint from the provided charset, clients would > find themselves w/o an escape hatch. For this reason, I think the best way to > fix this is to use unaligned operations when reading/writing the string. This pull request has now been integrated. Changeset: 42bb8526 Author:Maurizio Cimadamore URL: https://git.openjdk.org/jdk/commit/42bb8526967ce6d74b409c0f7aa6f8580af1aaa0 Stats: 56 lines in 2 files changed: 43 ins; 6 del; 7 mod 8321467: MemorySegment.setString(long, String, Charset) throws IAE(Misaligned access) Reviewed-by: pminborg - PR: https://git.openjdk.org/jdk/pull/16999
Re: RFR: 8321387: SegmentAllocator:allocateFrom(AddressLayout, MemorySegment) does not throw stated UnsupportedOperationException [v5]
On Thu, 7 Dec 2023 09:00:46 GMT, Per Minborg wrote: >> This PR proposes to change the exception type for exceptions thrown for >> certain methods with a parameter of type `MemorySegment` when it is >> `MemorySegment::isReadOnly`. Previously an `UnsupportedOperationException` >> was specified but in some cases, in reality, an `IllegalArgumentException` >> was thrown. >> >> The principle used in this PR is that operations acting on an MS where the >> MS is `this` should throw an `UnsupportedOperationException` whereas in >> cases where the MS is a *parameter* an `IllegalArgumentException` should be >> thrown. >> >> It should be noted that this PR retains the previous behavior for MS >> VarHandle access (even though the MS is a parameter to the accessor methods, >> the first parameter can be said to represent `this`). > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Revert checkAccess parameter type Looks good! - Marked as reviewed by mcimadamore (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16993#pullrequestreview-1769982170
Integrated: 8321270: Virtual Thread.yield consumes parking permit
On Mon, 4 Dec 2023 16:08:32 GMT, Alan Bateman wrote: > When a virtual thread continues after Thread.yield it currently consumes > thread's parking permit. This is an oversight, the parking permit should only > be consumed when continuing after park. > > The changes are straight-forward. The internal "RUNNABLE" state is replaced > with UNPARKED and YIELDED state, effectively encoding the previous state. So > for the most part, it's just replacing the usages of RUNNABLE. The additional > states require refactoring tryGetStackTrace, this is the method that is used > for Thread::getStackTrace when the virtual thread is unmounted. It is also > changed to not not swallow the REE if the reesubmit fails (tryStackTrace has > to resubmit as the task for the thread may run, or the thread unparked, while > "suspended" and sampling its stack trace). The changes are a subset of larger > changes in the loom repo, future PRs will propose to bring in other changes > to get main line up to date. > > For testing the existing ThreadAPI has new test cases. > > Testing: test1-5. This includes the JVMTI tests as it maps the thread states > to JVMTI thread states. This pull request has now been integrated. Changeset: 29d7a223 Author:Alan Bateman URL: https://git.openjdk.org/jdk/commit/29d7a22348e43cba253d0483c4c05922368f6b8a Stats: 158 lines in 4 files changed: 79 ins; 19 del; 60 mod 8321270: Virtual Thread.yield consumes parking permit Reviewed-by: sspitsyn - PR: https://git.openjdk.org/jdk/pull/16953
Re: RFR: 8321468: Remove StringUTF16::equals
On Wed, 6 Dec 2023 14:20:14 GMT, Claes Redestad wrote:
> https://bugs.openjdk.org/browse/JDK-8215017 removed the only use of
> `StringUTF16::equals`. At the time I did some performance verification
> focused on x86 showing that simplifying and only using `StringLatin1::equals`
> was either neutral or a win.
>
> I repeated this experiment recently, adding some focused tests on aarch64
> where the code generation actually tries to take advantage and generate
> slightly more efficient code for `StringUTF16::equals`:
> https://github.com/openjdk/jdk/pull/16933#discussion_r1414118658
>
> The indication here is that disabling use of `StringUTF16::equals` was the
> right choice: any effect from the low-level optimization (one less branch at
> the tail end) was offset by the `isLatin1()` branch and added code generation
> (that all gets inlined).
>
> In a `-XX:-CompactStrings` configuration the slightly improved code
> generation in `StringUTF16::equals` might help, since the `isLatin1()` test
> and subsequent call to `StringLatin1::equals` would be DCEd. To get the best
> of both worlds the code in `String::equals` _could_ be sharpened so that we
> statically pick the best implementation based on `CompactStrings` mode (see
> comment below). This shows a tiny win (up to -0.2ns/op per `String::equals`
> on M1; netural on x86). But is all this complexity worth it for a gain that
> will get lost in the noise on anything realistic?
>
> This PR instead proposes removing `StringUTF16::equals` and simplifying the
> mechanisms to support the `StringLatin1/UTF16::equals` pair of intrinsics in
> hotspot.
For reference these are the microbenchmarks used in the JDK-8215017
verification experiment:
diff --git a/test/micro/org/openjdk/bench/java/lang/StringEquals.java
b/test/micro/org/openjdk/bench/java/lang/StringEquals.java
index b0db6a7037e..effe855c228 100644
--- a/test/micro/org/openjdk/bench/java/lang/StringEquals.java
+++ b/test/micro/org/openjdk/bench/java/lang/StringEquals.java
@@ -43,6 +43,9 @@ public class StringEquals {
public String test5 = new String(test4); // equal to test4, but not same
public String test6 = new String("0123456780");
public String test7 = new String("0123\u01FE");
+public String test8 = new String("12\u01FE");
+public String test9 = new String("12\u01FF");
+public String test10 = new String("12\u01FE");
@Benchmark
public boolean different() {
@@ -73,5 +76,15 @@ public boolean differentCoders() {
public boolean equalsUTF16() {
return test5.equals(test4);
}
+
+@Benchmark
+public boolean equalsUTF16_3_NE() {
+return test8.equals(test9);
+}
+
+@Benchmark
+public boolean equalsUTF16_3_EQ() {
+return test8.equals(test10);
+}
}
And this is the change to `String` to get it back to pre-JDK-8215017 state:
diff --git a/src/java.base/share/classes/java/lang/String.java
b/src/java.base/share/classes/java/lang/String.java
index 5869e086191..18ad0e85d33 100644
--- a/src/java.base/share/classes/java/lang/String.java
+++ b/src/java.base/share/classes/java/lang/String.java
@@ -1900,9 +1900,13 @@ public boolean equals(Object anObject) {
if (this == anObject) {
return true;
}
-return (anObject instanceof String aString)
-&& (!COMPACT_STRINGS || this.coder == aString.coder)
-&& StringLatin1.equals(value, aString.value);
+if (anObject instanceof String aString) {
+if (coder() == aString.coder()) {
+return isLatin1() ? StringLatin1.equals(value, aString.value)
+: StringUTF16.equals(value, aString.value);
+}
+}
+return false;
}
/**
M1 experiment with `-XX:-CompactStrings` and a patch to choose the
`StringUTF16::equals` if `COMPACT_STRINGS` is false[1]:
Name Cnt Base Error Test Error Unit Change
StringEquals.equalsUTF16_3_EQ 5 1,799 ± 0,008 1,585 ± 0,009 ns/op 1,14x (p
= 0,000*)
StringEquals.equalsUTF16_3_NE 5 1,622 ± 0,007 1,541 ± 0,011 ns/op 1,05x (p
= 0,000*)
* = significant
[1]
diff --git a/src/java.base/share/classes/java/lang/String.java
b/src/java.base/share/classes/java/lang/String.java
index 5869e086191..10451bda83f 100644
--- a/src/java.base/share/classes/java/lang/String.java
+++ b/src/java.base/share/classes/java/lang/String.java
@@ -1900,9 +1900,14 @@ public boolean equals(Object anObject) {
if (this == anObject) {
return true;
}
-return (anObject instanceof String aString)
-&& (!COMPACT_STRINGS || this.coder == aString.coder)
-&& StringLatin1.equals(value, aString.value);
+if (anObject instanceof String aString) {
+if (COMPACT_STRINGS) {
+return this.coder == aString.coder &&
StringLatin1.equals(value, aString.value);
+} else {
+
RFR: 8321468: Remove StringUTF16::equals
https://bugs.openjdk.org/browse/JDK-8215017 removed the only use of `StringUTF16::equals`. At the time I did some performance verification focused on x86 showing that simplifying and only using `StringLatin1::equals` was either neutral or a win. I repeated this experiment recently, adding some focused tests on aarch64 where the code generation actually tries to take advantage and generate slightly more efficient code for `StringUTF16::equals`: https://github.com/openjdk/jdk/pull/16933#discussion_r1414118658 The indication here is that disabling use of `StringUTF16::equals` was the right choice: any effect from the low-level optimization (one less branch at the tail end) was offset by the `isLatin1()` branch and added code generation (that all gets inlined). In a `-XX:-CompactStrings` configuration the slightly improved code generation in `StringUTF16::equals` might help, since the `isLatin1()` test and subsequent call to `StringLatin1::equals` would be DCEd. To get the best of both worlds the code in `String::equals` _could_ be sharpened so that we statically pick the best implementation based on `CompactStrings` mode (see comment below). This shows a tiny win (up to -0.2ns/op per `String::equals` on M1; netural on x86). But is all this complexity worth it for a gain that will get lost in the noise on anything realistic? This PR instead proposes removing `StringUTF16::equals` and simplifying the mechanisms to support the `StringLatin1/UTF16::equals` pair of intrinsics in hotspot. - Commit messages: - Fix and further cleanup RISC - Remove StringUTF16::equals Changes: https://git.openjdk.org/jdk/pull/16995/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16995&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8321468 Stats: 138 lines in 14 files changed: 0 ins; 113 del; 25 mod Patch: https://git.openjdk.org/jdk/pull/16995.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16995/head:pull/16995 PR: https://git.openjdk.org/jdk/pull/16995
Re: RFR: 8321470: ThreadLocal.nextHashCode can be static final [v2]
On Wed, 6 Dec 2023 17:42:47 GMT, Brett Okken wrote: >> The static AtomicInteger used for the nextHashCode should be final. > > Brett Okken has updated the pull request incrementally with one additional > commit since the last revision: > > Update full name @bokken, you are good to `/integrate`. - PR Comment: https://git.openjdk.org/jdk/pull/16987#issuecomment-1845016772
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
On Thu, 7 Dec 2023 09:30:01 GMT, Xiaohong Gong wrote: >> Currently the vector floating-point math APIs like >> `VectorOperators.SIN/COS/TAN...` are not intrinsified on AArch64 platform, >> which causes large performance gap on AArch64. Note that those APIs are >> optimized by C2 compiler on X86 platforms by calling Intel's SVML code [1]. >> To close the gap, we would like to optimize these APIs for AArch64 by >> calling a third-party vector library called libsleef [2], which are >> available in mainstream Linux distros (e.g. [3] [4]). >> >> SLEEF supports multiple accuracies. To match Vector API's requirement and >> implement the math ops on AArch64, we 1) call 1.0 ULP accuracy with FMA >> instructions used stubs in libsleef for most of the operations by default, >> and 2) add the vector calling convention to apply with the runtime calls to >> stub code in libsleef. Note that for those APIs that libsleef does not >> support 1.0 ULP, we choose 0.5 ULP instead. >> >> To help loading the expected libsleef library, this patch also adds an >> experimental JVM option (i.e. `-XX:UseSleefLib`) for AArch64 platforms. >> People can use it to denote the libsleef path/name explicitly. By default, >> it points to the system installed library. If the library does not exist or >> the dynamic loading of it in runtime fails, the math vector ops will >> fall-back to use the default scalar version without error. But a warning is >> printed out if people specifies a nonexistent library explicitly. >> >> Note that this is a part of the original proposed patch in panama-dev [5], >> just with some initial review comments addressed. And now we'd like to get >> some wider feedbacks from more hotspot experts. >> >> [1] https://github.com/openjdk/jdk/pull/3638 >> [2] https://sleef.org/ >> [3] https://packages.fedoraproject.org/pkgs/sleef/sleef/ >> [4] https://packages.debian.org/bookworm/libsleef3 >> [5] https://mail.openjdk.org/pipermail/panama-dev/2022-December/018172.html > > Xiaohong Gong has updated the pull request incrementally with one additional > commit since the last revision: > > Fix potential attribute issue Build changes finally look good. Great, actually! Thanks for persisting, despite the many rounds of review. You will still need the 2 hotspot reviews for the hotspot part of the patch. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16234#pullrequestreview-1769660206
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]
> Currently the vector floating-point math APIs like > `VectorOperators.SIN/COS/TAN...` are not intrinsified on AArch64 platform, > which causes large performance gap on AArch64. Note that those APIs are > optimized by C2 compiler on X86 platforms by calling Intel's SVML code [1]. > To close the gap, we would like to optimize these APIs for AArch64 by calling > a third-party vector library called libsleef [2], which are available in > mainstream Linux distros (e.g. [3] [4]). > > SLEEF supports multiple accuracies. To match Vector API's requirement and > implement the math ops on AArch64, we 1) call 1.0 ULP accuracy with FMA > instructions used stubs in libsleef for most of the operations by default, > and 2) add the vector calling convention to apply with the runtime calls to > stub code in libsleef. Note that for those APIs that libsleef does not > support 1.0 ULP, we choose 0.5 ULP instead. > > To help loading the expected libsleef library, this patch also adds an > experimental JVM option (i.e. `-XX:UseSleefLib`) for AArch64 platforms. > People can use it to denote the libsleef path/name explicitly. By default, it > points to the system installed library. If the library does not exist or the > dynamic loading of it in runtime fails, the math vector ops will fall-back to > use the default scalar version without error. But a warning is printed out if > people specifies a nonexistent library explicitly. > > Note that this is a part of the original proposed patch in panama-dev [5], > just with some initial review comments addressed. And now we'd like to get > some wider feedbacks from more hotspot experts. > > [1] https://github.com/openjdk/jdk/pull/3638 > [2] https://sleef.org/ > [3] https://packages.fedoraproject.org/pkgs/sleef/sleef/ > [4] https://packages.debian.org/bookworm/libsleef3 > [5] https://mail.openjdk.org/pipermail/panama-dev/2022-December/018172.html Xiaohong Gong has updated the pull request incrementally with one additional commit since the last revision: Fix potential attribute issue - Changes: - all: https://git.openjdk.org/jdk/pull/16234/files - new: https://git.openjdk.org/jdk/pull/16234/files/c55357b6..7a4be736 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16234&range=08 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16234&range=07-08 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16234.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16234/head:pull/16234 PR: https://git.openjdk.org/jdk/pull/16234
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v7]
On Wed, 6 Dec 2023 11:46:03 GMT, Magnus Ihse Bursie wrote: >> Xiaohong Gong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Add "--with-libsleef-lib" and "--with-libsleef-include" options > > make/modules/jdk.incubator.vector/Lib.gmk line 45: > >> 43: $(eval $(call SetupJdkLibrary, BUILD_LIBVMATH, \ >> 44: NAME := vmath, \ >> 45: CFLAGS := $(CFLAGS_JDKLIB) $(LIBSLEEF_CFLAGS) >> -fvisibility=default, \ > > Why `-fvisibility=default`? (Sorry, only noticed this now) Yeah. Considering all the symbols in this lib are global and need to be exported, I added this flag here instead of the source code. I'v removed this in latest commit, and added the attribute visibility in source code like other jdk code. Please help to review again. Thanks a lot! - PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1418458207
Integrated: JDK-8320538: Obsolete CSS styles in collection framework doc-file
On Wed, 6 Dec 2023 16:22:24 GMT, Hannes Wallnöfer wrote:
> Please review a simple change to remove a stray inline CSS element from the
> Collection Framework index doc file. The only thing the `a {font-weight:
> bold;}` rule did was to make all links in the header and footer bold as [can
> be seen here][1]; the three content links to the other doc files are still
> displayed in bold font because they are contained in `` tags. The other
> CSS rules had no effect in the page.
>
> [1]:
> https://download.java.net/java/early_access/jdk22/docs/api/java.base/java/util/doc-files/coll-index.html
>
> Screenshot of the fixed page:
> src="https://github.com/openjdk/jdk/assets/15975/0bd3b613-e535-438c-bd4d-7c8817df5702";>
>
> I also added the `` meta tag which is present in all other pages (both doc-files
> and generated by JavaDoc).
This pull request has now been integrated.
Changeset: 9a87e52c
Author:Hannes Wallnöfer
URL:
https://git.openjdk.org/jdk/commit/9a87e52c0ca6754092845c8ebc9e324c58936c72
Stats: 10 lines in 1 file changed: 0 ins; 9 del; 1 mod
8320538: Obsolete CSS styles in collection framework doc-file
Reviewed-by: alanb
-
PR: https://git.openjdk.org/jdk/pull/16997
Re: RFR: 8321387: SegmentAllocator:allocateFrom(AddressLayout, MemorySegment) does not throw stated UnsupportedOperationException [v5]
> This PR proposes to change the exception type for exceptions thrown for > certain methods with a parameter of type `MemorySegment` when it is > `MemorySegment::isReadOnly`. Previously an `UnsupportedOperationException` > was specified but in some cases, in reality, an `IllegalArgumentException` > was thrown. > > The principle used in this PR is that operations acting on an MS where the MS > is `this` should throw an `UnsupportedOperationException` whereas in cases > where the MS is a *parameter* an `IllegalArgumentException` should be thrown. > > It should be noted that this PR retains the previous behavior for MS > VarHandle access (even though the MS is a parameter to the accessor methods, > the first parameter can be said to represent `this`). Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Revert checkAccess parameter type - Changes: - all: https://git.openjdk.org/jdk/pull/16993/files - new: https://git.openjdk.org/jdk/pull/16993/files/491e71fc..f674b9c6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16993&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16993&range=03-04 Stats: 54 lines in 2 files changed: 0 ins; 11 del; 43 mod Patch: https://git.openjdk.org/jdk/pull/16993.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16993/head:pull/16993 PR: https://git.openjdk.org/jdk/pull/16993
Re: RFR: 8321387: SegmentAllocator:allocateFrom(AddressLayout, MemorySegment) does not throw stated UnsupportedOperationException [v3]
On Wed, 6 Dec 2023 15:36:57 GMT, Maurizio Cimadamore wrote: >> Per Minborg has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update throws docs fror SegmentAllocator > > src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java line 95: > >> 93: * @param str the Java string to be converted into a C string >> 94: * @return a new native segment containing the converted C string >> 95: * @throws IllegalArgumentException if the allocated segment is > > I don't think the changes here are useful. What does it mean for an allocated > segment to be read-only? I think all these conditions are tied to > `prefixAllocator` blindly accepting read-only segments, which should NOT be > the case. I suggest to revert all the chnages here and document (and throw) a > new exception for when a prefix allocator is created from a read-only segment. I have updated the PR according to this. It should be noted that, theoretically, it is still possible to write a bespoke `SegmentAllocator` that returns read-only segments. I think then, the user made a deliberate choice and perhaps understands the consequences. - PR Review Comment: https://git.openjdk.org/jdk/pull/16993#discussion_r1418543624
Re: RFR: 8321387: SegmentAllocator:allocateFrom(AddressLayout, MemorySegment) does not throw stated UnsupportedOperationException [v4]
> This PR proposes to change the exception type for exceptions thrown for > certain methods with a parameter of type `MemorySegment` when it is > `MemorySegment::isReadOnly`. Previously an `UnsupportedOperationException` > was specified but in some cases, in reality, an `IllegalArgumentException` > was thrown. > > The principle used in this PR is that operations acting on an MS where the MS > is `this` should throw an `UnsupportedOperationException` whereas in cases > where the MS is a *parameter* an `IllegalArgumentException` should be thrown. > > It should be noted that this PR retains the previous behavior for MS > VarHandle access (even though the MS is a parameter to the accessor methods, > the first parameter can be said to represent `this`). Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Disallow read only segments for SA factories - Changes: - all: https://git.openjdk.org/jdk/pull/16993/files - new: https://git.openjdk.org/jdk/pull/16993/files/d7b08a40..491e71fc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16993&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16993&range=02-03 Stats: 68 lines in 2 files changed: 22 ins; 44 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/16993.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16993/head:pull/16993 PR: https://git.openjdk.org/jdk/pull/16993
