Integrated: 8326915: NPE when a validating parser is restricted

2024-03-01 Thread Joe Wang
On Thu, 29 Feb 2024 21:00:09 GMT, Joe Wang  wrote:

> Fix a NPE when a validating parser is restricted by the JDKCatalog resolve 
> property. Also slightly improved the error msg with the catalog name.
> 
> Test: new test added
>  existing test CatalogSupport5 would fail without the Null check on 
> ErrorReporter. It's a separate issue not covered by this fix.

This pull request has now been integrated.

Changeset: a3d51d20
Author:Joe Wang 
URL:   
https://git.openjdk.org/jdk/commit/a3d51d2027c18e88703022d1c65a3048b9f2967e
Stats: 116 lines in 5 files changed: 101 ins; 0 del; 15 mod

8326915: NPE when a validating parser is restricted

Reviewed-by: lancea, naoto

-

PR: https://git.openjdk.org/jdk/pull/18071


RFR: 8327138: Clean up status management in cdsConfig.hpp and CDS.java

2024-03-01 Thread Ioi Lam
A few clean ups:

1. Rename functions like "`s_loading_full_module_graph()` to 
`is_using_full_module_graph()`. The meaning of "loading" is not clear: it might 
be interpreted as to cover only the period where the artifact is being loaded, 
but not the period after the artifact is completely loaded. However, the 
function is meant to cover both periods, so "using" is a more precise term.

2. The cumbersome sounding `disable_loading_full_module_graph()` is changed to 
`stop_using_full_module_graph()`, etc.

3. The status of `is_using_optimized_module_handling()` is moved from 
metaspaceShared.hpp to cdsConfig.hpp, to be consolidated with other types of 
CDS status.

4. The status of CDS was communicated to the Java class `jdk.internal.misc.CDS` 
by ad-hoc native methods. This is now changed to a single method, 
`CDS.getCDSConfigStatus()` that returns a bit field. That way we don't need to 
add a new native method for each type of status.

5. `CDS.isDumpingClassList()` was a misnomer. It's changed to 
`CDS.isLoggingLambdaFormInvokers()`.

-

Commit messages:
 - 8327138: Clean up status management in cdsConfig.hpp and CDS.java

Changes: https://git.openjdk.org/jdk/pull/18095/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18095&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8327138
  Stats: 191 lines in 19 files changed: 43 ins; 53 del; 95 mod
  Patch: https://git.openjdk.org/jdk/pull/18095.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18095/head:pull/18095

PR: https://git.openjdk.org/jdk/pull/18095


RFR: JDK-8326908: DecimalFormat::toPattern throws OutOfMemoryError when pattern is empty string

2024-03-01 Thread Justin Lu
Please review this PR and corresponding CSR which prevents an OutOfMemoryError 
by restricting the initial maximum fraction digits for an empty pattern 
DecimalFormat.

The cause is that the maximum fraction digits is initialized to 
`Integer.MAX_VALUE` which causes StringBuilder to eventually throw OOME when 
internally doubling capacity too many times when toPattern is invoked. CSR 
covers potential behavioral compatibility changes.

-

Commit messages:
 - minor additions
 - init

Changes: https://git.openjdk.org/jdk/pull/18094/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18094&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8326908
  Stats: 52 lines in 2 files changed: 51 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/18094.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18094/head:pull/18094

PR: https://git.openjdk.org/jdk/pull/18094


Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref [v11]

