Re: RFR: JDK-8322979: Add informative discussion to Modifier [v3]
> Add a few apiNote concerning source-level modifiers that are not represented > in java.lang.reflect.Modifier. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Refine wording per review feedback. - Changes: - all: https://git.openjdk.org/jdk/pull/17338/files - new: https://git.openjdk.org/jdk/pull/17338/files/e5816889..cc67f321 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17338&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17338&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17338.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17338/head:pull/17338 PR: https://git.openjdk.org/jdk/pull/17338
Re: RFR: 8323562: SaslInputStream.read() may return wrong value
On Fri, 12 Jan 2024 07:22:09 GMT, Alan Bateman wrote: > Just curious if this was found by inspection or when debugging some issue > with LDAP authentication? Asking on whether it is feasible or not to have > more tests in this area. It was found by the code inspection. - PR Comment: https://git.openjdk.org/jdk/pull/17365#issuecomment-1888567273
Re: RFR: 8323562: SaslInputStream.read() may return wrong value
On Thu, 11 Jan 2024 06:28:51 GMT, Sergey Bylokhov wrote: > SaslInputStream.read() should return a value in the range from 0 to 255 per > the spec of InputStream.read() but it returns the signed byte from the inBuf > as is. Just curious if this was found by inspection or when debugging some issue with LDAP authentication? Asking on whether it is feasible or not to have more tests in this area. - PR Comment: https://git.openjdk.org/jdk/pull/17365#issuecomment-1888555729
RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed
Reviewed-by: Yi Yang - Commit messages: - 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed Changes: https://git.openjdk.org/jdk/pull/17386/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17386&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8323640 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17386.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17386/head:pull/17386 PR: https://git.openjdk.org/jdk/pull/17386
Re: RFR: 8323296: java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java#id1 timed out [v2]
On Thu, 11 Jan 2024 16:50:42 GMT, Alan Bateman wrote: >> This test was recently dialled down via JDK-8323002 but it still makes slow >> progress on some test machines, esp. macox-x64-debug builds. The issue is >> that the sampling of the target thread is skewed towards the unmounted case >> so the target thread is disabled from being scheduled and doesn't make >> progress. The test is re-worked to use a barrier so that the main thread and >> target virtual thread run in lock step. This allows the virtual thread to >> make progress at each iteration and also increases the chances of sampling >> the stack trace at around the time that the target thread transitions from >> being unmounted (due to the Thread.yield) and parking while pinned. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Run in othervm mode Thank you Alan, the changes look good to me. - Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17353#pullrequestreview-1817235395
[jdk22] Integrated: 8323571: Regression in source resolution process
On Thu, 11 Jan 2024 22:56:38 GMT, Joe Wang wrote: > Backport to fix the regression introduced in JDK 22. This pull request has now been integrated. Changeset: 46f1df38 Author:Joe Wang URL: https://git.openjdk.org/jdk22/commit/46f1df38a0f972333d7ff93a00c7eb6b14d3dfb9 Stats: 357 lines in 4 files changed: 339 ins; 0 del; 18 mod 8323571: Regression in source resolution process Reviewed-by: naoto, iris Backport-of: e4389d8dc224419b8c1ee08e9f2dea0f103c6845 - PR: https://git.openjdk.org/jdk22/pull/63
Re: RFR: 8320699: Add parameter to skip progress logging of OutputAnalyzer [v3]
On Fri, 5 Jan 2024 09:07:57 GMT, Stefan Karlsson wrote: >> Tests using ProcessTools.executeProcess gets the following output written to >> stdout: >> [2023-11-24T09:58:16.797540608Z] Gathering output for process 2517117 >> [2023-11-24T09:58:23.070781345Z] Waiting for completion for process 2517117 >> [2023-11-24T09:58:23.071045055Z] Waiting for completion finished for process >> 2517117 >> >> This might be good for some use cases and debugging, but some tests spawns a >> large number of processes and for those this output fills up the log files. >> >> I propose that we add a way to turn of this output for tests where we find >> this output to be too noisy. > > Stefan Karlsson has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains three commits: > > - Merge remote-tracking branch 'upstream/master' into > 8320699_OutputAnalyzer_progress_logging > - Update OutputBuffer.java copyright years > - 8320699: Add parameter to skip progress logging of OutputAnalyzer Needed to update copyrights now. - Marked as reviewed by lmesnik (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16807#pullrequestreview-1817096686
RFR: 8323562: SaslInputStream.read() may return wrong value
SaslInputStream.read() should return a value in the range from 0 to 255 per the spec of InputStream.read() but it returns the signed byte from the inBuf as is. - Commit messages: - 8323562: SaslInputStream.read() may return wrong value Changes: https://git.openjdk.org/jdk/pull/17365/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17365&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8323562 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17365.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17365/head:pull/17365 PR: https://git.openjdk.org/jdk/pull/17365
Re: RFR: 8323562: SaslInputStream.read() may return wrong value
On Thu, 11 Jan 2024 06:28:51 GMT, Sergey Bylokhov wrote: > SaslInputStream.read() should return a value in the range from 0 to 255 per > the spec of InputStream.read() but it returns the signed byte from the inBuf > as is. I'll wait until GA will be fixed. - PR Comment: https://git.openjdk.org/jdk/pull/17365#issuecomment-1888222498
Re: [jdk22] RFR: 8323571: Regression in source resolution process
On Thu, 11 Jan 2024 22:56:38 GMT, Joe Wang wrote: > Backport to fix the regression introduced in JDK 22. Verified changes same as those in main-line. - Marked as reviewed by iris (Reviewer). PR Review: https://git.openjdk.org/jdk22/pull/63#pullrequestreview-1817021441
Re: [jdk22] RFR: 8323571: Regression in source resolution process
On Thu, 11 Jan 2024 22:56:38 GMT, Joe Wang wrote: > Backport to fix the regression introduced in JDK 22. Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk22/pull/63#pullrequestreview-1817006707
Re: RFR: JDK-8320448 Accelerate IndexOf using AVX2 [v7]
> 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 with a new target base due to a merge or a rebase. The pull request now contains 22 commits: - Merge branch 'openjdk:master' into indexof - Merge branch 'openjdk:master' into indexof - Addressing review comments. - Fix for JDK-8321599 - Support UU IndexOf - Only use optimization when EnableX86ECoreOpts is true - Fix whitespace - Merge branch 'openjdk:master' into indexof - Comments; added exhaustive-ish test - Subtracting 0x10 twice. - ... and 12 more: https://git.openjdk.org/jdk/compare/8e12053e...3e58d0c2 - Changes: https://git.openjdk.org/jdk/pull/16753/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16753&range=06 Stats: 3060 lines in 14 files changed: 2918 ins; 7 del; 135 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
[jdk22] RFR: 8323571: Regression in source resolution process
Backport to fix the regression introduced in JDK 22. - Commit messages: - Backport e4389d8dc224419b8c1ee08e9f2dea0f103c6845 Changes: https://git.openjdk.org/jdk22/pull/63/files Webrev: https://webrevs.openjdk.org/?repo=jdk22&pr=63&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8323571 Stats: 357 lines in 4 files changed: 339 ins; 0 del; 18 mod Patch: https://git.openjdk.org/jdk22/pull/63.diff Fetch: git fetch https://git.openjdk.org/jdk22.git pull/63/head:pull/63 PR: https://git.openjdk.org/jdk22/pull/63
Integrated: 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64)
On Thu, 11 Jan 2024 18:19:15 GMT, Alexey Semenyuk wrote: > Change the full path of a unit file passed to `systemctl enable` command to > the unit file name. This prevents the command from creating a symlink in > `/etc/systemd/system/` directory This pull request has now been integrated. Changeset: 8e12053e Author:Alexey Semenyuk URL: https://git.openjdk.org/jdk/commit/8e12053e0352a26ecd7f2b9bc298ddb8fb4bb61b Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64) Reviewed-by: almatvee - PR: https://git.openjdk.org/jdk/pull/17376
Re: RFR: 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64)
On Thu, 11 Jan 2024 18:19:15 GMT, Alexey Semenyuk wrote: > Change the full path of a unit file passed to `systemctl enable` command to > the unit file name. This prevents the command from creating a symlink in > `/etc/systemd/system/` directory Marked as reviewed by almatvee (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17376#pullrequestreview-1816945852
Integrated: 8323571: Regression in source resolution process
On Thu, 11 Jan 2024 18:36:39 GMT, Joe Wang wrote: > Fix a regression in the source resolution process where it failed to > recognize a custom InputSource when both the public and system IDs are null. > The particular issue is fixed at line 1233, the rest of the changes is > formatting to make the null-check on InputSource the first. > > Test is provided by Marc, thanks Marc! > (https://github.com/junit-team/junit5/issues/3594) > > XML tests, including the new test, pass with this change. This pull request has now been integrated. Changeset: e4389d8d Author:Joe Wang URL: https://git.openjdk.org/jdk/commit/e4389d8dc224419b8c1ee08e9f2dea0f103c6845 Stats: 357 lines in 4 files changed: 339 ins; 0 del; 18 mod 8323571: Regression in source resolution process Reviewed-by: lancea, naoto - PR: https://git.openjdk.org/jdk/pull/17377
RFR: 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64)
Change the full path of a unit file passed to `systemctl enable` command to the unit file name. This prevents the command from creating a symlink in `/etc/systemd/system/` directory - Commit messages: - empty - Merge branch 'master' into JDK-8322799 - 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64) - Make sure worker thread terminates before the main thread does. - 8294699: Launcher causes lingering busy cursor Changes: https://git.openjdk.org/jdk/pull/17376/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17376&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8322799 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17376.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17376/head:pull/17376 PR: https://git.openjdk.org/jdk/pull/17376
Re: RFR: 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64)
On Thu, 11 Jan 2024 18:19:15 GMT, Alexey Semenyuk wrote: > Change the full path of a unit file passed to `systemctl enable` command to > the unit file name. This prevents the command from creating a symlink in > `/etc/systemd/system/` directory Fix looks good. I will approve once "Integration blocker" issue is resolved. - PR Comment: https://git.openjdk.org/jdk/pull/17376#issuecomment-1887841042
Re: RFR: 8322799: Test JPKG003-013: ServiceTest fails because the user cannot uninstall the "servicetest" package on OEL 9.2 x64 and OEL 9.2 64-bit Arm (aarch64)
On Thu, 11 Jan 2024 18:19:15 GMT, Alexey Semenyuk wrote: > Change the full path of a unit file passed to `systemctl enable` command to > the unit file name. This prevents the command from creating a symlink in > `/etc/systemd/system/` directory @sashamatveev please review - PR Comment: https://git.openjdk.org/jdk/pull/17376#issuecomment-1887787798
Integrated: 8310813: Simplify and modernize equals, hashCode, and compareTo for BigInteger
On Fri, 23 Jun 2023 17:27:00 GMT, Pavel Rappo wrote: > Please review this PR to use modern APIs and language features to simplify > equals, hashCode, and compareTo for BigInteger. If you have any performance > concerns, please raise them. > > This PR is cherry-picked from a bigger, not-yet-published PR, to test the > waters. That latter PR will be published soon. This pull request has now been integrated. Changeset: 49e61213 Author:Pavel Rappo URL: https://git.openjdk.org/jdk/commit/49e61213474b846fd081e890e5abf9b79e3c Stats: 527 lines in 6 files changed: 500 ins; 16 del; 11 mod 8310813: Simplify and modernize equals, hashCode, and compareTo for BigInteger Reviewed-by: rriggs, redestad, rgiulietti - PR: https://git.openjdk.org/jdk/pull/14630
Re: RFR: 8323571: Regression in source resolution process [v2]
> Fix a regression in the source resolution process where it failed to > recognize a custom InputSource when both the public and system IDs are null. > The particular issue is fixed at line 1233, the rest of the changes is > formatting to make the null-check on InputSource the first. > > Test is provided by Marc, thanks Marc! > (https://github.com/junit-team/junit5/issues/3594) > > XML tests, including the new test, pass with this change. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: Fix copyright year - Changes: - all: https://git.openjdk.org/jdk/pull/17377/files - new: https://git.openjdk.org/jdk/pull/17377/files/2eb23440..010c5db8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17377&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17377&range=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17377.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17377/head:pull/17377 PR: https://git.openjdk.org/jdk/pull/17377
Re: RFR: 8323571: Regression in source resolution process [v2]
On Thu, 11 Jan 2024 20:35:33 GMT, Naoto Sato wrote: > LGTM. If the file was modified this year, put 2024 in the header Copyright year fixed. Thanks. - PR Comment: https://git.openjdk.org/jdk/pull/17377#issuecomment-1887951201
Integrated: JDK-8322235: Split up and improve LocaleProvidersRun
On Wed, 3 Jan 2024 23:30:41 GMT, Justin Lu wrote: > Please review this PR which splits up the _LocaleProvidersRun_ test file for > performance and maintenance reasons. > > _LocaleProvidersRun_ which tests against the various Locale Providers (CLDR, > HOST, SPI, etc.) was getting rather long, as all related bugs were added to > the single test file. To improve maintainability, this change splits up the > single test file into separate test files focused on specific areas (ex: > _j.text.Format_). The original _LocaleProvidersRun_ test file remains and is > used for more general Locale Provider testing such as the adapter loading. > > In addition, the previously single test file used to suffer from performance > issues, as each test method had to launch a new JVM (since Locale Providers > are set at Java startup time). With this change, these tests files can be ran > with VM flags and not cause timeout, thus `@requires vm.flagless` is no > longer needed (Tiers 6-8 tested). > > Other updates > - For OS/locale specific tests, the OS/locale is now checked before (not > after) launching a JVM > - Added comments for each test method This pull request has now been integrated. Changeset: 4ea7b364 Author:Justin Lu URL: https://git.openjdk.org/jdk/commit/4ea7b36447ea96d62b1ca164c34e2b2b74a16579 Stats: 569 lines in 8 files changed: 425 ins; 85 del; 59 mod 8322235: Split up and improve LocaleProvidersRun Reviewed-by: naoto - PR: https://git.openjdk.org/jdk/pull/17257
Re: RFR: 8323571: Regression in source resolution process
On Thu, 11 Jan 2024 18:36:39 GMT, Joe Wang wrote: > Fix a regression in the source resolution process where it failed to > recognize a custom InputSource when both the public and system IDs are null. > The particular issue is fixed at line 1233, the rest of the changes is > formatting to make the null-check on InputSource the first. > > Test is provided by Marc, thanks Marc! > (https://github.com/junit-team/junit5/issues/3594) > > XML tests, including the new test, pass with this change. LGTM. If the file was modified this year, put 2024 in the header - Marked as reviewed by naoto (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17377#pullrequestreview-1816602664
Re: RFR: JDK-8321545: Override toString() for Format subclasses [v2]
On Thu, 11 Jan 2024 19:30:09 GMT, Justin Lu wrote:
>> Please review this PR which implements toString() for the `Format`
>> subclasses. Corresponding CSR:
>> [JDK-8323088](https://bugs.openjdk.org/browse/JDK-8323088)
>>
>> The general specification follows a template that provides the locale (if
>> the class is localized) and any relevant patterns. The specification was
>> intentionally kept minimal and deliberately worded as "for debugging".
>>
>> An example of all the classes has output such as
>>
>>
>> CompactNumberFormat [locale: "English (United States)", compact patterns:
>> "[, , , {one:0K other:0K}, {one:00K other:00K}, {one:000K other:000K},
>> {one:0M other:0M}, {one:00M other:00M}, {one:000M other:000M}, {one:0B
>> other:0B}, {one:00B other:00B}, {one:000B other:000B}, {one:0T other:0T},
>> {one:00T other:00T}, {one:000T other:000T}]", decimal pattern:
>> "foo#0.00#baz"]
>>
>> DecimalFormat [locale: "English (United States)", pattern: "foo#0.00#baz"]
>>
>> SimpleDateFormat [locale: "Chinese (China)", pattern: "EEE, MMM d, ''yy"]
>>
>> ListFormat [locale: "English (United States)", start: "{0}, {1}", middle:
>> "{0}, {1}", end: "{0}, and {1}", two: "{0} and {1}", three: "{0}, {1}, and
>> {2}"]
>>
>> MessageFormat [locale: "Chinese (China)", pattern: "foo {0}"]
>>
>> ChoiceFormat [pattern: "0#foo"]
>
> Justin Lu 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:
>
> - Merge branch 'master' into JDK-8321545-toString-j.text.Format
> - account for null locale for SDF through deserialization
> - Merge branch 'master' into JDK-8321545-toString-j.text.Format
> - init
I think test cases for these new overridden `toString()` methods would be
helpful.
-
PR Comment: https://git.openjdk.org/jdk/pull/17355#issuecomment-1887851029
Re: RFR: JDK-8321545: Override toString() for Format subclasses [v2]
> Please review this PR which implements toString() for the `Format`
> subclasses. Corresponding CSR:
> [JDK-8323088](https://bugs.openjdk.org/browse/JDK-8323088)
>
> The general specification follows a template that provides the locale (if the
> class is localized) and any relevant patterns. The specification was
> intentionally kept minimal and deliberately worded as "for debugging".
>
> An example of all the classes has output such as
>
>
> CompactNumberFormat [locale: "English (United States)", compact patterns: "[,
> , , {one:0K other:0K}, {one:00K other:00K}, {one:000K other:000K}, {one:0M
> other:0M}, {one:00M other:00M}, {one:000M other:000M}, {one:0B other:0B},
> {one:00B other:00B}, {one:000B other:000B}, {one:0T other:0T}, {one:00T
> other:00T}, {one:000T other:000T}]", decimal pattern: "foo#0.00#baz"]
>
> DecimalFormat [locale: "English (United States)", pattern: "foo#0.00#baz"]
>
> SimpleDateFormat [locale: "Chinese (China)", pattern: "EEE, MMM d, ''yy"]
>
> ListFormat [locale: "English (United States)", start: "{0}, {1}", middle:
> "{0}, {1}", end: "{0}, and {1}", two: "{0} and {1}", three: "{0}, {1}, and
> {2}"]
>
> MessageFormat [locale: "Chinese (China)", pattern: "foo {0}"]
>
> ChoiceFormat [pattern: "0#foo"]
Justin Lu 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:
- Merge branch 'master' into JDK-8321545-toString-j.text.Format
- account for null locale for SDF through deserialization
- Merge branch 'master' into JDK-8321545-toString-j.text.Format
- init
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17355/files
- new: https://git.openjdk.org/jdk/pull/17355/files/dfc49edb..1dfc949d
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=17355&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=17355&range=00-01
Stats: 3057 lines in 78 files changed: 1931 ins; 706 del; 420 mod
Patch: https://git.openjdk.org/jdk/pull/17355.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17355/head:pull/17355
PR: https://git.openjdk.org/jdk/pull/17355
Re: RFR: 8322149: ConcurrentHashMap copy constructor should not transfer from old table on presizing [v2]
On Thu, 11 Jan 2024 09:40:07 GMT, Volker Simonis wrote: > tryPresize(int size) is doing and if its size argument is supposed to contain > the additional number of elements which will be inserted into the hash map or > if it is a hint for the new total size of the hash map Argument `size` for `tryPresize()` is a hint for the **new total size** of the hash map. If the size is too small compared to the current size of the map, there will be no resizing. - PR Review Comment: https://git.openjdk.org/jdk/pull/17116#discussion_r1449296202
Re: RFR: 8323571: Regression in source resolution process
On Thu, 11 Jan 2024 18:36:39 GMT, Joe Wang wrote: > Fix a regression in the source resolution process where it failed to > recognize a custom InputSource when both the public and system IDs are null. > The particular issue is fixed at line 1233, the rest of the changes is > formatting to make the null-check on InputSource the first. > > Test is provided by Marc, thanks Marc! > (https://github.com/junit-team/junit5/issues/3594) > > XML tests, including the new test, pass with this change. Marked as reviewed by lancea (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17377#pullrequestreview-1816320558
RFR: 8323571: Regression in source resolution process
Fix a regression in the source resolution process where it failed to recognize a custom InputSource when both the public and system IDs are null. The particular issue is fixed at line 1233, the rest of the changes is formatting to make the null-check on InputSource the first. Test is provided by Marc, thanks Marc! (https://github.com/junit-team/junit5/issues/3594) XML tests, including the new test, pass with this change. - Commit messages: - 8323571: Regression in source resolution process Changes: https://git.openjdk.org/jdk/pull/17377/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17377&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8323571 Stats: 355 lines in 4 files changed: 339 ins; 0 del; 16 mod Patch: https://git.openjdk.org/jdk/pull/17377.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17377/head:pull/17377 PR: https://git.openjdk.org/jdk/pull/17377
Re: RFR: 8323529: Relativize test image dependencies in microbenchmarks
On Thu, 11 Jan 2024 14:17:39 GMT, Erik Joelsson wrote: >> In principle, I think tests should be executed from their corresponding >> test-support directory. (I think there is some JBS issue for this that's >> been around for some while.) The rationale is that this is a good directory >> for any arbitrary output generated from the test, for files that are just >> dumped in the current directory. (This happens from time to time with tests.) >> >> But if executing from the test image improves the code quality of the tests, >> I am not going to argue heavily against it. > >> In principle, I think tests should be executed from their corresponding >> test-support directory. (I think there is some JBS issue for this that's >> been around for some while.) The rationale is that this is a good directory >> for any arbitrary output generated from the test, for files that are just >> dumped in the current directory. (This happens from time to time with tests.) > > Magnus does bring up a good point. The root of the test image is a very bad > place to accidentally drop arbitrary files when run in our distributed test > system, but we don't really run the micros there, at least not like this, do > we? > If the test image dir could be relative to the test-support directory (via a > symlink, or knowing that `../test/` always points to the test image) then > running from out of there could work. As @erikj79 has figured out then the > root issue for me here is that it's not feasible to pass the test image > directory as a parameter (since the `@Fork` annotation needs compile-time > constants), so if it's possible to configure test-support directories to be > in arbitrary places then that would become very fragile. > > None of our JMH tests pollutes the current directory as far as I'm aware, > though. And if they did the current situation where the micros are run out of > the make directory might arguably be worse. Fair enough. This isn't worse than the current CWD. The test image dir and the support output dir have no guaranteed relationship relative to each other, so we can't rely any relative path between them. - PR Comment: https://git.openjdk.org/jdk/pull/17349#issuecomment-1887672959
Integrated: 8319571: Update jni/nullCaller/NullCallerTest.java to accept flags or mark as flagless
On Fri, 17 Nov 2023 16:27:44 GMT, Tim Prinzing wrote: > Updated to use ProcessTool helper class to run test passing flags. This pull request has now been integrated. Changeset: b78896b9 Author:Tim Prinzing Committer: Mandy Chung URL: https://git.openjdk.org/jdk/commit/b78896b9aafcb15f453eaed6e154a5461581407b Stats: 27 lines in 1 file changed: 1 ins; 21 del; 5 mod 8319571: Update jni/nullCaller/NullCallerTest.java to accept flags or mark as flagless Reviewed-by: mchung - PR: https://git.openjdk.org/jdk/pull/16711
Re: RFR: 8303866: Allow ZipInputStream.readEnd to parse small Zip64 ZIP files [v9]
On Wed, 10 Jan 2024 17:02:05 GMT, Eirik Bjørsnøs wrote: >> Sounds like a CSR is needed for the behavioral change proposed here. > >> Sounds like a CSR is needed for the behavioral change proposed here. > > Thanks for flagging this @jddarcy > > I'm personally not 100% convinced a CSR is warranted in this case, but I'm of > course happy to extend the following into a CSR if you or other reviewer > think that's the best way forward: > > Let's review the intended and unintended behavioral changes in this PR: > > The _intended_ behavioral change is to extend the set of ZIP entries parsable > by `ZipInputStream` to include "small Zip64 entries". Such entries meet the > following criteria: > > 1. They are clearly marked as using the Zip64 format, meaning that the LOC's > 'compressed size' and 'uncompressed size' fields are set to the "Zip64 magic > value" 0x and that the LOC's extra field includes a valid _Zip64 > Extended Information Field_. > 2. They use _streaming mode_, meaning that the 'general purpose bit flag' 3 > is set, that the Zip64 'Original Size' and 'Compressed Size' are both set to > zero and that file data is followed by a 'Data Descriptor' containing the > actual size values. > 3. The Data Descriptor represents the 'compressed size' and 'uncompressed' > size fields using 8 byte fields, instead of the regular 4 byte fields. > > These entires are currently not parsable by `ZipInputStream`. Trying to parse > them fails with an exception similar to the following: > > > java.util.zip.ZipException: invalid entry size (expected 0 but got 6 bytes) > at > java.base/java.util.zip.ZipInputStream.readEnd(ZipInputStream.java:616) > > > The _unintended_ behavioral change in this PR is that ZIP entries marked as > Zip64, but still having a Data Descriptor using 4 bytes to represent the size > fields will now become unparsable. Such entries meet critera 1 and 2 above, > but not 3. > > While I have not performed a corpus search, I believe such entries are rare, > if they exist at all: > > - They would be in clear violation of the `APPNOTE.txt` spec, which in > 4.3.9.2 says _ZIP64 format MAY be used regardless of the size of a file. > When extracting, if the zip64 extended information extra field is present for > the file the compressed and uncompressed sizes will be 8 byte values_ > - The `zipdetails` tool fails to parse such a file, instead reading the two > 4-byte numbers into one 8-byte 'compressed size' > - The file cannot be parsed by external APIs similar to `ZipInputStream`, > such as the Python `stream-unzip` module. > > To avoid unintended behavioral changes in the implementation, care has been > taken to make the Zip64 extra field validation code robust ... > @eirbjo, either of the intended or unintended effects of the PR on their own > would merit a CSR and this is a case where the inclusive-OR applies so a CSR > is definitely needed; thanks. Thanks Joe! A CSR has been proposed and is ready for the first round of feedback: https://bugs.openjdk.org/browse/JDK-8323583 - PR Comment: https://git.openjdk.org/jdk/pull/12524#issuecomment-1887559379
Re: RFR: 8323296: java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java#id1 timed out [v2]
On Thu, 11 Jan 2024 13:21:16 GMT, Jaikiran Pai wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Run in othervm mode
>
> test/jdk/java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java
> line 52:
>
>> 50: // need at least two carrier threads when main thread is a
>> virtual thread
>> 51: if (Thread.currentThread().isVirtual()) {
>> 52: VThreadRunner.ensureParallelism(2);
>
> Given that this changes the JVM level scheduler's parallelism, should we now
> use `/othervm` for these tests to prevent this change interfering with other
> tests?
The ensureParallelism here is to allow the test run on a container configured
with a single CPU. I don't think it will impact any other tests that run in the
same agent VM, at least I couldn't find any tests where it would create an
issue. But you are right that it would be better to restore it or run the test
in /othervm mode. The equivalent test in the loom repo does run in /othervm
mode as it has to launch with some VM options so we can keep it consistent with
that. Thanks.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17353#discussion_r1449134168
Re: RFR: 8323296: java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java#id1 timed out [v2]
> This test was recently dialled down via JDK-8323002 but it still makes slow > progress on some test machines, esp. macox-x64-debug builds. The issue is > that the sampling of the target thread is skewed towards the unmounted case > so the target thread is disabled from being scheduled and doesn't make > progress. The test is re-worked to use a barrier so that the main thread and > target virtual thread run in lock step. This allows the virtual thread to > make progress at each iteration and also increases the chances of sampling > the stack trace at around the time that the target thread transitions from > being unmounted (due to the Thread.yield) and parking while pinned. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Run in othervm mode - Changes: - all: https://git.openjdk.org/jdk/pull/17353/files - new: https://git.openjdk.org/jdk/pull/17353/files/55978a82..4b174603 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17353&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17353&range=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17353.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17353/head:pull/17353 PR: https://git.openjdk.org/jdk/pull/17353
Re: RFR: 8323552: AbstractMemorySegmentImpl#mismatch returns -1 when comparing distinct areas of the same instance of MemorySegment [v2]
On Thu, 11 Jan 2024 15:23:14 GMT, Jorn Vernee wrote: >> Peter Levart has updated the pull request incrementally with one additional >> commit since the last revision: >> >> move special-case check for equal segments to the instance method (more >> probable) > > src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java > line 299: > >> 297: checkValidState(); >> 298: return -1; >> 299: } > > I'm skeptical of trying to optimize this without a more thorough performance > investigation. We shouldn't add this complexity on a whim. > > I'm more in favor of just dropping the check from the static mismatch method > and leaving it at that. (And adding a test as Maurizio mentioned). I have similar thoughts. This is low-level API. Code using it has more context knowledge and is more fit to do optimisations like that or not, depending if they are worth it. I would want such low level API to do as little checks as possible to accommodate faster common case scenario. - PR Review Comment: https://git.openjdk.org/jdk/pull/17354#discussion_r1449093606
Re: RFR: 8323552: AbstractMemorySegmentImpl#mismatch returns -1 when comparing distinct areas of the same instance of MemorySegment [v3]
> I belive there is a bug in `AbstractMemorySegmentImpl#mismatch` method. It > returns `-1` (meaning that regions are equal) when passing the same instance > of MemorySegment as both `srcSegment` and `dstSegment` parameters regardless > of whether `srcFromOffset` and `dstFromOffset` as well as `srcToOffset` and > `dstToOffset` are also equal. > > Am I right? Peter Levart has updated the pull request incrementally with one additional commit since the last revision: remove special-case check from instance method - test pending - Changes: - all: https://git.openjdk.org/jdk/pull/17354/files - new: https://git.openjdk.org/jdk/pull/17354/files/5112839b..22417ea3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17354&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17354&range=01-02 Stats: 11 lines in 1 file changed: 0 ins; 11 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17354.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17354/head:pull/17354 PR: https://git.openjdk.org/jdk/pull/17354
Re: RFR: 8323552: AbstractMemorySegmentImpl#mismatch returns -1 when comparing distinct areas of the same instance of MemorySegment [v2]
On Thu, 11 Jan 2024 13:30:44 GMT, Peter Levart wrote: >> I belive there is a bug in `AbstractMemorySegmentImpl#mismatch` method. It >> returns `-1` (meaning that regions are equal) when passing the same instance >> of MemorySegment as both `srcSegment` and `dstSegment` parameters regardless >> of whether `srcFromOffset` and `dstFromOffset` as well as `srcToOffset` and >> `dstToOffset` are also equal. >> >> Am I right? > > Peter Levart has updated the pull request incrementally with one additional > commit since the last revision: > > move special-case check for equal segments to the instance method (more > probable) src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java line 299: > 297: checkValidState(); > 298: return -1; > 299: } I'm skeptical of trying to optimize this without a more thorough performance investigation. We shouldn't add this complexity on a whim. I'm more in favor of just dropping the check from the static mismatch method and leaving it at that. (And adding a test as Maurizio mentioned). - PR Review Comment: https://git.openjdk.org/jdk/pull/17354#discussion_r1449022827
Re: RFR: 8323529: Relativize test image dependencies in microbenchmarks
On Thu, 11 Jan 2024 14:17:39 GMT, Erik Joelsson wrote: >> In principle, I think tests should be executed from their corresponding >> test-support directory. (I think there is some JBS issue for this that's >> been around for some while.) The rationale is that this is a good directory >> for any arbitrary output generated from the test, for files that are just >> dumped in the current directory. (This happens from time to time with tests.) >> >> But if executing from the test image improves the code quality of the tests, >> I am not going to argue heavily against it. > >> In principle, I think tests should be executed from their corresponding >> test-support directory. (I think there is some JBS issue for this that's >> been around for some while.) The rationale is that this is a good directory >> for any arbitrary output generated from the test, for files that are just >> dumped in the current directory. (This happens from time to time with tests.) > > Magnus does bring up a good point. The root of the test image is a very bad > place to accidentally drop arbitrary files when run in our distributed test > system, but we don't really run the micros there, at least not like this, do > we? If the test image dir could be relative to the test-support directory (via a symlink, or knowing that `../test/` always points to the test image) then running from out of there could work. As @erikj79 has figured out then the root issue for me here is that it's not feasible to pass the test image directory as a parameter (since the `@Fork` annotation needs compile-time constants), so if it's possible to configure test-support directories to be in arbitrary places then that would become very fragile. None of our JMH tests pollutes the current directory as far as I'm aware, though. And if they did the current situation where the micros are run out of the make directory might arguably be worse. - PR Comment: https://git.openjdk.org/jdk/pull/17349#issuecomment-1887373227
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v2]
On Thu, 28 Dec 2023 15:19:23 GMT, Jan Kratochvil wrote: >> The testcase requires root permissions. > > Jan Kratochvil has updated the pull request incrementally with one additional > commit since the last revision: > > Fix gtest testcases compilation errors I'm looking at it. Review should be ready this or early next week. - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-1887311496
Re: RFR: 8323529: Relativize test image dependencies in microbenchmarks
On Thu, 11 Jan 2024 11:23:55 GMT, Magnus Ihse Bursie wrote: > In principle, I think tests should be executed from their corresponding > test-support directory. (I think there is some JBS issue for this that's been > around for some while.) The rationale is that this is a good directory for > any arbitrary output generated from the test, for files that are just dumped > in the current directory. (This happens from time to time with tests.) Magnus does bring up a good point. The root of the test image is a very bad place to accidentally drop arbitrary files when run in our distributed test system, but we don't really run the micros there, at least not like this, do we? - PR Comment: https://git.openjdk.org/jdk/pull/17349#issuecomment-1887269464
Re: RFR: 8323296: java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java#id1 timed out
On Thu, 11 Jan 2024 14:12:03 GMT, Alan Bateman wrote: > There are intermittent states and the stack walk will depend on whether the > target is mounted or not. Thank you for that detail. - PR Review Comment: https://git.openjdk.org/jdk/pull/17353#discussion_r1448930993
Re: RFR: 8323296: java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java#id1 timed out
On Thu, 11 Jan 2024 13:49:14 GMT, Jaikiran Pai wrote:
>> This test was recently dialled down via JDK-8323002 but it still makes slow
>> progress on some test machines, esp. macox-x64-debug builds. The issue is
>> that the sampling of the target thread is skewed towards the unmounted case
>> so the target thread is disabled from being scheduled and doesn't make
>> progress. The test is re-worked to use a barrier so that the main thread and
>> target virtual thread run in lock step. This allows the virtual thread to
>> make progress at each iteration and also increases the chances of sampling
>> the stack trace at around the time that the target thread transitions from
>> being unmounted (due to the Thread.yield) and parking while pinned.
>
> test/jdk/java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java
> line 65:
>
>> 63: for (int i = 0; i < iterations; i++) {
>> 64: // wait for main thread to arrive
>> 65: barrier.await();
>
> Given the goal of this test, which is "Stress test Thread.getStackTrace on a
> virtual thread that is pinned", shouldn't this `barrier.await()` be a few
> lines below, inside the `synchronized (GetStackTraceALotWhenPinned.class)`
> block, just before it parks itself? That way, the main thread when it too
> reaches the barrier and when it next issues a getStackTrace() call, the
> chances of the other thread being pinned would be high(er)?
It's in the right place. This is a stress test to bash on Thread::getStackTrace
where the target virtual thread is transitioning from being unmounted to
parked-while-pinned. There are intermittent states and the stack walk will
depend on whether the target is mounted or not. There are other tests that
exercise Thread::getStackTrace on pinned threads, specifically
ThreadAPI.testGetStackTrace6 and ThreadAPI.testGetStackTrace7, but different to
the stress testing that is done here.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17353#discussion_r1448924715
Re: RFR: 8323296: java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java#id1 timed out
On Wed, 10 Jan 2024 20:25:21 GMT, Alan Bateman wrote:
> This test was recently dialled down via JDK-8323002 but it still makes slow
> progress on some test machines, esp. macox-x64-debug builds. The issue is
> that the sampling of the target thread is skewed towards the unmounted case
> so the target thread is disabled from being scheduled and doesn't make
> progress. The test is re-worked to use a barrier so that the main thread and
> target virtual thread run in lock step. This allows the virtual thread to
> make progress at each iteration and also increases the chances of sampling
> the stack trace at around the time that the target thread transitions from
> being unmounted (due to the Thread.yield) and parking while pinned.
test/jdk/java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java line
65:
> 63: for (int i = 0; i < iterations; i++) {
> 64: // wait for main thread to arrive
> 65: barrier.await();
Given the goal of this test, which is "Stress test Thread.getStackTrace on a
virtual thread that is pinned", shouldn't this `barrier.await()` be a few lines
below, inside the `synchronized (GetStackTraceALotWhenPinned.class)` block,
just before it parks itself? That way, the main thread when it too reaches the
barrier and when it next issues a getStackTrace() call, the chances of the
other thread being pinned would be high(er)?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17353#discussion_r1448894710
Re: RFR: 8323552: AbstractMemorySegmentImpl#mismatch returns -1 when comparing distinct areas of the same instance of MemorySegment [v2]
> I belive there is a bug in `AbstractMemorySegmentImpl#mismatch` method. It > returns `-1` (meaning that regions are equal) when passing the same instance > of MemorySegment as both `srcSegment` and `dstSegment` parameters regardless > of whether `srcFromOffset` and `dstFromOffset` as well as `srcToOffset` and > `dstToOffset` are also equal. > > Am I right? Peter Levart has updated the pull request incrementally with one additional commit since the last revision: move special-case check for equal segments to the instance method (more probable) - Changes: - all: https://git.openjdk.org/jdk/pull/17354/files - new: https://git.openjdk.org/jdk/pull/17354/files/7bcf0273..5112839b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17354&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17354&range=00-01 Stats: 15 lines in 1 file changed: 11 ins; 4 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17354.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17354/head:pull/17354 PR: https://git.openjdk.org/jdk/pull/17354
Re: RFR: 8323296: java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java#id1 timed out
On Wed, 10 Jan 2024 20:25:21 GMT, Alan Bateman wrote:
> This test was recently dialled down via JDK-8323002 but it still makes slow
> progress on some test machines, esp. macox-x64-debug builds. The issue is
> that the sampling of the target thread is skewed towards the unmounted case
> so the target thread is disabled from being scheduled and doesn't make
> progress. The test is re-worked to use a barrier so that the main thread and
> target virtual thread run in lock step. This allows the virtual thread to
> make progress at each iteration and also increases the chances of sampling
> the stack trace at around the time that the target thread transitions from
> being unmounted (due to the Thread.yield) and parking while pinned.
test/jdk/java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java line
52:
> 50: // need at least two carrier threads when main thread is a
> virtual thread
> 51: if (Thread.currentThread().isVirtual()) {
> 52: VThreadRunner.ensureParallelism(2);
Given that this changes the JVM level scheduler's parallelism, should we now
use `/othervm` for these tests to prevent this change interfering with other
tests?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17353#discussion_r1448858813
Re: RFR: 8322744: VirtualThread.notifyJvmtiDisableSuspend should be static [v2]
> The notification method `VirtualThread.notifyJvmtiDisableSuspend` should be > static. > The method disables/enables suspend of the current virtual thread, a no-op if > the current thread is a platform thread. It is confusing for this to be an > instance method, it should be static to make it clearer that it doesn't > change the target thread. > The notification method `VirtualThread.notifyJvmtiHideFrames` also has to be > static as it does not use/need the virtual thread `this` argument. > One detail to underline is that he intrinsic implementation needs to use the > argument #0 instead of #1. > > Testing: > - The mach5 tiers 1-6 show no regressions Serguei Spitsyn 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 two additional commits since the last revision: - Merge - 8322744: VirtualThread.notifyJvmtiDisableSuspend should be static - Changes: - all: https://git.openjdk.org/jdk/pull/17298/files - new: https://git.openjdk.org/jdk/pull/17298/files/2b684607..66bcce95 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17298&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17298&range=00-01 Stats: 17642 lines in 345 files changed: 12363 ins; 3111 del; 2168 mod Patch: https://git.openjdk.org/jdk/pull/17298.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17298/head:pull/17298 PR: https://git.openjdk.org/jdk/pull/17298
[jdk22] Integrated: 8318971: Better Error Handling for Jar Tool When Processing Non-existent Files
On Wed, 10 Jan 2024 17:42:21 GMT, Weibing Xiao wrote: > Better Error Handling for Jar Tool When Processing Non-existent Files. > > It needs to be backported to JDK22. This pull request has now been integrated. Changeset: 3daa936f Author:Weibing Xiao Committer: Jaikiran Pai URL: https://git.openjdk.org/jdk22/commit/3daa936f2da4da853e0cbe8dcb081e3a7509734f Stats: 86 lines in 2 files changed: 84 ins; 0 del; 2 mod 8318971: Better Error Handling for Jar Tool When Processing Non-existent Files Reviewed-by: jpai Backport-of: 8ae309ebacd6947bbad2ef168ca13702e1cba099 - PR: https://git.openjdk.org/jdk22/pull/56
[jdk22] Integrated: 8322324: java/foreign/TestStubAllocFailure.java times out while waiting for forked process
On Wed, 10 Jan 2024 13:18:17 GMT, Jorn Vernee wrote: > Hi all, > > This pull request contains a backport of commit > [d2d58dd6](https://github.com/openjdk/jdk/commit/d2d58dd6a8ec366a4bc3eb12a253b252de24557e) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Jorn Vernee on 10 Jan 2024 and > was reviewed by Maurizio Cimadamore. > > This is a P2 test only change, and we are currently in Ramp Down Phase 1. The > release process allows P1-P5 test-only changes during RDP1: > https://openjdk.org/jeps/3#Quick-reference > > Thanks! This pull request has now been integrated. Changeset: db342758 Author:Jorn Vernee URL: https://git.openjdk.org/jdk22/commit/db3427582df5913e9299e8ecceecd9bddb3b7358 Stats: 16 lines in 1 file changed: 3 ins; 0 del; 13 mod 8322324: java/foreign/TestStubAllocFailure.java times out while waiting for forked process 8322637: java/foreign/critical/TestCriticalUpcall.java timed out Reviewed-by: mcimadamore Backport-of: d2d58dd6a8ec366a4bc3eb12a253b252de24557e - PR: https://git.openjdk.org/jdk22/pull/52
Re: [jdk22] RFR: 8318971: Better Error Handling for Jar Tool When Processing Non-existent Files
On Wed, 10 Jan 2024 17:42:21 GMT, Weibing Xiao wrote: > Better Error Handling for Jar Tool When Processing Non-existent Files. > > It needs to be backported to JDK22. Clean backport of a P3 bug fix. - Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk22/pull/56#pullrequestreview-1815371942
Re: RFR: 8303374: Compiler Implementation for Primitive types in patterns, instanceof, and switch (Preview) [v38]
On Thu, 14 Dec 2023 11:04:09 GMT, Aggelos Biboudis wrote: >> This is the proposed patch for Primitive types in patterns, instanceof, and >> switch (Preview). >> >> Draft spec here: https://cr.openjdk.org/~abimpoudis/instanceof/latest/ > > Aggelos Biboudis has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 54 commits: > > - Merge branch 'master' into primitive-patterns > - Cleanup > - Merge branch 'master' into primitive-patterns > - Remove trailing spaces > - Merge branch 'primitive-patterns-and-generating-dispatch' into > primitive-patterns > - Fixed switch in the cases of unboxing and widening > - Merge branch 'JDK-8319220' into primitive-patterns > - Merge branch 'master' into JDK-8319220 > - reflecting review comment: fixing letter case. > - Reflecting review feedback. > - ... and 44 more: https://git.openjdk.org/jdk/compare/d2ba3b1e...a03fea7c Keep open 🚀🚀🚀 - PR Comment: https://git.openjdk.org/jdk/pull/15638#issuecomment-1886978935
Re: RFR: 8323529: Relativize test image dependencies in microbenchmarks
On Wed, 10 Jan 2024 15:10:58 GMT, Claes Redestad wrote: > JMH microbenchmarks may have dependencies on artifacts in the test image > outside of the benchmarks.jar. This includes native libraries (built into > `$TEST_IMAGE/micro/native`) and may soon include other test libraries like > wb.jar (built into `$TEST_IMAGE/lib-test/`) > > By moving execution to the test image root (currently we run out of the > `make` directory) we can make do with relative paths, which means benchmark > can supply a build-time constant flag themselves rather than have the test > runner provide it for you. And needing to be explicit about external > dependencies is good, I think. > > Taken together this makes the benchmarks.jar more self-contained and simpler > to run: all you need is to run `java -jar micro/benchmarks.jar` from the test > image root. > > This patch also drive-by fixes some lang.foreign tests that were printing > warnings due to a missing `--enable-native-access=ALL-UNNAMED` flag. In principle, I think tests should be executed from their corresponding test-support directory. (I think there is some JBS issue for this that's been around for some while.) The rationale is that this is a good directory for any arbitrary output generated from the test, for files that are just dumped in the current directory. (This happens from time to time with tests.) But if executing from the test image improves the code quality of the tests, I am not going to argue heavily against it. - PR Comment: https://git.openjdk.org/jdk/pull/17349#issuecomment-1886933844
Withdrawn: 8319200: Don't use test thread factory in ProcessTools.createLimitedTestJavaProcessBuilder()
On Wed, 1 Nov 2023 00:06:35 GMT, Leonid Mesnik wrote: > Test thread factory is a mode similar to VM flags and should not be used in > ProcessTools.createLimitedTestJavaProcessBuilder(). Only > createTestJavaProcessBuilder() should use it like jtreg VM options. > > Adding the test thread factory requires the injection of arguments in the > middle of the list. I don't think it makes sense to modify arguments in > several places so I replaced it with the flag isLimited and moved all > modifications in createJavaProcessBuilder(). > > Testing tier1-5. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/16442
Re: RFR: 8310813: Simplify and modernize equals, hashCode, and compareTo for BigInteger [v12]
On Thu, 11 Jan 2024 10:34:45 GMT, Pavel Rappo wrote: >> Please review this PR to use modern APIs and language features to simplify >> equals, hashCode, and compareTo for BigInteger. If you have any performance >> concerns, please raise them. >> >> This PR is cherry-picked from a bigger, not-yet-published PR, to test the >> waters. That latter PR will be published soon. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Revert copyright year change > > Those files were first published in 2023 and haven't been changed since. LGTM - Marked as reviewed by rgiulietti (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14630#pullrequestreview-1815217411
Re: RFR: 8310813: Simplify and modernize equals, hashCode, and compareTo for BigInteger [v12]
> Please review this PR to use modern APIs and language features to simplify > equals, hashCode, and compareTo for BigInteger. If you have any performance > concerns, please raise them. > > This PR is cherry-picked from a bigger, not-yet-published PR, to test the > waters. That latter PR will be published soon. Pavel Rappo has updated the pull request incrementally with one additional commit since the last revision: Revert copyright year change Those files were first published in 2023 and haven't been changed since. - Changes: - all: https://git.openjdk.org/jdk/pull/14630/files - new: https://git.openjdk.org/jdk/pull/14630/files/08e6adca..31e9eefd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14630&range=11 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14630&range=10-11 Stats: 5 lines in 5 files changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/14630.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14630/head:pull/14630 PR: https://git.openjdk.org/jdk/pull/14630
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v2]
On Thu, 28 Dec 2023 15:19:23 GMT, Jan Kratochvil wrote: >> The testcase requires root permissions. > > Jan Kratochvil has updated the pull request incrementally with one additional > commit since the last revision: > > Fix gtest testcases compilation errors ping, are there some concerns during review of this patch? If you don't like too much some part just point at it, I will try to improve it. Thanks. - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-1886731985
Re: RFR: 8322149: ConcurrentHashMap copy constructor should not transfer from old table on presizing [v2]
On Mon, 8 Jan 2024 20:27:59 GMT, Joshua Cao wrote: > We don't need to compute max() here. > [tryPresize()](https://github.com/openjdk/jdk/blob/8a4dc79e1a40e7115e2971af81623b6b0368f41c/src/java.base/share/classes/java/util/concurrent/ConcurrentHashMap.java#L2397) > does that already. The code you reference is only executed if `table == null` but in this case we know it is not `null` because we only call `tryPresize()` if `table != null`. In general, it's hard fro me to understand what `tryPresize(int size)` is doing and if its `size` argument is supposed to contain the additional number of elements which will be inserted into the hash map or if it is a hint for the new total size of the hash map. Can you explain? - PR Review Comment: https://git.openjdk.org/jdk/pull/17116#discussion_r1448566797
Re: RFR: JDK-8322979: Add informative discussion to Modifier [v2]
On Wed, 10 Jan 2024 10:17:10 GMT, Pavel Rappo wrote: >> Joe Darcy has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Implement review feedback. > > src/java.base/share/classes/java/lang/reflect/Modifier.java line 238: > >> 236: * modifiers of a class or member, source-level modifiers that do >> 237: * not not have a constant in the this class should be >> 238: * included and ordered consistent with the full recommended > > Suggestion: > > * included and ordered consistently with the full recommended Yes, "ordered consistently with" or "with order consistent with", I think both will work here. - PR Review Comment: https://git.openjdk.org/jdk/pull/17338#discussion_r1448564687
Re: RFR: 8314515: java/util/concurrent/SynchronousQueue/Fairness.java failed with "Error: fair=false i=8 j=0" [v2]
On Wed, 10 Jan 2024 20:30:26 GMT, Alan Bateman wrote: > Updated version looks much better. Thanks @AlanBateman! - PR Comment: https://git.openjdk.org/jdk/pull/17082#issuecomment-1886591099
Integrated: 8314515: java/util/concurrent/SynchronousQueue/Fairness.java failed with "Error: fair=false i=8 j=0"
On Tue, 12 Dec 2023 15:13:01 GMT, Viktor Klang wrote: > While this might not fix 8314515, it should at least make it more exact. This pull request has now been integrated. Changeset: 35e96627 Author:Viktor Klang URL: https://git.openjdk.org/jdk/commit/35e9662767cc0a1dea9b5afa2a6d61a85297253c Stats: 35 lines in 1 file changed: 28 ins; 0 del; 7 mod 8314515: java/util/concurrent/SynchronousQueue/Fairness.java failed with "Error: fair=false i=8 j=0" Reviewed-by: alanb - PR: https://git.openjdk.org/jdk/pull/17082
Re: RFR: 8320788: The system properties page is missing some properties [v3]
On Tue, 9 Jan 2024 23:40:35 GMT, Naoto Sato wrote: >> Adding an explanation of the locale-related system properties in the >> `System.getProperties()` method. Corresponding CSR has also been drafted. > > Naoto Sato has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 18 commits: > > - Merge branch 'master' of https://git.openjdk.org/jdk into > JDK-8320788-system.getProperties > - Reflects review comments > - initial commit > - Reflects review comments > - Reflects review comments > - Reflects review comments > - Reflects review comments > - Review comments > - Update src/java.base/share/classes/java/util/Locale.java > >Co-authored-by: Justin Lu > - Update src/java.base/share/classes/java/util/Locale.java > >Co-authored-by: Justin Lu > - ... and 8 more: https://git.openjdk.org/jdk/compare/376051a9...7444bb51 We need to think about whether the change proposed here will set precedence or not. There are many places in the API docs where system properties are specified but they aren't linked from System::getProperties. - PR Comment: https://git.openjdk.org/jdk/pull/17317#issuecomment-1886570976
Re: RFR: 8323159: Consider adding some text re. memory zeroing in Arena::allocate [v2]
> This PR proposes to add a clarification that an `Arena` always returns > zeroed-out segments for `Arena::allocate` methods. > > Note that other overloaded methods refer to the abstract `Arena::allocate` > method via implementation notes. Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Move and change text on zeroing out - Changes: - all: https://git.openjdk.org/jdk/pull/17308/files - new: https://git.openjdk.org/jdk/pull/17308/files/4c777398..04074d71 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17308&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17308&range=00-01 Stats: 9 lines in 1 file changed: 5 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/17308.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17308/head:pull/17308 PR: https://git.openjdk.org/jdk/pull/17308