2024-03-01 Thread Brent Christian
> Classes in the `java.lang.ref` package would benefit from an update to bring 
> the spec in line with how the VM already behaves. The changes would focus on 
> _happens-before_ edges at some key points during reference processing.
> 
> A couple key things we want to be able to say are:
> - `Reference.reachabilityFence(x)` _happens-before_ reference processing 
> occurs for 'x'.
> - `Cleaner.register()` _happens-before_ the Cleaner thread runs the 
> registered cleaning action.
> 
> This will bring Cleaner in line (or close) with the memory visibility 
> guarantees made for finalizers in [JLS 
> 17.4.5](https://docs.oracle.com/javase/specs/jls/se18/html/jls-17.html#jls-17.4.5):
> _"There is a happens-before edge from the end of a constructor of an object 
> to the start of a finalizer (§12.6) for that object."_

Brent Christian has updated the pull request incrementally with one additional 
commit since the last revision:

  Tiny clarification to 'strongly reachable' definition; add link to Reference 
class

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16644/files
  - new: https://git.openjdk.org/jdk/pull/16644/files/adb1fbe3..855b13a8

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16644&range=10
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16644&range=09-10

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/16644.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16644/head:pull/16644

PR: https://git.openjdk.org/jdk/pull/16644


Re: RFR: 8309622: Re-examine the cache mechanism in BaseLocale [v11]

2024-03-01 Thread Naoto Sato
> This is stemming from the PR: https://github.com/openjdk/jdk/pull/14211 where 
> aggressive GC can cause NPE in `BaseLocale$Key` class. I refactored the 
> in-house cache with WeakHashMap, and removed the Key class as it is no longer 
> needed (thus the original NPE will no longer be possible). Also with the new 
> JMH test case, it gains some performance improvement:
> 
> (w/o fix)
> 
> Benchmark   Mode  Cnt  Score Error  Units
> LocaleCache.testForLanguageTag  avgt   20   5781.275 ± 569.580  ns/op
> LocaleCache.testLocaleOfavgt   20  62564.079 ± 406.697  ns/op
> 
> (w/ fix)
> Benchmark   Mode  Cnt  Score Error  Units
> LocaleCache.testForLanguageTag  avgt   20   4801.175 ± 371.830  ns/op
> LocaleCache.testLocaleOfavgt   20  60394.652 ± 352.471  ns/op

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  copyright year revert

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14404/files
  - new: https://git.openjdk.org/jdk/pull/14404/files/84f5f692..41150e12

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14404&range=10
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14404&range=09-10

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/14404.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14404/head:pull/14404

PR: https://git.openjdk.org/jdk/pull/14404


Re: RFR: 8309622: Re-examine the cache mechanism in BaseLocale [v11]

2024-03-01 Thread Roger Riggs
On Fri, 1 Mar 2024 21:29:17 GMT, Naoto Sato  wrote:

>> This is stemming from the PR: https://github.com/openjdk/jdk/pull/14211 
>> where aggressive GC can cause NPE in `BaseLocale$Key` class. I refactored 
>> the in-house cache with WeakHashMap, and removed the Key class as it is no 
>> longer needed (thus the original NPE will no longer be possible). Also with 
>> the new JMH test case, it gains some performance improvement:
>> 
>> (w/o fix)
>> 
>> Benchmark   Mode  Cnt  Score Error  Units
>> LocaleCache.testForLanguageTag  avgt   20   5781.275 ± 569.580  ns/op
>> LocaleCache.testLocaleOfavgt   20  62564.079 ± 406.697  ns/op
>> 
>> (w/ fix)
>> Benchmark   Mode  Cnt  Score Error  Units
>> LocaleCache.testForLanguageTag  avgt   20   4801.175 ± 371.830  ns/op
>> LocaleCache.testLocaleOfavgt   20  60394.652 ± 352.471  ns/op
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   copyright year revert

Much simpler and looks good.  Thanks

-

Marked as reviewed by rriggs (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14404#pullrequestreview-1912051206


Integrated: JDK-8316708: Augment WorstCaseTests with more cases

2024-03-01 Thread Joe Darcy
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.

This pull request has now been integrated.

Changeset: 7f02f07f
Author:Joe Darcy 
URL:   
https://git.openjdk.org/jdk/commit/7f02f07f754c942735ba15d70858cd1661a658c0
Stats: 307 lines in 13 files changed: 275 ins; 0 del; 32 mod

8316708: Augment WorstCaseTests with more cases

Reviewed-by: rgiulietti

-

PR: https://git.openjdk.org/jdk/pull/15879


Re: RFR: 8309622: Re-examine the cache mechanism in BaseLocale [v10]

2024-03-01 Thread Naoto Sato
> This is stemming from the PR: https://github.com/openjdk/jdk/pull/14211 where 
> aggressive GC can cause NPE in `BaseLocale$Key` class. I refactored the 
> in-house cache with WeakHashMap, and removed the Key class as it is no longer 
> needed (thus the original NPE will no longer be possible). Also with the new 
> JMH test case, it gains some performance improvement:
> 
> (w/o fix)
> 
> Benchmark   Mode  Cnt  Score Error  Units
> LocaleCache.testForLanguageTag  avgt   20   5781.275 ± 569.580  ns/op
> LocaleCache.testLocaleOfavgt   20  62564.079 ± 406.697  ns/op
> 
> (w/ fix)
> Benchmark   Mode  Cnt  Score Error  Units
> LocaleCache.testForLanguageTag  avgt   20   4801.175 ± 371.830  ns/op
> LocaleCache.testLocaleOfavgt   20  60394.652 ± 352.471  ns/op

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Removed the gc test case

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14404/files
  - new: https://git.openjdk.org/jdk/pull/14404/files/7c1ca90e..84f5f692

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14404&range=09
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14404&range=08-09

  Stats: 14 lines in 1 file changed: 0 ins; 14 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/14404.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14404/head:pull/14404

PR: https://git.openjdk.org/jdk/pull/14404


Re: RFR: 8326891: Prefer RPATH over RUNPATH for $ORIGIN rpaths in internal JDK binaries [v2]

2024-03-01 Thread Magnus Ihse Bursie
On Fri, 1 Mar 2024 13:58:08 GMT, Erik Joelsson  wrote:

>> Executables and dynamic libraries on Linux can encode a search path that the 
>> dynamic linker will use when looking up library dependencies, generally 
>> referred to as an "rpath". In the JDK we use this with the $ORIGIN feature 
>> to set search paths relative to the location of the binary itself. Typically 
>> executables in the bin/ directory have the rpath "$ORIGIN/../lib" to find 
>> libjli.so. Most of the libraries in lib/ have rpath set to just "$ORIGIN" to 
>> find each other.
>> 
>> There are two different types of such rpaths, RPATH and RUNPATH. The former 
>> is the earlier incantation but RUNPATH has been around since about 2003 and 
>> has been default in prominent Linux distros for a long time, and now also 
>> seems to be default in the linker directly from binutils. The toolchain used 
>> by Oracle defaulted to RPATH until at least JDK 11, but since then with some 
>> toolchain upgrade, the default was flipped to RUNPATH.
>> 
>> The main (relevant in this case) difference between the two is that RPATH is 
>> considered before the LD_LIBRARY_PATH environment variable, while RUNPATH is 
>> only considered after LD_LIBRARY_PATH. For libraries that are part of a 
>> Linux distribution, letting users, or the system, control and override 
>> builtin rpaths with LD_LIBRARY_PATH seems like a reasonable thing to prefer. 
>> However, for the JDK, there really is no usecase for having an externally 
>> configured LD_LIBRARY_PATH potentially getting in the way of the JDK 
>> libraries finding each other correctly. If a user environment sets 
>> LD_LIBRARY_PATH, and there is a library in that path with the same name as a 
>> JDK library (e.g. libnet.so or some other generically named library) that 
>> external library will be loaded instead of the JDK internal library and that 
>> is basically guaranteed to break the JDK. There is no supported usecase that 
>> I can think of for injecting other versions of such libraries in a JDK 
>> distribution.
>> 
>> I propose that we explicitly configure the JDK build to set RPATH instead of 
>> RUNPATH for Linux binaries. This is done with the linker flag 
>> "--disable-new-dtags".
>
> Erik Joelsson has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use @requires in test

Marked as reviewed by ihse (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/18050#pullrequestreview-1911804757


Re: RFR: 8326915: NPE when a validating parser is restricted [v2]

2024-03-01 Thread Lance Andersen
On Fri, 1 Mar 2024 18:02:24 GMT, Joe Wang  wrote:

>> Fix a NPE when a validating parser is restricted by the JDKCatalog resolve 
>> property. Also slightly improved the error msg with the catalog name.
>> 
>> Test: new test added
>>  existing test CatalogSupport5 would fail without the Null check on 
>> ErrorReporter. It's a separate issue not covered by this fix.
>
> Joe Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   input.substring second argument not needed; proper test method name

Marked as reviewed by lancea (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/18071#pullrequestreview-1911761210


Re: RFR: JDK-8316708: Augment WorstCaseTests with more cases [v9]

2024-03-01 Thread Raffaello Giulietti
On Fri, 1 Mar 2024 18:01:11 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.
>
> Joe Darcy 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 18 additional commits since 
> the last revision:
> 
>  - Respond to review feedback and fix typos.
>  - Merge branch 'master' into JDK-8316708
>  - Respond to review feedback, adjust worst-case bounds.
>  - Appease jcheck.
>  - Update atan2 and hypot tests.
>  - Expand fingerprinting to more StrictMath tests.
>  - Fix missing minus sign.
>  - Checkpoint.
>  - Checkpoint StrictMath test for trig, inverse trig, and hyperbolic trig 
> fingerprint values.
>  - Add cos test cases.
>  - ... and 8 more: https://git.openjdk.org/jdk/compare/673a11b1...e705e10a

Marked as reviewed by rgiulietti (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/15879#pullrequestreview-1911741566


Re: RFR: 8326915: NPE when a validating parser is restricted [v2]

2024-03-01 Thread Joe Wang
> Fix a NPE when a validating parser is restricted by the JDKCatalog resolve 
> property. Also slightly improved the error msg with the catalog name.
> 
> Test: new test added
>  existing test CatalogSupport5 would fail without the Null check on 
> ErrorReporter. It's a separate issue not covered by this fix.

Joe Wang has updated the pull request incrementally with one additional commit 
since the last revision:

  input.substring second argument not needed; proper test method name

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18071/files
  - new: https://git.openjdk.org/jdk/pull/18071/files/6efb3c22..d847cf5d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18071&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18071&range=00-01

  Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/18071.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18071/head:pull/18071

PR: https://git.openjdk.org/jdk/pull/18071


Re: RFR: JDK-8316708: Augment WorstCaseTests with more cases [v9]

2024-03-01 Thread Joe Darcy
> 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.

Joe Darcy 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 18 additional commits since the 
last revision:

 - Respond to review feedback and fix typos.
 - Merge branch 'master' into JDK-8316708
 - Respond to review feedback, adjust worst-case bounds.
 - Appease jcheck.
 - Update atan2 and hypot tests.
 - Expand fingerprinting to more StrictMath tests.
 - Fix missing minus sign.
 - Checkpoint.
 - Checkpoint StrictMath test for trig, inverse trig, and hyperbolic trig 
fingerprint values.
 - Add cos test cases.
 - ... and 8 more: https://git.openjdk.org/jdk/compare/fd67967c...e705e10a

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15879/files
  - new: https://git.openjdk.org/jdk/pull/15879/files/98c8e69f..e705e10a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=15879&range=08
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15879&range=07-08

  Stats: 6391 lines in 800 files changed: 3300 ins; 1492 del; 1599 mod
  Patch: https://git.openjdk.org/jdk/pull/15879.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15879/head:pull/15879

PR: https://git.openjdk.org/jdk/pull/15879


Re: RFR: 8326915: NPE when a validating parser is restricted [v2]

2024-03-01 Thread Joe Wang
On Fri, 1 Mar 2024 00:53:17 GMT, Naoto Sato  wrote:

>> Joe Wang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   input.substring second argument not needed; proper test method name
>
> src/java.xml/share/classes/jdk/xml/internal/SecuritySupport.java line 402:
> 
>> 400: int i = input.lastIndexOf('/');
>> 401: if (i > 0) {
>> 402: return input.substring(i+1, input.length());
> 
> Nit: the second argument is not needed.

Thanks. Fixed. Also replaced the test's method name with a proper name.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18071#discussion_r1509351975


Re: RFR: 8326915: NPE when a validating parser is restricted

2024-03-01 Thread Naoto Sato
On Thu, 29 Feb 2024 21:00:09 GMT, Joe Wang  wrote:

> Fix a NPE when a validating parser is restricted by the JDKCatalog resolve 
> property. Also slightly improved the error msg with the catalog name.
> 
> Test: new test added
>  existing test CatalogSupport5 would fail without the Null check on 
> ErrorReporter. It's a separate issue not covered by this fix.

LGTM

src/java.xml/share/classes/jdk/xml/internal/SecuritySupport.java line 402:

> 400: int i = input.lastIndexOf('/');
> 401: if (i > 0) {
> 402: return input.substring(i+1, input.length());

Nit: the second argument is not needed.

-

Marked as reviewed by naoto (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18071#pullrequestreview-1910127344
PR Review Comment: https://git.openjdk.org/jdk/pull/18071#discussion_r1508361029


Re: RFR: JDK-8316708: Augment WorstCaseTests with more cases [v8]

2024-03-01 Thread Raffaello Giulietti
On Wed, 28 Feb 2024 06:08:18 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.
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Respond to review feedback, adjust worst-case bounds.

test/jdk/java/lang/Math/WorstCaseTests.java line 35:

> 33: 
> 34: /**
> 35:  * This test containst two distinct kinds of worst-case inputs:

Suggestion:

 * This test contains two distinct kinds of worst-case inputs:

test/jdk/java/lang/Math/WorstCaseTests.java line 39:

> 37:  * 1) Exact numerical results that are nearly half-way between
> 38:  * representable numbers or very close to a representable
> 39:  * number. (Half-way caess are hardest for round to nearest even;

Suggestion:

 * number. (Half-way cases are hardest for round to nearest even;

test/jdk/java/lang/Math/WorstCaseTests.java line 43:

> 41:  * roundings.)
> 42:  *
> 43:  * 2) Worst-case errors as observed emprically across different

Suggestion:

 * 2) Worst-case errors as observed empirically across different

test/jdk/java/lang/Math/WorstCaseTests.java line 46:

> 44:  * implementations that are not correctly rounded.
> 45:  *
> 46:  * For the first categpory, the "Table Maker's Dilemma" results from

Suggestion:

 * For the first category, the "Table Maker's Dilemma" results from

test/jdk/java/lang/Math/WorstCaseTests.java line 48:

> 46:  * For the first categpory, the "Table Maker's Dilemma" results from
> 47:  * Jean-Michel Muller and Vincent Lefèvre, are used.
> 48:  * See http://perso.ens-lyon.fr/jean-michel.muller/TMD.html for original

Suggestion:

 * See https://perso.ens-lyon.fr/jean-michel.muller/TMD.html for original

test/jdk/java/lang/Math/WorstCaseTests.java line 50:

> 48:  * See http://perso.ens-lyon.fr/jean-michel.muller/TMD.html for original
> 49:  * test vectors from 2000 and see
> 50:  * http://perso.ens-lyon.fr/jean-michel.muller/TMDworstcases.pdf with

Suggestion:

 * https://perso.ens-lyon.fr/jean-michel.muller/TMDworstcases.pdf with

test/jdk/java/lang/Math/WorstCaseTests.java line 245:

> 243: {-0x1.842d8ec8f752fp+21,-0x1.6ce864edeaffdp-1},
> 244: {-0x1.07e4c92b5349dp+4, +0x1.6a096375ffb23p-1},
> 245: // {-0x1.13a5ccd87c9bbp+1008,  -0x1.27b3964185d8dp-1}, // 
> check -- need +/-

High-precision computations confirm `-0x1.27b3964185d8dp-1`

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15879#discussion_r1509239002
PR Review Comment: https://git.openjdk.org/jdk/pull/15879#discussion_r1509239088
PR Review Comment: https://git.openjdk.org/jdk/pull/15879#discussion_r1509239189
PR Review Comment: https://git.openjdk.org/jdk/pull/15879#discussion_r1509239253
PR Review Comment: https://git.openjdk.org/jdk/pull/15879#discussion_r1509239477
PR Review Comment: https://git.openjdk.org/jdk/pull/15879#discussion_r1509239558
PR Review Comment: https://git.openjdk.org/jdk/pull/15879#discussion_r1509239711


Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]

2024-03-01 Thread Magnus Ihse Bursie
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

Iirc, your assessment is right; the code should be ready for integration; I 
just don't know about the state of testing.

-

PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1973364027


Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v9]

2024-03-01 Thread Hamlin Li
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

Just a quick update, I've rebased the code, and will continue the work soon.

I've looked through the previous discussion, seems there is no blocking issues, 
if I misunderstood or miss some information, please feel free to let me know, 
also if you have any further comment. Thanks!

-

PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1973345855


Re: RFR: JDK-8316708: Augment WorstCaseTests with more cases [v8]

2024-03-01 Thread Raffaello Giulietti
On Wed, 28 Feb 2024 06:08:18 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.
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Respond to review feedback, adjust worst-case bounds.

Tests.java L:111 should read

 * @param d floating-point number whose exponent is to be extracted

-

PR Comment: https://git.openjdk.org/jdk/pull/15879#issuecomment-1973309784


Re: RFR: 8326891: Prefer RPATH over RUNPATH for $ORIGIN rpaths in internal JDK binaries [v2]

2024-03-01 Thread Erik Joelsson
> Executables and dynamic libraries on Linux can encode a search path that the 
> dynamic linker will use when looking up library dependencies, generally 
> referred to as an "rpath". In the JDK we use this with the $ORIGIN feature to 
> set search paths relative to the location of the binary itself. Typically 
> executables in the bin/ directory have the rpath "$ORIGIN/../lib" to find 
> libjli.so. Most of the libraries in lib/ have rpath set to just "$ORIGIN" to 
> find each other.
> 
> There are two different types of such rpaths, RPATH and RUNPATH. The former 
> is the earlier incantation but RUNPATH has been around since about 2003 and 
> has been default in prominent Linux distros for a long time, and now also 
> seems to be default in the linker directly from binutils. The toolchain used 
> by Oracle defaulted to RPATH until at least JDK 11, but since then with some 
> toolchain upgrade, the default was flipped to RUNPATH.
> 
> The main (relevant in this case) difference between the two is that RPATH is 
> considered before the LD_LIBRARY_PATH environment variable, while RUNPATH is 
> only considered after LD_LIBRARY_PATH. For libraries that are part of a Linux 
> distribution, letting users, or the system, control and override builtin 
> rpaths with LD_LIBRARY_PATH seems like a reasonable thing to prefer. However, 
> for the JDK, there really is no usecase for having an externally configured 
> LD_LIBRARY_PATH potentially getting in the way of the JDK libraries finding 
> each other correctly. If a user environment sets LD_LIBRARY_PATH, and there 
> is a library in that path with the same name as a JDK library (e.g. libnet.so 
> or some other generically named library) that external library will be loaded 
> instead of the JDK internal library and that is basically guaranteed to break 
> the JDK. There is no supported usecase that I can think of for injecting 
> other versions of such libraries in a JDK distribution.
> 
> I propose that we explicitly configure the JDK build to set RPATH instead of 
> RUNPATH for Linux binaries. This is done with the linker flag 
> "--disable-new-dtags".

Erik Joelsson has updated the pull request incrementally with one additional 
commit since the last revision:

  Use @requires in test

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18050/files
  - new: https://git.openjdk.org/jdk/pull/18050/files/749a95f9..592281f2

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18050&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18050&range=00-01

  Stats: 9 lines in 1 file changed: 2 ins; 3 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/18050.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18050/head:pull/18050

PR: https://git.openjdk.org/jdk/pull/18050


Re: RFR: 8326891: Prefer RPATH over RUNPATH for $ORIGIN rpaths in internal JDK binaries [v2]

2024-03-01 Thread Erik Joelsson
On Fri, 1 Mar 2024 06:30:14 GMT, David Holmes  wrote:

>> Erik Joelsson has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Use @requires in test
>
> test/jdk/tools/launcher/RunpathTest.java line 27:
> 
>> 25:  * @test
>> 26:  * @bug 7190813 8022719
>> 27:  * @summary Check for extended RPATHs on Linux
> 
> Pre-existing but really the restriction to Linux should be via `@requires` 
> not a runtime test.

Fixed

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18050#discussion_r1509047068


Re: RFR: JDK-8326389: [test] improve assertEquals failure output [v3]

2024-03-01 Thread Matthias Baesken
On Mon, 26 Feb 2024 06:57:49 GMT, Jaikiran Pai  wrote:

> Hello David, the updated text that I proposed to Matthias, of the form 
> "expected: ... but was: ..." was borrowed from what junit5

Unfortunately we get now error messages like this 

java.lang.RuntimeException: VM output should contain exactly one rtm locking 
statistics entry for method compiler.testlibrary.rtm.XAbortProvoker::forceAbort 
expected: 0 but was: 1

It should be ... expected: 1 but was: 0 ; the assertEquals has this interface 
`assertEquals(Object lhs, Object rhs, String msg)`  so we have a left hand 
(lhs) and a right hand side (rhs) of a comparison but was is expected and what 
is what we got is not defined  .

-

PR Comment: https://git.openjdk.org/jdk/pull/17952#issuecomment-1973131407


Integrated: 8319648: java/lang/SecurityManager tests ignore vm flags

2024-03-01 Thread Matthew Donovan
On Thu, 15 Feb 2024 17:06:25 GMT, Matthew Donovan  wrote:

> In this PR I updated the tests to use the newer 
> ProcessTools.createTestJavaProcessBuilder methods to pass VM options to child 
> processes.

This pull request has now been integrated.

Changeset: 437cf354
Author:Matthew Donovan 
URL:   
https://git.openjdk.org/jdk/commit/437cf354e2d1f7df79fa32265ccf86a0e84257b5
Stats: 9 lines in 2 files changed: 1 ins; 3 del; 5 mod

8319648: java/lang/SecurityManager tests ignore vm flags

Reviewed-by: mullan

-

PR: https://git.openjdk.org/jdk/pull/17878


Re: RFR: 8326915: NPE when a validating parser is restricted

2024-03-01 Thread Lance Andersen
On Thu, 29 Feb 2024 21:00:09 GMT, Joe Wang  wrote:

> Fix a NPE when a validating parser is restricted by the JDKCatalog resolve 
> property. Also slightly improved the error msg with the catalog name.
> 
> Test: new test added
>  existing test CatalogSupport5 would fail without the Null check on 
> ErrorReporter. It's a separate issue not covered by this fix.

Marked as reviewed by lancea (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/18071#pullrequestreview-1910945957


Re: RFR: 8325579: Inconsistent behavior in com.sun.jndi.ldap.Connection::createSocket [v7]

2024-03-01 Thread Christoph Langer
> During analysing a customer case I figured out that we have an inconsistency 
> between documentation and actual behavior in class 
> com.sun.jndi.ldap.Connection. The [method documentation of 
> com.sun.jndi.ldap.Connection::createSocket](https://github.com/openjdk/jdk/blob/3ebe6c192a5dd5cc46ae2d263713c9ff38cd46bb/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#L281)
>  states: "If a timeout is supplied but unconnected sockets are not supported 
> then the timeout is ignored and a connected socket is created."
> 
> This, however does not happen. If a SocketFactory would not support 
> unconnected sockets, it would likely throw a SocketException in 
> [SocketFactory::createSocket()](https://github.com/openjdk/jdk/blob/6303c0e7136436a2d3cb6043b88edf788c0067cc/src/java.base/share/classes/javax/net/SocketFactory.java#L123).
>  And since [the 
> code](https://github.com/openjdk/jdk/blob/3ebe6c192a5dd5cc46ae2d263713c9ff38cd46bb/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#L336)
>  does not check for this behavior, a connection with timeout value through a 
> SocketFactory that does not support unconnected sockets would simply fail 
> with an IOException.
> 
> So we should either make the code adhere to what is documented or adapt the 
> documentation to the actual behavior.
> 
> I hereby try to fix the connect coding. Alternatively, we could also adapt 
> the description - I have no strong opinion. What do the experts suggest?

Christoph Langer 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 10 additional commits 
since the last revision:

 - Indentation
 - Merge branch 'master' into JDK-8325579
 - Review feedback
 - Rename back to LdapSSLHandshakeFailureTest to ease reviewing
 - Merge branch 'master' into JDK-8325579
 - Typo
 - Merge branch 'master' into JDK-8325579
 - Rename test and refine comment
 - Enhance test
 - JDK-8325579

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17797/files
  - new: https://git.openjdk.org/jdk/pull/17797/files/ec6579d2..97299970

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=17797&range=06
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17797&range=05-06

  Stats: 17321 lines in 1249 files changed: 9833 ins; 3144 del; 4344 mod
  Patch: https://git.openjdk.org/jdk/pull/17797.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17797/head:pull/17797

PR: https://git.openjdk.org/jdk/pull/17797


Re: RFR: 8326591: New test JmodExcludedFiles.java fails on Windows when --with-external-symbols-in-bundles=public is used

2024-03-01 Thread Matthias Baesken
On Fri, 23 Feb 2024 19:18:46 GMT, Christoph Langer  wrote:

> The new test JmodExcludedFiles.java checks that no debug symbol files are 
> contained in the jmod files. It does not take into account that with 
> configure option --with-external-symbols-in-bundles=public we want to have 
> stripped pdb files, also in jmods, to get native callstacks with line number.
> 
> This modifies the test and checks if equivalent *stripped.pdb files exist 
> when .pdb files are encountered inside jmods. In that case one can assume 
> that --with-external-symbols-in-bundles=public was chosen as configure option.

Marked as reviewed by mbaesken (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/17990#pullrequestreview-1910541971


Withdrawn: 8325567: jspawnhelper without args fails with segfault

2024-03-01 Thread Vladimir Petko
On Fri, 1 Mar 2024 01:50:46 GMT, Vladimir Petko  wrote:

> This MR fixes segsegv in jspawnhelper when it is called without args. 
> This scenario happens when a long running Java process is not restarted 
> during upgrade. 
> 
> It updates test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java to 
> check that jspawnhelper exits with code 1:
> 
> After test update:
> 
> $ make CONF=linux-x86_64-server-fastdebug test 
> TEST=test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
> ...
> Running jspawnhelper without args
> STDERR:
> java.lang.Exception: Parent process exited with 12
> at 
> JspawnhelperProtocol.simulateJspawnhelperWithoutArgs(JspawnhelperProtocol.java:126)
> at JspawnhelperProtocol.main(JspawnhelperProtocol.java:267)
> at 
> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
> at java.base/java.lang.reflect.Method.invoke(Method.java:580)
> at 
> com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
> at java.base/java.lang.Thread.run(Thread.java:1575)
> ...
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
>>>   1 0 1 0 <<
> ==
> TEST FAILURE
> 
> After jspawnhelper change the test passes:
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
>  1 1 0 0  
>  
> ==
> TEST SUCCESS
> 
> 
> The user will see the following output in the logs:
> 
> An earlier version of Java is trying to call jspawnhelper.
> Please restart Java process.
> Exception in thread "main" java.io.IOException: Cannot run program "ls": 
> error=0, Failed to exec spawn helper: pid: 2168121, exit value: 1
> at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1143)
> at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1073)
> at Test.main(Test.java:3)
> Caused by: java.io.IOException: error=0, Failed to exec spawn helper: pid: 
> 2168121, exit value: 1
> at java.base/java.lang.ProcessImpl.forkAndExec(Native Method)
> at java.base/java.lang.ProcessImpl.(ProcessImpl.java:314)
> at java.base/java.lang.ProcessIm...

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.org/jdk/pull/18074