RFR: 8315558: Undocumented exceptions in java.text.StringCharacterIterator

[Justin Lu]

Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8315558) 
which is a conformance change to document some exceptions in the specification 
of java.text.StringCharacterIterator.

-

Commit messages:
 - copyright year
 - init

Changes: https://git.openjdk.org/jdk/pull/15547/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=15547=00
  Issue: https://bugs.openjdk.org/browse/JDK-8315558
  Stats: 26 lines in 1 file changed: 14 ins; 6 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/15547.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15547/head:pull/15547

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

Re: RFR: 5066247: Refine the spec of equals() and hashCode() for j.text.Format classes [v3]

[Naoto Sato]

On Fri, 1 Sep 2023 22:26:00 GMT, Justin Lu  wrote:

>> Please review this PR which refines the spec of `equals()` and `hashCode()` 
>> in `java.text.Format` related classes.
>> 
>> The current spec for most of these methods is either  "_Overrides 
>> _" or are incomplete/wrong (i.e. see `ChoiceFormat`).
>> 
>> This fix adjusts the spec to provide a consistent definition for the 
>> overridden methods and specify what is being compared/used to generate a 
>> hash code value.
>> 
>> For implementations that use at most a few fields, the values are stated, 
>> otherwise a more general term is used as a substitution (i.e. see 
>> `DecimalFormat`).
>
> Justin Lu has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   instanceof should not be camel case

Marked as reviewed by naoto (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/15459#pullrequestreview-1607727326

Re: RFR: 5066247: Refine the spec of equals() and hashCode() for j.text.Format classes [v3]

[Justin Lu]

> Please review this PR which refines the spec of `equals()` and `hashCode()` 
> in `java.text.Format` related classes.
> 
> The current spec for most of these methods is either  "_Overrides 
> _" or are incomplete/wrong (i.e. see `ChoiceFormat`).
> 
> This fix adjusts the spec to provide a consistent definition for the 
> overridden methods and specify what is being compared/used to generate a hash 
> code value.
> 
> For implementations that use at most a few fields, the values are stated, 
> otherwise a more general term is used as a substitution (i.e. see 
> `DecimalFormat`).

Justin Lu has updated the pull request incrementally with one additional commit 
since the last revision:

  instanceof should not be camel case

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15459/files
  - new: https://git.openjdk.org/jdk/pull/15459/files/3e2d6745..e322412c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=15459=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=15459=01-02

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

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

Re: RFR: 6228794: java.text.ChoiceFormat pattern behavior is not well documented. [v2]

[Justin Lu]

> Please review this PR and associated 
> [CSR](https://bugs.openjdk.org/browse/JDK-8314546) which expands on the 
> `java.text.ChoiceFormat` specification regarding its pattern.
> 
> `j.text.ChoiceFormat` provides an example pattern in the class description, 
> but beyond that it does not specify any well-defined syntax for creating a 
> pattern. In addition, methods related to the pattern String are 
> under-specified.
> 
> The wording for `getLimits()` and `getFormats()` was also adjusted, as there 
> are other ways to set the limits and formats beyond the constructor.
> 
> The pattern syntax may be easier to view -> 
> https://cr.openjdk.org/~jlu/api/java.base/java/text/ChoiceFormat.html

Justin Lu has updated the pull request with a new target base due to a merge or 
a rebase. The pull request now contains ten commits:

 - merge master and resolve conflicts
 - Include Unicode sequence in pattern
 - Drop throws tag, that change can go to JDK8314925
 - Clarify ≤
 - Adjust pattern definition
 - Remove leftover comments
 - Revert the example changes to focus on the more important changes
 - Adjust wording of getter methods
 - Init

-

Changes: https://git.openjdk.org/jdk/pull/15392/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=15392=01
  Stats: 64 lines in 1 file changed: 42 ins; 7 del; 15 mod
  Patch: https://git.openjdk.org/jdk/pull/15392.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15392/head:pull/15392

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

Re: RFR: 5066247: Refine the spec of equals() and hashCode() for j.text.Format classes [v2]

[Naoto Sato]

On Fri, 1 Sep 2023 19:56:53 GMT, Justin Lu  wrote:

>> Please review this PR which refines the spec of `equals()` and `hashCode()` 
>> in `java.text.Format` related classes.
>> 
>> The current spec for most of these methods is either  "_Overrides 
>> _" or are incomplete/wrong (i.e. see `ChoiceFormat`).
>> 
>> This fix adjusts the spec to provide a consistent definition for the 
>> overridden methods and specify what is being compared/used to generate a 
>> hash code value.
>> 
>> For implementations that use at most a few fields, the values are stated, 
>> otherwise a more general term is used as a substitution (i.e. see 
>> `DecimalFormat`).
>
> Justin Lu has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Reflect review comments: Make implementers aware of default classIdentity 
> notion, generalize all hashcodes/equals, adjust hashCode spec to be more 
> accurate

src/java.base/share/classes/java/text/ChoiceFormat.java line 521:

> 519:  *
> 520:  * @implNote Implementers should be aware that the default 
> implementation does an
> 521:  * equality check using {@code getClass} rather than {@code 
> instanceOf} when

Since `instanceof` is a Java keyword, I'd expect it not to be camel-cased.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15459#discussion_r1313583508

Re: RFR: 8314604: j.text.DecimalFormat behavior regarding patterns is not clear [v3]

[Naoto Sato]

On Fri, 1 Sep 2023 19:56:28 GMT, Justin Lu  wrote:

>> Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8314607) 
>> which clarifies the behavior of patterns  in regards to the max integer 
>> digits in j.text.DecimalFormat.
>> 
>> The current specification (of `applyPattern`) states that patterns do not 
>> set the value of max integer digits. This is incorrect, these 
>> methods/constructors do set a value for the max integer digits. If the 
>> pattern is in scientific notation, the max integer digits value is derived 
>> from the pattern. Otherwise, the pattern is ignored, and the limit is set to 
>> `Integer.MAX_VALUE`.
>> 
>> See below,
>> 
>> DecimalFormat df = new DecimalFormat();
>> df.applyPattern("000.000E0");
>> df.getMaximumIntegerDigits(); //  ==> 3
>> df.applyPattern("000.000");
>> df.getMaximumIntegerDigits(); // ==> 2147483647
>> 
>> DecimalFormat df = new DecimalFormat("000.000");
>> df.getMaximumIntegerDigits(); // ==> 2147483647
>> DecimalFormat df = new DecimalFormat("000.000E0");
>> df.getMaximumIntegerDigits(); // ==> 3
>> 
>> 
>> Method descriptions should be fixed, and the relevant constructors need to 
>> mention the behavior as well.
>
> 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 six additional commits since 
> the last revision:
> 
>  - Reflect CSR comment, use anchor and reference
>  - Merge branch 'master' into JDK-8314604
>  - Reflect review comments: Non sci notation first. Link to Scientific 
> Notation section
>  - Include constructors
>  - Re-clarify spec
>  - Init

Update looks good

-

Marked as reviewed by naoto (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15349#pullrequestreview-1607712190

Re: RFR: 8311220: Optimization for StringLatin UpperLower [v3]

[温绍锦]

On Thu, 31 Aug 2023 11:45:31 GMT, Claes Redestad  wrote:

> I think this overall looks reasonable, but I think a more thorough proof / 
> test would help to build confidence that all these changes are semantically 
> neutral.
> 
> The `isLowerCaseEx` needs to explain why two lower-case codepoints are 
> omitted (perhaps this should be `hasUpperCase`?)


String str1 = new String(new byte[]{(byte) 0xb5}, StandardCharsets.ISO_8859_1);
String str2 = new String(new byte[]{(byte) 0xdf}, StandardCharsets.ISO_8859_1);
System.out.println(str1 + "\t" + str1.toUpperCase());
System.out.println(str2 + "\t" + str2.toUpperCase());


result : 

µ   Μ
ß   SS


0xb5 and 0xdf call toUpperCaseEx return and input are different, these two 
codepoints are to ensure that the behavior has not changed.

-

PR Comment: https://git.openjdk.org/jdk/pull/14751#issuecomment-1703265488

Re: RFR: 5066247: Refine the spec of equals() and hashCode() for j.text.Format classes [v2]

[Justin Lu]

> Please review this PR which refines the spec of `equals()` and `hashCode()` 
> in `java.text.Format` related classes.
> 
> The current spec for most of these methods is either  "_Overrides 
> _" or are incomplete/wrong (i.e. see `ChoiceFormat`).
> 
> This fix adjusts the spec to provide a consistent definition for the 
> overridden methods and specify what is being compared/used to generate a hash 
> code value.
> 
> For implementations that use at most a few fields, the values are stated, 
> otherwise a more general term is used as a substitution (i.e. see 
> `DecimalFormat`).

Justin Lu has updated the pull request incrementally with one additional commit 
since the last revision:

  Reflect review comments: Make implementers aware of default classIdentity 
notion, generalize all hashcodes/equals, adjust hashCode spec to be more 
accurate

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15459/files
  - new: https://git.openjdk.org/jdk/pull/15459/files/84bd4708..3e2d6745

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=15459=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=15459=00-01

  Stats: 58 lines in 9 files changed: 29 ins; 4 del; 25 mod
  Patch: https://git.openjdk.org/jdk/pull/15459.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15459/head:pull/15459

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

Re: RFR: 8314604: j.text.DecimalFormat behavior regarding patterns is not clear [v3]

[Justin Lu]

> Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8314607) 
> which clarifies the behavior of patterns  in regards to the max integer 
> digits in j.text.DecimalFormat.
> 
> The current specification (of `applyPattern`) states that patterns do not set 
> the value of max integer digits. This is incorrect, these 
> methods/constructors do set a value for the max integer digits. If the 
> pattern is in scientific notation, the max integer digits value is derived 
> from the pattern. Otherwise, the pattern is ignored, and the limit is set to 
> `Integer.MAX_VALUE`.
> 
> See below,
> 
> DecimalFormat df = new DecimalFormat();
> df.applyPattern("000.000E0");
> df.getMaximumIntegerDigits(); //  ==> 3
> df.applyPattern("000.000");
> df.getMaximumIntegerDigits(); // ==> 2147483647
> 
> DecimalFormat df = new DecimalFormat("000.000");
> df.getMaximumIntegerDigits(); // ==> 2147483647
> DecimalFormat df = new DecimalFormat("000.000E0");
> df.getMaximumIntegerDigits(); // ==> 3
> 
> 
> Method descriptions should be fixed, and the relevant constructors need to 
> mention the behavior as well.

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 six additional commits since the 
last revision:

 - Reflect CSR comment, use anchor and reference
 - Merge branch 'master' into JDK-8314604
 - Reflect review comments: Non sci notation first. Link to Scientific Notation 
section
 - Include constructors
 - Re-clarify spec
 - Init

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15349/files
  - new: https://git.openjdk.org/jdk/pull/15349/files/93b0053e..258ce6d8

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=15349=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=15349=01-02

  Stats: 16482 lines in 501 files changed: 11217 ins; 2724 del; 2541 mod
  Patch: https://git.openjdk.org/jdk/pull/15349.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15349/head:pull/15349

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

Re: RFR: 8311220: Optimization for StringLatin UpperLower [v3]

[温绍锦]

On Thu, 31 Aug 2023 11:39:57 GMT, Claes Redestad  wrote:

>> 温绍锦 has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   add method CharacterDataLatin1#isLowerCaseEx
>
> src/java.base/share/classes/java/lang/CharacterDataLatin1.java.template line 
> 94:
> 
>> 92: 
>> 93: boolean isLowerCaseEx(int ch) {
>> 94: return ch >= 'a' && (ch <= 'z' || ch == 181 || (ch >= 223 && ch 
>> != 247));
> 
> What is the contract for this? Specifically there are two special 
> superscripte codepoints (170 and 186) which are lower-case 
> (`Character.isLowerCase(170) => true`) but doesn't have an upper-case 
> (`Character.toUpperCase(170) => 170`). It seems reasonable to exclude them if 
> only used for operations like toUpper/toLower (since they won't change), but 
> it should be spelled out to avoid surprises.
> 
> For consistency I think we should use hex literals in this file, e.g. `0xDF` 
> instead of `223`

The current implementation of the isLowerCaseEx method and the previous 
implementation "cp != CharacterDataLatin1.instance.toUpperCaseEx(cp)"
The result is exactly the same.

The code below compares all numbers in [-128, 128]


import sun.misc.Unsafe;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.reflect.Field;
import java.nio.charset.StandardCharsets;

import static com.alibaba.fastjson2.util.JDKUtils.UNSAFE;

public class CharacterDataLatin1Test {
public static void main(String[] args) throws Throwable {
for (int i = Byte.MIN_VALUE; i <= Byte.MAX_VALUE; ++i) {
byte b = (byte) i;
int cp = b & 0xff;

boolean r0 = cp != toUpperCaseEx(cp);
boolean r1 = isLowerCaseEx(cp);
if (r0) {
System.out.println(cp + "\t0x" + Integer.toHexString(cp)
+ "\t" + new String(new byte[] {b}, 
StandardCharsets.ISO_8859_1));
}

if (r0 != r1) {
System.out.println("error " + i);
}
}
}

static boolean isLowerCaseEx(int ch) {
return ch >= 'a' && (ch <= 'z' || ch == 0xb5 || (ch >= 0xdf && ch != 
0xf7));
}

static int toUpperCaseEx(int cp) throws Throwable {
Field theUnsafeField = Unsafe.class.getDeclaredField("theUnsafe");
theUnsafeField.setAccessible(true);
Unsafe unsafe = (Unsafe) theUnsafeField.get(null);

Class charbinClass = Class.forName("java.lang.CharacterDataLatin1");
Field field = charbinClass.getDeclaredField("instance");
long fieldOffset = unsafe.staticFieldOffset(field);
Object instance = unsafe.getObject(charbinClass, fieldOffset);

Class lookupClass = MethodHandles.Lookup.class;
Field implLookup = lookupClass.getDeclaredField("IMPL_LOOKUP");
MethodHandles.Lookup trustedLookup = (MethodHandles.Lookup) 
unsafe.getObject(lookupClass,
UNSAFE.staticFieldOffset(implLookup));

MethodHandles.lookup();
MethodHandle toLowerCase = trustedLookup
.findVirtual(charbinClass, "toUpperCaseEx", 
MethodType.methodType(int.class, int.class));

return (Integer) toLowerCase.invoke(instance, cp);
}
}

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14751#discussion_r1313453795

Re: RFR: 8311220: Optimization for StringLatin UpperLower [v4]

[温绍锦]

> # Benchmark Result
> 
> 
> sh make/devkit/createJMHBundle.sh
> bash configure --with-jmh=build/jmh/jars
> make test TEST="micro:java.lang.StringUpperLower.*"
> 
> 
> 
> ## 1. 
> [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i)
> * cpu : intel xeon sapphire rapids (x64)
> 
> ``` diff
> -Benchmark  Mode  Cnt   Score   Error  Units (baseline)
> -StringUpperLower.lowerToLower  avgt   15  27.180 ± 0.017  ns/op
> -StringUpperLower.lowerToUpper  avgt   15  47.196 ± 0.066  ns/op
> -StringUpperLower.mixedToLower  avgt   15  32.307 ± 0.072  ns/op
> -StringUpperLower.mixedToUpper  avgt   15  44.005 ± 0.414  ns/op
> -StringUpperLower.upperToLower  avgt   15  32.310 ± 0.033  ns/op
> -StringUpperLower.upperToUpper  avgt   15  42.053 ± 0.341  ns/op
> 
> +Benchmark  Mode  Cnt   Score   Error  Units (Update 01)
> +StringUpperLower.lowerToLower  avgt   15  16.964 ± 0.021  ns/op (+60.96%)
> +StringUpperLower.lowerToUpper  avgt   15  46.491 ± 0.036  ns/op (+1.51%)
> +StringUpperLower.mixedToLower  avgt   15  35.947 ± 0.254  ns/op (-10.12%)
> +StringUpperLower.mixedToUpper  avgt   15  41.976 ± 0.326  ns/op (+4.83%)
> +StringUpperLower.upperToLower  avgt   15  33.466 ± 4.036  ns/op (-3.45%)
> +StringUpperLower.upperToUpper  avgt   15  17.446 ± 1.036  ns/op (+141.04%)
> 
> +Benchmark  Mode  Cnt   Score   Error  Units (Update 00)
> +StringUpperLower.lowerToLower  avgt   15  16.976 ± 0.043  ns/op (+60.160)
> +StringUpperLower.lowerToUpper  avgt   15  46.373 ± 0.086  ns/op (+1.77%)
> +StringUpperLower.mixedToLower  avgt   15  32.018 ± 0.061  ns/op (+0.9%)
> +StringUpperLower.mixedToUpper  avgt   15  42.019 ± 0.473  ns/op (+4.72%)
> +StringUpperLower.upperToLower  avgt   15  32.052 ± 0.051  ns/op (+0.8%)
> +StringUpperLower.upperToUpper  avgt   15  16.978 ± 0.190  ns/op (+147.69%)
> 
> 
> ## 2. 
> [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a)
> * cpu : amd epc genoa (x64)
> 
> ``` diff
> -Benchmark  Mode  Cnt   Score   Error  Units (baseline)
> -StringUpperLower.lowerToLower  avgt   15  22.164 ± 0.021  ns/op
> -StringUpperLower.lowerToUpper  avgt   15  46.113 ± 0.047  ns/op
> -StringUpperLower.mixedToLower  avgt   15  28.501 ± 0.037  ns/op
> -StringUpperLower.mixedToUpper  avgt   15  38.782 ± 0.038  ns/op
> -StringUpperLower.upperToLower  avgt   15  28.625 ± 0.162  ns/op
> -StringUpperLower.upperToUpper  avgt   15  27.960 ± 0.038  ns/op
> 
> +Benchmark  Mode  Cnt   Score   Error  Units (Update 01)
> +StringUpperLower.lowerToLo...

温绍锦 has updated the pull request incrementally with one additional commit since 
the last revision:

  use hex literal

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14751/files
  - new: https://git.openjdk.org/jdk/pull/14751/files/bc03c99a..a5ace579

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14751=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=14751=02-03

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

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

Re: RFR: 8310929: Optimization for Integer.toString [v17]

[温绍锦]

> Optimization for:
> Integer.toString
> Long.toString
> StringBuilder#append(int)
> 
> # Benchmark Result
> 
> 
> sh make/devkit/createJMHBundle.sh
> bash configure --with-jmh=build/jmh/jars
> make test TEST="micro:java.lang.Integers.toString*" 
> make test TEST="micro:java.lang.Longs.toString*" 
> make test TEST="micro:java.lang.StringBuilders.toStringCharWithInt8"
> 
> 
> ## 1. 
> [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i)
> * cpu : intel xeon sapphire rapids (x64)
> 
> ``` diff
> -Benchmark   (size)  Mode  Cnt  Score   Error  Units (baseline)
> -Integers.toStringBig   500  avgt   15  6.825 ± 0.023  us/op
> -Integers.toStringSmall 500  avgt   15  4.823 ± 0.023  us/op
> -Integers.toStringTiny  500  avgt   15  3.878 ± 0.101  us/op
> 
> +Benchmark   (size)  Mode  Cnt  Score   Error  Units (PR Update 
> 04 f4aa1989)
> +Integers.toStringBig   500  avgt   15  6.002 ± 0.054  us/op (+13.71%)
> +Integers.toStringSmall 500  avgt   15  4.025 ± 0.020  us/op (+19.82%)
> +Integers.toStringTiny  500  avgt   15  3.874 ± 0.067  us/op (+0.10%)
> 
> -Benchmark(size)  Mode  Cnt  Score   Error  Units (baseline)
> -Longs.toStringBig   500  avgt   15  9.224 ± 0.021  us/op
> -Longs.toStringSmall 500  avgt   15  4.621 ± 0.087  us/op
> 
> +Benchmark(size)  Mode  Cnt  Score   Error  Units (PR Update 04 
> f4aa1989)
> +Longs.toStringBig   500  avgt   15  7.483 ± 0.018  us/op (+23.26%)
> +Longs.toStringSmall 500  avgt   15  4.020 ± 0.016  us/op (+14.95%)
> 
> -Benchmark   Mode  Cnt ScoreError  Units 
> (baseline)
> -StringBuilders.toStringCharWithInt8 avgt   1589.327 ±  0.733  ns/op
> 
> +BenchmarkMode  Cnt   Score   Error  Units (PR 
> Update 04 f4aa1989)
> +StringBuilders.toStringCharWithInt8  avgt   15  36.639 ± 0.422  ns/op 
> (+143.80%)
> 
> 
> 
> ## 2. 
> [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a)
> * cpu : amd epc genoa (x64)
> 
> ``` diff
> -Benchmark   (size)  Mode  Cnt  Score   Error  Units (baseline)
> -Integers.toStringBig   500  avgt   15  6.753 ± 0.007  us/op
> -Integers.toStringSmall 500  avgt   15  4.470 ± 0.005  us/op
> -Integers.toStringTiny  500  avgt   15  2.764 ± 0.020  us/op
> 
> +Benchmark   (size)  Mode  Cnt  Score   Error  Units (PR Update 
> 04 f4aa1989)
> +Integers.toStringBig   500  avgt   15  5.036 ± 0.005  us/op (+34.09%)
> +Integers.toStringSmall 500  avgt   15  3.491 ± 0.025  us/op (+28.04%)
> +Integers.toStringTiny  5...

温绍锦 has updated the pull request with a new target base due to a merge or a 
rebase. The pull request now contains 16 commits:

 - remove unused import
 - UTF16 & Latin1 use same lookup table
 - assert bounds check
 - Integer/Long toString test against compact strings
   
   Co-authored-by: liach 
 - Adapt endian in pack
   
   Co-authored-by: liach 
 - use upper case for static final field
 - private static final Unsafe
 - code format
 - simplify code
 - format code & comment
 - ... and 6 more: https://git.openjdk.org/jdk/compare/2f7c65ec...0d149fa9

-

Changes: https://git.openjdk.org/jdk/pull/14699/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=14699=16
  Stats: 175 lines in 5 files changed: 107 ins; 20 del; 48 mod
  Patch: https://git.openjdk.org/jdk/pull/14699.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14699/head:pull/14699

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

Re: RFR: 8310929: Optimization for Integer.toString [v16]

[温绍锦]

> Optimization for:
> Integer.toString
> Long.toString
> StringBuilder#append(int)
> 
> # Benchmark Result
> 
> 
> sh make/devkit/createJMHBundle.sh
> bash configure --with-jmh=build/jmh/jars
> make test TEST="micro:java.lang.Integers.toString*" 
> make test TEST="micro:java.lang.Longs.toString*" 
> make test TEST="micro:java.lang.StringBuilders.toStringCharWithInt8"
> 
> 
> ## 1. 
> [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i)
> * cpu : intel xeon sapphire rapids (x64)
> 
> ``` diff
> -Benchmark   (size)  Mode  Cnt  Score   Error  Units (baseline)
> -Integers.toStringBig   500  avgt   15  6.825 ± 0.023  us/op
> -Integers.toStringSmall 500  avgt   15  4.823 ± 0.023  us/op
> -Integers.toStringTiny  500  avgt   15  3.878 ± 0.101  us/op
> 
> +Benchmark   (size)  Mode  Cnt  Score   Error  Units (PR Update 
> 04 f4aa1989)
> +Integers.toStringBig   500  avgt   15  6.002 ± 0.054  us/op (+13.71%)
> +Integers.toStringSmall 500  avgt   15  4.025 ± 0.020  us/op (+19.82%)
> +Integers.toStringTiny  500  avgt   15  3.874 ± 0.067  us/op (+0.10%)
> 
> -Benchmark(size)  Mode  Cnt  Score   Error  Units (baseline)
> -Longs.toStringBig   500  avgt   15  9.224 ± 0.021  us/op
> -Longs.toStringSmall 500  avgt   15  4.621 ± 0.087  us/op
> 
> +Benchmark(size)  Mode  Cnt  Score   Error  Units (PR Update 04 
> f4aa1989)
> +Longs.toStringBig   500  avgt   15  7.483 ± 0.018  us/op (+23.26%)
> +Longs.toStringSmall 500  avgt   15  4.020 ± 0.016  us/op (+14.95%)
> 
> -Benchmark   Mode  Cnt ScoreError  Units 
> (baseline)
> -StringBuilders.toStringCharWithInt8 avgt   1589.327 ±  0.733  ns/op
> 
> +BenchmarkMode  Cnt   Score   Error  Units (PR 
> Update 04 f4aa1989)
> +StringBuilders.toStringCharWithInt8  avgt   15  36.639 ± 0.422  ns/op 
> (+143.80%)
> 
> 
> 
> ## 2. 
> [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a)
> * cpu : amd epc genoa (x64)
> 
> ``` diff
> -Benchmark   (size)  Mode  Cnt  Score   Error  Units (baseline)
> -Integers.toStringBig   500  avgt   15  6.753 ± 0.007  us/op
> -Integers.toStringSmall 500  avgt   15  4.470 ± 0.005  us/op
> -Integers.toStringTiny  500  avgt   15  2.764 ± 0.020  us/op
> 
> +Benchmark   (size)  Mode  Cnt  Score   Error  Units (PR Update 
> 04 f4aa1989)
> +Integers.toStringBig   500  avgt   15  5.036 ± 0.005  us/op (+34.09%)
> +Integers.toStringSmall 500  avgt   15  3.491 ± 0.025  us/op (+28.04%)
> +Integers.toStringTiny  5...

温绍锦 has updated the pull request incrementally with 736 additional commits 
since the last revision:

 - 8303427: Fixpath confused if unix root contains "/jdk"
   
   Reviewed-by: mikael
 - 8314319: LogCompilation doesn't reset lateInlining when it encounters a 
failure.
   
   Reviewed-by: ecaspole, kvn
 - 8258970: Disabled JPasswordField foreground color is wrong with GTK LAF
   
   Reviewed-by: tr, dnguyen, psadhukhan
 - 8315195: RISC-V: Update hwprobe query for new extensions
   
   Reviewed-by: fyang, fjiang, luhenry
 - 8315446: G1: Remove unused G1AllocRegion::attempt_allocation
   
   Reviewed-by: iwalulya, tschatzl
 - 8315098: Improve URLEncodeDecode microbenchmark
   
   Reviewed-by: ecaspole, dfuchs
 - 8315321: [aix] os::attempt_reserve_memory_at must map at the requested 
address or fail
   
   Reviewed-by: mdoerr
 - 8315436: HttpsServer does not send TLS alerts
   
   Reviewed-by: dfuchs, michaelm
 - 8315069: Relativize extended_sp in interpreter frames
   
   Reviewed-by: haosun, aph, fyang
 - 8313983: jmod create --target-platform should replace existing ModuleTarget 
attribute
   
   Reviewed-by: alanb, mchung
 - ... and 726 more: https://git.openjdk.org/jdk/compare/cf2f8395...a635f903

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14699/files
  - new: https://git.openjdk.org/jdk/pull/14699/files/cf2f8395..a635f903

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14699=15
 - incr: https://webrevs.openjdk.org/?repo=jdk=14699=14-15

  Stats: 156684 lines in 3194 files changed: 65663 ins; 71956 del; 19065 mod
  Patch: https://git.openjdk.org/jdk/pull/14699.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14699/head:pull/14699

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

Re: RFR: 8315004: Runtime.halt() debug logging

[Roger Riggs]

On Fri, 1 Sep 2023 08:29:41 GMT, Alan Bateman  wrote:

>> I think you may have missed the comment in the JBS issue. Logging means 
>> running potentially arbitrary code, doing this at Runtime.halt time is 
>> problematic. I thought the conclusion from the work on Runtime.exit was not 
>> to log in Runtime.halt?
>
>> @AlanBateman Sorry for missing your comment on JBS. I can't find any 
>> discussion of the need for logs in Runtime.halt in JDK-8301627, so I'm not 
>> sure if it was intentional that no logging output was added to Runtime.halt.
>> However, if Runtime.halt is overlooked without discussion, I think it should 
>> be added after considering the need.
>> I think it's the same problem as Runtime.exit when it comes to executing 
>> arbitrary code.
>> Are there any issues specific to Runtime.halt?
> 
> The purpose of the halt method is to terminate immediately, it doesn't 
> initiate the shutdown sequence. My recollection of the System.exit logging is 
> that it was deliberate to not change the halt method. Changing halt to log 
> means it would not terminate immediately. Also I think there were concerns 
> with logging libraries that needed the shutdown hook to execute in order to 
> fush/close logs. @RogerRiggs or @stuart-marks may be able to say more about 
> this.

As @AlanBateman said, the purpose of halt is to immediately exit.
If there are concerns about use of System.halt() a static scan of the class 
files will identify its use.

-

PR Comment: https://git.openjdk.org/jdk/pull/15426#issuecomment-1703133971

Re: RFR: 8315383: jlink SystemModulesPlugin incorrectly parses the options [v2]

[Mandy Chung]

On Wed, 30 Aug 2023 23:39:31 GMT, Oliver Kopp  wrote:

>> Fixes https://bugs.openjdk.org/browse/JDK-8315383
>
> Oliver Kopp has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove obsolete imports

sure, I can help.

-

PR Comment: https://git.openjdk.org/jdk/pull/15495#issuecomment-1703046225

Re: RFR: 8315444: Convert test/jdk/tools to Classfile API

[Adam Sotona]

On Fri, 1 Sep 2023 08:14:26 GMT, Qing Xiao  wrote:

> /test/jdk/tools/lib/tests/JImageValidator.java, tests in 
> /test/jdk/tools/jlink and /test/jdk/tools/jimage use on 
> com.sun.tools.classfile and should be converted to Classfile API.

It looks good, thanks.

-

PR Comment: https://git.openjdk.org/jdk/pull/15529#issuecomment-1702761805

RFR: 8315444: Convert test/jdk/tools to Classfile API

[Qing Xiao]

/test/jdk/tools/lib/tests/JImageValidator.java, tests in /test/jdk/tools/jlink 
and /test/jdk/tools/jimage use on com.sun.tools.classfile and should be 
converted to Classfile API.

-

Commit messages:
 - migrate /test/jdk/tools/lib/tests/JImageValidator.java and tests in 
/test/jdk/tools/jlink and /test/jdk/tools/jimage use Classfile API

Changes: https://git.openjdk.org/jdk/pull/15529/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=15529=00
  Issue: https://bugs.openjdk.org/browse/JDK-8315444
  Stats: 168 lines in 23 files changed: 111 ins; 15 del; 42 mod
  Patch: https://git.openjdk.org/jdk/pull/15529.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15529/head:pull/15529

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

RFR: 8315487: Security Providers Filter

[Martin Balao]

In addition to the goals, scope, motivation, specification and requirement 
notes in [JDK-8315487](https://bugs.openjdk.org/browse/JDK-8315487), we would 
like to describe the most relevant decisions taken during the implementation of 
this enhancement. These notes are organized by feature, may encompass more than 
one file or code segment, and are aimed to provide a high-level view of this PR.

## ProvidersFilter

### Filter construction (parser)

The providers filter is constructed from a string value, taken from either a 
system or a security property with name "jdk.security.providers.filter". This 
process occurs at sun.security.jca.ProvidersFilter class —simply referred as 
ProvidersFilter onward— static initialization. Thus, changes to the filter's 
overridable property are not effective afterwards and no assumptions should be 
made regarding when this class gets initialized.

The filter's string value is processed with a custom parser of order 'n', being 
'n' the number of characters. The parser, represented by the 
ProvidersFilter.Parser class, can be characterized as a Deterministic Finite 
Automaton (DFA). The ProvidersFilter.Parser::parse method is the starting point 
to get characters from the filter's string value and generate state transitions 
in the parser's internal state-machine. See ProvidersFilter.Parser::nextState 
for more details about the parser's states and both valid and invalid 
transitions. The ParsingState enum defines valid parser states and Transition 
the reasons to move between states. If a filter string cannot be parsed, a 
ProvidersFilter.ParserException exception is thrown, and turned into an 
unchecked IllegalArgumentException in the ProvidersFilter.Filter constructor.

While we analyzed —and even tried, at early stages of the development— the use 
of regular expressions for filter parsing, we discarded the approach in order 
to get maximum performance, support a more advanced syntax and have flexibility 
for further extensions in the future.

### Filter (structure and behavior)

A filter is represented by the ProvidersFilter.Filter class. It consists of an 
ordered list of rules, returned by the parser, that represents filter patterns 
from left to right (see the filter syntax for reference). At the end of this 
list, a match-all and deny rule is added for default behavior. When a service 
is evaluated against the filter, each filter rule is checked in the 
ProvidersFilter.Filter::apply method. The rule makes an allow or deny decision 
if the service matches it. Otherwise, the filter moves to the next rule in the 
sequence.

Rules are made of 3 regular expressions, derived from a filter pattern: 
provider, service type and algorithm or alias. A service matches a rule when 
its provider, service type and algorithm or alias matches the corresponding 
regular expressions in the rule. When a rule is matched by a service, it casts 
a decision (represented by the ProvidersFilter::FilterDecision class) that has 
two values: an allow or deny result and a priority that depends on how early 
(or left, in filter string terms) the rule is positioned in relative terms. 
Priorities are used for services that have aliases, as a mechanism to 
disambiguate contradictory decision results depending on which alias or 
algorithm is evaluated.

When a service with aliases is passed through a filter, independent evaluations 
are made for the algorithm and each alias. The decision with highest priority 
(lowest in absolute numbers) is finally effective.

### Filter decisions cache

To accomplish the goal of maximizing performance, most services are passed 
through the Providers filter at registration time, when added with the 
java.security.Provider::putService or java.security.Provider::put APIs. While 
uncommon, providers may override java.security.Provider::getService or 
java.security.Provider::getServices APIs and return services that were never 
registered. In these cases, the service is evaluated against the Providers 
filter the first time used.

Once a service is evaluated against the filter, the decision is stored in the 
private isAllowed Provider.Service class field. When authorizing further uses 
of the service, the value from this cache is read, instead of performing a new 
filter evaluation. If the service does not experience any change, such as 
gaining or losing an alias (only possible with the legacy API), the cached 
value remains valid. Otherwise, a new filter evaluation has to take place. For 
example, a service could have been not allowed but a new alias matches an 
authorization rule in the filter that flips the previous decision.

The method Provider.Service::computeIsAllowed (that internally invokes 
ProvidersFilter::computeIsAllowed) can be used to force the recomputation of an 
authorization cached decision. The method ProvidersFilter::isAllowed, when 
filtering capabilities are enabled, tries to get the service authorization from 
the Provider.Service isAllowed 

Re: RFR: 8233160: Add java.vendor.url.bug to list of recognized standard system properties

[Jaikiran Pai]

On Thu, 31 Aug 2023 06:53:45 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which proposes to address 
> https://bugs.openjdk.org/browse/JDK-8233160?
> 
> It has been noted in https://bugs.openjdk.org/browse/JDK-8232753 that:
> 
>> The java.vendor.url.bug property has been defined by every Sun/Oracle JDK 
>> going all the way back to JDK 5 (and possibly earlier; JDK 5 is the oldest 
>> release that I can still run on my development machine). Yet, it's never 
>> been specified.
> 
> The OpenJDK builds too by default set a value for this system property.
> 
> The commit in this PR updates the javadoc of `System.getProperties()` to 
> include this system property in the list of specified properties. 
> Additionally, the `test/jdk/java/lang/System/PropertyTest.java` test has been 
> updated to verify that this property is indeed available in the Properties 
> returned by `System.getProperites()`.
> 
> I'll create a CSR shortly for this change.

I asked for Mark's and Joe's inputs on this in the linked JBS issue 
https://bugs.openjdk.org/browse/JDK-8233160. Mark has suggested that we keep 
this `java.vendor.url.bug` system property non-optional and also not to add any 
expectations of the value for this property to be a parsable URL.

Both of those inputs match with what is currently proposed in this PR. I'll go 
ahead and draft a CSR shortly.

-

PR Comment: https://git.openjdk.org/jdk/pull/15504#issuecomment-1702893039

Re: RFR: 8288899: java/util/concurrent/ExecutorService/CloseTest.java failed with "InterruptedException: sleep interrupted" [v20]

[Doug Lea]

> Addresses Jdk 8288899 : java/util/concurrent/ExecutorService/CloseTest.java 
> failed with "InterruptedException: sleep interrupted" and related issues.
> 
> This is a major ForkJoin update (and hard to review -- sorry) that finally 
> addresses incompatibilities between ExecutorService and ForkJoinPool (which 
> claims to implement it), with the goal of avoiding continuing bug reports and 
> incompatibilities. Doing this required reworking internal control to use 
> phaser/seqlock-style versioning schemes (affecting nearly every method) that 
> ensure consistent data structures and actions without requiring global 
> synchronization or locking on every task execution that would massively 
> degrade performance. The previous lack of a solution to this was the main 
> reason for these incompatibilities.

Doug Lea 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 64 additional commits since the 
last revision:

 - Merge branch 'openjdk:master' into JDK-8288899
 - Fix tests, undo workarounds
 - Avoid unwanted jtreg interrupts; undo unnecessary changes
 - Merge branch 'openjdk:master' into JDK-8288899
 - Clear interrupts at top level
 - Conditionalize new tests
 - Isolate unexpected interrupt status issues
 - Ordering for termination triggering
 - Resync CloseTest
 - Merge branch 'openjdk:master' into JDK-8288899
 - ... and 54 more: https://git.openjdk.org/jdk/compare/6c66a521...c40a3245

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14301/files
  - new: https://git.openjdk.org/jdk/pull/14301/files/53599566..c40a3245

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14301=19
 - incr: https://webrevs.openjdk.org/?repo=jdk=14301=18-19

  Stats: 11417 lines in 274 files changed: 7432 ins; 1985 del; 2000 mod
  Patch: https://git.openjdk.org/jdk/pull/14301.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14301/head:pull/14301

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

Re: RFR: 8315454: Add a way to create an immutable snapshot of a BitSet [v3]

[Per Minborg]

> This PR proposes adding a new method to BitSet that provides an immutable 
> snapshot of the set in the form of an `IntPredicate`.
> 
> The predicate is eligible for constant folding.
> 
> Here are some classes in the JDK that would benefit directly from 
> constant-folding of BitSets:
> 
> PoolReader (6)
> URLEncoder (1) - Updated in this PR
> HtmlTree (2)
> 
> More over, the implementation of the predicate is @ValueBased and this would 
> provide additional benefits in the future.
> 
> Initial benchmarks with the URLEncoder show encouraging results:
> 
> 
> Name   (encodeChars) (maxLength) (unchanged) Cnt  
> Base   Error   Test   Error  Unit  Diff%
> URLEncodeDecode.testEncodeUTF8 61024   0  15 
> 2,371 ± 0,016  2,073 ± 0,184 ms/op  12,6% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024  75  15 
> 1,772 ± 0,013  1,387 ± 0,032 ms/op  21,8% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024 100  15 
> 1,230 ± 0,009  1,140 ± 0,011 ms/op   7,3% (p = 0,000*)

Per Minborg has updated the pull request incrementally with one additional 
commit since the last revision:

  Add comment on defensive copying

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15530/files
  - new: https://git.openjdk.org/jdk/pull/15530/files/2cd9eebd..c3977aa3

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=15530=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=15530=01-02

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

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

Re: RFR: 8315454: Add an immutable BitSet [v2]

[Per Minborg]

> This PR proposes adding a new method to BitSet that provides an immutable 
> snapshot of the set in the form of an `IntPredicate`.
> 
> The predicate is eligible for constant folding.
> 
> Here are some classes in the JDK that would benefit directly from 
> constant-folding of BitSets:
> 
> PoolReader (6)
> URLEncoder (1) - Updated in this PR
> HtmlTree (2)
> 
> More over, the implementation of the predicate is @ValueBased and this would 
> provide additional benefits in the future.
> 
> Initial benchmarks with the URLEncoder show encouraging results:
> 
> 
> Name   (encodeChars) (maxLength) (unchanged) Cnt  
> Base   Error   Test   Error  Unit  Diff%
> URLEncodeDecode.testEncodeUTF8 61024   0  15 
> 2,371 ± 0,016  2,073 ± 0,184 ms/op  12,6% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024  75  15 
> 1,772 ± 0,013  1,387 ± 0,032 ms/op  21,8% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024 100  15 
> 1,230 ± 0,009  1,140 ± 0,011 ms/op   7,3% (p = 0,000*)

Per Minborg has updated the pull request incrementally with three additional 
commits since the last revision:

 - Fix test
 - Remove unused imports
 - Convert into an internal API

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15530/files
  - new: https://git.openjdk.org/jdk/pull/15530/files/e4be1ff0..2cd9eebd

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=15530=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=15530=00-01

  Stats: 111 lines in 4 files changed: 59 ins; 47 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/15530.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15530/head:pull/15530

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

Re: RFR: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock. [v2]

[Alan Bateman]

On Fri, 1 Sep 2023 13:10:19 GMT, Matthias Baesken  wrote:

> Hi Alan , Your assumption 'I assume the use of System.getProperty is 
> problematic when running with a SM.' is most likely correct.

You'll need to test with a SM that denies reading the system property to be 
sure. There are classes in many tool APIs (you've listed some) where this 
doesn't arise.

In any case, I assume this lookup will go away once you replace this method.

-

PR Comment: https://git.openjdk.org/jdk/pull/15308#issuecomment-1702749867

Re: RFR: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock. [v2]

[Matthias Baesken]

On Fri, 1 Sep 2023 12:36:28 GMT, Matthias Baesken  wrote:

>> We run into some BackingStoreException: Couldn't get file lock. e.g. here :
>> 
>> [JShell] Exception in thread "main" java.lang.IllegalStateException: 
>> java.util.prefs.BackingStoreException: Couldn't get file lock.
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:313)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellTool$ReplayableHistory.storeHistory(JShellTool.java:692)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellTool.start(JShellTool.java:1008)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder.start(JShellToolBuilder.java:261)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider.main(JShellToolProvider.java:120)
>> [JShell] Caused by: java.util.prefs.BackingStoreException: Couldn't get file 
>> lock.
>> [JShell] at 
>> java.prefs/java.util.prefs.FileSystemPreferences.sync(FileSystemPreferences.java:769)
>> [JShell] at 
>> java.prefs/java.util.prefs.FileSystemPreferences.flush(FileSystemPreferences.java:864)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:311)
>> [JShell] ... 4 more
>> 
>> The BackingStoreException should be enhanced e.g. by adding the 
>> errno/errorCode that is already available in the coding
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   add for some errnos also the string

Hi Martin, thanks for the review !  I did some small adjustments to follow your 
coding style suggestions.

-

PR Comment: https://git.openjdk.org/jdk/pull/15308#issuecomment-1702741358

Re: RFR: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock. [v3]

[Matthias Baesken]

> We run into some BackingStoreException: Couldn't get file lock. e.g. here :
> 
> [JShell] Exception in thread "main" java.lang.IllegalStateException: 
> java.util.prefs.BackingStoreException: Couldn't get file lock.
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:313)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellTool$ReplayableHistory.storeHistory(JShellTool.java:692)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellTool.start(JShellTool.java:1008)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder.start(JShellToolBuilder.java:261)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider.main(JShellToolProvider.java:120)
> [JShell] Caused by: java.util.prefs.BackingStoreException: Couldn't get file 
> lock.
> [JShell] at 
> java.prefs/java.util.prefs.FileSystemPreferences.sync(FileSystemPreferences.java:769)
> [JShell] at 
> java.prefs/java.util.prefs.FileSystemPreferences.flush(FileSystemPreferences.java:864)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:311)
> [JShell] ... 4 more
> 
> The BackingStoreException should be enhanced e.g. by adding the 
> errno/errorCode that is already available in the coding

Matthias Baesken has updated the pull request incrementally with one additional 
commit since the last revision:

  follow Martin's codestyle suggestions

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15308/files
  - new: https://git.openjdk.org/jdk/pull/15308/files/9230e1e3..d385dac1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=15308=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=15308=01-02

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

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

Re: RFR: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock. [v2]

[Matthias Baesken]

On Fri, 1 Sep 2023 12:36:28 GMT, Matthias Baesken  wrote:

>> We run into some BackingStoreException: Couldn't get file lock. e.g. here :
>> 
>> [JShell] Exception in thread "main" java.lang.IllegalStateException: 
>> java.util.prefs.BackingStoreException: Couldn't get file lock.
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:313)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellTool$ReplayableHistory.storeHistory(JShellTool.java:692)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellTool.start(JShellTool.java:1008)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder.start(JShellToolBuilder.java:261)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider.main(JShellToolProvider.java:120)
>> [JShell] Caused by: java.util.prefs.BackingStoreException: Couldn't get file 
>> lock.
>> [JShell] at 
>> java.prefs/java.util.prefs.FileSystemPreferences.sync(FileSystemPreferences.java:769)
>> [JShell] at 
>> java.prefs/java.util.prefs.FileSystemPreferences.flush(FileSystemPreferences.java:864)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:311)
>> [JShell] ... 4 more
>> 
>> The BackingStoreException should be enhanced e.g. by adding the 
>> errno/errorCode that is already available in the coding
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   add for some errnos also the string

Hi Alan ,
Your assumption   'I assume the use of System.getProperty is problematic when 
running with a SM.'is most likely correct.


java.desktop/share/classes/sun/awt/FontConfiguration.java-133-protected 
void setOsNameAndVersion() {
java.desktop/share/classes/sun/awt/FontConfiguration.java:134:osName = 
System.getProperty("os.name");
java.desktop/share/classes/sun/awt/FontConfiguration.java-135-osVersion 
= System.getProperty("os.version");
java.desktop/share/classes/sun/awt/FontConfiguration.java-136-}
java.desktop/share/classes/sun/awt/FontConfiguration.java-137-
--
java.management/share/classes/javax/management/loading/MLet.java-1054- 
// the architecture-specific path name.  e.g. if user
java.management/share/classes/javax/management/loading/MLet.java-1055- 
// requested a load for "foo" on Solaris SPARC 5.7 we try to
java.management/share/classes/javax/management/loading/MLet.java-1056- 
// load "SunOS/sparc/5.7/lib/libfoo.so" from the JAR file.
java.management/share/classes/javax/management/loading/MLet.java-1057- 
//
java.management/share/classes/javax/management/loading/MLet.java:1058: 
nativelibname = removeSpace(System.getProperty("os.name")) + File.separator +
java.management/share/classes/javax/management/loading/MLet.java-1059-  
   removeSpace(System.getProperty("os.arch")) + File.separator +
java.management/share/classes/javax/management/loading/MLet.java-1060-  
   removeSpace(System.getProperty("os.version")) + File.separator +
java.management/share/classes/javax/management/loading/MLet.java-1061-  
   "lib" + File.separator + nativelibname;
java.management/share/classes/javax/management/loading/MLet.java-1062- 
if (MLET_LOGGER.isLoggable(Level.TRACE)) {
--
java.management/share/classes/sun/management/VMManagementImpl.java-223-// 
Operating System
java.management/share/classes/sun/management/VMManagementImpl.java-224-
public String getOsName() {
java.management/share/classes/sun/management/VMManagementImpl.java:225:
return System.getProperty("os.name");
java.management/share/classes/sun/management/VMManagementImpl.java-226-}
java.management/share/classes/sun/management/VMManagementImpl.java-227-
public String getOsArch() {
java.management/share/classes/sun/management/VMManagementImpl.java-228-
return System.getProperty("os.arch");
java.management/share/classes/sun/management/VMManagementImpl.java-229-}
--
jdk.compiler/share/classes/com/sun/tools/javac/jvm/JNIWriter.java-99-
jdk.compiler/share/classes/com/sun/tools/javac/jvm/JNIWriter.java-100-
private Context context;
jdk.compiler/share/classes/com/sun/tools/javac/jvm/JNIWriter.java-101-
jdk.compiler/share/classes/com/sun/tools/javac/jvm/JNIWriter.java-102-
private static final boolean isWindows =
jdk.compiler/share/classes/com/sun/tools/javac/jvm/JNIWriter.java:103:
System.getProperty("os.name").startsWith("Windows");
jdk.compiler/share/classes/com/sun/tools/javac/jvm/JNIWriter.java-104-
--
jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PlatformInfo.java-29-
jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PlatformInfo.java-30-public
 class PlatformInfo {
jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/PlatformInfo.java-31- 
 /* Returns "win32" if Windows; "linux" if Linux. */

Re: RFR: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock. [v2]

[Matthias Baesken]

On Fri, 1 Sep 2023 12:36:28 GMT, Matthias Baesken  wrote:

>> We run into some BackingStoreException: Couldn't get file lock. e.g. here :
>> 
>> [JShell] Exception in thread "main" java.lang.IllegalStateException: 
>> java.util.prefs.BackingStoreException: Couldn't get file lock.
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:313)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellTool$ReplayableHistory.storeHistory(JShellTool.java:692)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellTool.start(JShellTool.java:1008)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder.start(JShellToolBuilder.java:261)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider.main(JShellToolProvider.java:120)
>> [JShell] Caused by: java.util.prefs.BackingStoreException: Couldn't get file 
>> lock.
>> [JShell] at 
>> java.prefs/java.util.prefs.FileSystemPreferences.sync(FileSystemPreferences.java:769)
>> [JShell] at 
>> java.prefs/java.util.prefs.FileSystemPreferences.flush(FileSystemPreferences.java:864)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:311)
>> [JShell] ... 4 more
>> 
>> The BackingStoreException should be enhanced e.g. by adding the 
>> errno/errorCode that is already available in the coding
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   add for some errnos also the string

Hi Alan,
> If there are only a couple of errno's that occur for these cases, it might be 
> easier to hardwire the translation.
I just followed the idea suggested above to do the translation in the Java 
class itself . Do you have a good example where it is done by strerror and then 
passed back to Java , without much overhead?

-

PR Comment: https://git.openjdk.org/jdk/pull/15308#issuecomment-1702721259

Re: RFR: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock. [v2]

[Alan Bateman]

On Fri, 1 Sep 2023 12:36:28 GMT, Matthias Baesken  wrote:

>> We run into some BackingStoreException: Couldn't get file lock. e.g. here :
>> 
>> [JShell] Exception in thread "main" java.lang.IllegalStateException: 
>> java.util.prefs.BackingStoreException: Couldn't get file lock.
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:313)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellTool$ReplayableHistory.storeHistory(JShellTool.java:692)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellTool.start(JShellTool.java:1008)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder.start(JShellToolBuilder.java:261)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider.main(JShellToolProvider.java:120)
>> [JShell] Caused by: java.util.prefs.BackingStoreException: Couldn't get file 
>> lock.
>> [JShell] at 
>> java.prefs/java.util.prefs.FileSystemPreferences.sync(FileSystemPreferences.java:769)
>> [JShell] at 
>> java.prefs/java.util.prefs.FileSystemPreferences.flush(FileSystemPreferences.java:864)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:311)
>> [JShell] ... 4 more
>> 
>> The BackingStoreException should be enhanced e.g. by adding the 
>> errno/errorCode that is already available in the coding
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   add for some errnos also the string

src/java.prefs/unix/classes/java/util/prefs/FileSystemPreferences.java line 722:

> 720: case 1:  return "EPERM";
> 721: }
> 722: return "";

I assume the use of System.getProperty is problematic when running with a SM.

In any case, I don't think this is the right way to do it, instead you want a 
native method to call strerror to translate the error.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15308#discussion_r1312988133

Re: RFR: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock. [v2]

[Martin Doerr]

On Fri, 1 Sep 2023 12:36:28 GMT, Matthias Baesken  wrote:

>> We run into some BackingStoreException: Couldn't get file lock. e.g. here :
>> 
>> [JShell] Exception in thread "main" java.lang.IllegalStateException: 
>> java.util.prefs.BackingStoreException: Couldn't get file lock.
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:313)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellTool$ReplayableHistory.storeHistory(JShellTool.java:692)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellTool.start(JShellTool.java:1008)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder.start(JShellToolBuilder.java:261)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider.main(JShellToolProvider.java:120)
>> [JShell] Caused by: java.util.prefs.BackingStoreException: Couldn't get file 
>> lock.
>> [JShell] at 
>> java.prefs/java.util.prefs.FileSystemPreferences.sync(FileSystemPreferences.java:769)
>> [JShell] at 
>> java.prefs/java.util.prefs.FileSystemPreferences.flush(FileSystemPreferences.java:864)
>> [JShell] at 
>> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:311)
>> [JShell] ... 4 more
>> 
>> The BackingStoreException should be enhanced e.g. by adding the 
>> errno/errorCode that is already available in the coding
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   add for some errnos also the string

Nice improvement! Please see my minor comments.

src/java.prefs/unix/classes/java/util/prefs/FileSystemPreferences.java line 723:

> 721: }
> 722: return "";
> 723: }

Would be a nice use case of switch expressions. But, I'm ok with it.

src/java.prefs/unix/classes/java/util/prefs/FileSystemPreferences.java line 731:

> 729: if (errCode != 0) {
> 730: String errStr = getErrorString(errCode);
> 731: if (! errStr.equals("")) errStr = " (" + errStr + ")";

Coding style: whitespace after '!'

src/java.prefs/unix/classes/java/util/prefs/FileSystemPreferences.java line 732:

> 730: String errStr = getErrorString(errCode);
> 731: if (! errStr.equals("")) errStr = " (" + errStr + ")";
> 732: throw(new BackingStoreException("Couldn't get file lock. 
> errno is " + errCode + errStr));

Strange coding style (also in the original code). I'd use "throw new ...".

src/java.prefs/unix/classes/java/util/prefs/FileSystemPreferences.java line 798:

> 796:if (!shared) sharingMode = "nonshared";
> 797:String errStr = getErrorString(errCode);
> 798:if (! errStr.equals("")) errStr = " (" + errStr + ")";

as above

src/java.prefs/unix/classes/java/util/prefs/FileSystemPreferences.java line 799:

> 797:String errStr = getErrorString(errCode);
> 798:if (! errStr.equals("")) errStr = " (" + errStr + ")";
> 799:throw(new BackingStoreException("Couldn't get file lock. 
> errno is " + errCode + errStr + " mode is " + sharingMode));

as above

-

Marked as reviewed by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15308#pullrequestreview-1606876656
PR Review Comment: https://git.openjdk.org/jdk/pull/15308#discussion_r1312979770
PR Review Comment: https://git.openjdk.org/jdk/pull/15308#discussion_r1312980202
PR Review Comment: https://git.openjdk.org/jdk/pull/15308#discussion_r1312982035
PR Review Comment: https://git.openjdk.org/jdk/pull/15308#discussion_r1312982899
PR Review Comment: https://git.openjdk.org/jdk/pull/15308#discussion_r1312983075

Re: Crash during optimizing exploded image with my patch

[erik . joelsson]

Hello Chen Liang,

The exploded image optimization step runs the newly built JDK (in the 
exploded image) to generate data for itself. This means that if you 
change something fundamental in the JDK core libraries or the JVM, that 
may cause crashes in this build step. To diagnose further, you can 
run/debug the exploded image directly.


/Erik

On 8/31/23 19:54, - wrote:

Hello,
I am trying to create a patch [1] that optimizes forEach for immutable 
factory collections (from List/Set.of) to see how it compares with 
constant-folded random access. However, this patch somehow tampers 
with exploded image optimization and modules during build, and I 
cannot build this patch on my windows 11 device.


I have tried using both Oracle JDK 20 build 20+36-2344 (in JAVA_HOME) 
and the JDK 21 build 35 as boot JDK. JDK 20 fails with an IAE with 
message "Bad package name" (seems to be from modules.cpp) and JDK 21 
fails with an NPE 'java.lang.NullPointerException: Cannot invoke 
"java.lang.module.ResolvedModule.name 
()" because 
"resolvedModules[index]" is null', both happening after reconfiguring 
and cleaning. These are my command-line:


bash configure --with-boot-jdk=/cygdrive/c/java/boot/jdk-21 
--enable-hsdis-bundling --with-hsdis=capstone 
--with-capstone=/cygdrive/c/java/capstone-4.0.2-win64 
--with-jmh=build/jmh/jars --with-conf-name=windows-x64 
--enable-jtreg-failure-handler


Does anybody have any clue on why my patch fails the build? And how 
can I resolve this problem?


Chen Liang

[1] https://github.com/liachmodded/jdk/pull/new/feature/imm-coll-stream

Re: RFR: 8310929: Optimization for Integer.toString [v15]

[温绍锦]

> Optimization for:
> Integer.toString
> Long.toString
> StringBuilder#append(int)
> 
> # Benchmark Result
> 
> 
> sh make/devkit/createJMHBundle.sh
> bash configure --with-jmh=build/jmh/jars
> make test TEST="micro:java.lang.Integers.toString*" 
> make test TEST="micro:java.lang.Longs.toString*" 
> make test TEST="micro:java.lang.StringBuilders.toStringCharWithInt8"
> 
> 
> ## 1. 
> [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i)
> * cpu : intel xeon sapphire rapids (x64)
> 
> ``` diff
> -Benchmark   (size)  Mode  Cnt  Score   Error  Units (baseline)
> -Integers.toStringBig   500  avgt   15  6.825 ± 0.023  us/op
> -Integers.toStringSmall 500  avgt   15  4.823 ± 0.023  us/op
> -Integers.toStringTiny  500  avgt   15  3.878 ± 0.101  us/op
> 
> +Benchmark   (size)  Mode  Cnt  Score   Error  Units (PR Update 
> 04 f4aa1989)
> +Integers.toStringBig   500  avgt   15  6.002 ± 0.054  us/op (+13.71%)
> +Integers.toStringSmall 500  avgt   15  4.025 ± 0.020  us/op (+19.82%)
> +Integers.toStringTiny  500  avgt   15  3.874 ± 0.067  us/op (+0.10%)
> 
> -Benchmark(size)  Mode  Cnt  Score   Error  Units (baseline)
> -Longs.toStringBig   500  avgt   15  9.224 ± 0.021  us/op
> -Longs.toStringSmall 500  avgt   15  4.621 ± 0.087  us/op
> 
> +Benchmark(size)  Mode  Cnt  Score   Error  Units (PR Update 04 
> f4aa1989)
> +Longs.toStringBig   500  avgt   15  7.483 ± 0.018  us/op (+23.26%)
> +Longs.toStringSmall 500  avgt   15  4.020 ± 0.016  us/op (+14.95%)
> 
> -Benchmark   Mode  Cnt ScoreError  Units 
> (baseline)
> -StringBuilders.toStringCharWithInt8 avgt   1589.327 ±  0.733  ns/op
> 
> +BenchmarkMode  Cnt   Score   Error  Units (PR 
> Update 04 f4aa1989)
> +StringBuilders.toStringCharWithInt8  avgt   15  36.639 ± 0.422  ns/op 
> (+143.80%)
> 
> 
> 
> ## 2. 
> [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a)
> * cpu : amd epc genoa (x64)
> 
> ``` diff
> -Benchmark   (size)  Mode  Cnt  Score   Error  Units (baseline)
> -Integers.toStringBig   500  avgt   15  6.753 ± 0.007  us/op
> -Integers.toStringSmall 500  avgt   15  4.470 ± 0.005  us/op
> -Integers.toStringTiny  500  avgt   15  2.764 ± 0.020  us/op
> 
> +Benchmark   (size)  Mode  Cnt  Score   Error  Units (PR Update 
> 04 f4aa1989)
> +Integers.toStringBig   500  avgt   15  5.036 ± 0.005  us/op (+34.09%)
> +Integers.toStringSmall 500  avgt   15  3.491 ± 0.025  us/op (+28.04%)
> +Integers.toStringTiny  5...

温绍锦 has updated the pull request incrementally with one additional commit since 
the last revision:

  remove unused import

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14699/files
  - new: https://git.openjdk.org/jdk/pull/14699/files/d8a3d676..cf2f8395

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14699=14
 - incr: https://webrevs.openjdk.org/?repo=jdk=14699=13-14

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

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

Re: RFR: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock.

[Matthias Baesken]

On Wed, 16 Aug 2023 13:36:38 GMT, Matthias Baesken  wrote:

> We run into some BackingStoreException: Couldn't get file lock. e.g. here :
> 
> [JShell] Exception in thread "main" java.lang.IllegalStateException: 
> java.util.prefs.BackingStoreException: Couldn't get file lock.
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:313)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellTool$ReplayableHistory.storeHistory(JShellTool.java:692)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellTool.start(JShellTool.java:1008)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder.start(JShellToolBuilder.java:261)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider.main(JShellToolProvider.java:120)
> [JShell] Caused by: java.util.prefs.BackingStoreException: Couldn't get file 
> lock.
> [JShell] at 
> java.prefs/java.util.prefs.FileSystemPreferences.sync(FileSystemPreferences.java:769)
> [JShell] at 
> java.prefs/java.util.prefs.FileSystemPreferences.flush(FileSystemPreferences.java:864)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:311)
> [JShell] ... 4 more
> 
> The BackingStoreException should be enhanced e.g. by adding the 
> errno/errorCode that is already available in the coding

I added the output of some errno related strings (like EACCES) to the 
exceptions.

-

PR Comment: https://git.openjdk.org/jdk/pull/15308#issuecomment-1702671601

Re: RFR: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock. [v2]

[Matthias Baesken]

> We run into some BackingStoreException: Couldn't get file lock. e.g. here :
> 
> [JShell] Exception in thread "main" java.lang.IllegalStateException: 
> java.util.prefs.BackingStoreException: Couldn't get file lock.
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:313)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellTool$ReplayableHistory.storeHistory(JShellTool.java:692)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellTool.start(JShellTool.java:1008)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder.start(JShellToolBuilder.java:261)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider.main(JShellToolProvider.java:120)
> [JShell] Caused by: java.util.prefs.BackingStoreException: Couldn't get file 
> lock.
> [JShell] at 
> java.prefs/java.util.prefs.FileSystemPreferences.sync(FileSystemPreferences.java:769)
> [JShell] at 
> java.prefs/java.util.prefs.FileSystemPreferences.flush(FileSystemPreferences.java:864)
> [JShell] at 
> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:311)
> [JShell] ... 4 more
> 
> The BackingStoreException should be enhanced e.g. by adding the 
> errno/errorCode that is already available in the coding

Matthias Baesken has updated the pull request incrementally with one additional 
commit since the last revision:

  add for some errnos also the string

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15308/files
  - new: https://git.openjdk.org/jdk/pull/15308/files/49519782..9230e1e3

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=15308=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=15308=00-01

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

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

Re: RFR: 8312491: Update Classfile API snippets and examples [v11]

[Adam Sotona]

> This pull request updates Classfile API snippets and examples and adds 
> missing javadoc.
> 
> Please review.
> 
> Thanks,
> Adam

Adam Sotona has updated the pull request incrementally with one additional 
commit since the last revision:

  added notes to attributes

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14968/files
  - new: https://git.openjdk.org/jdk/pull/14968/files/f4f74cac..ac8709c5

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14968=10
 - incr: https://webrevs.openjdk.org/?repo=jdk=14968=09-10

  Stats: 135 lines in 37 files changed: 132 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/14968.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14968/head:pull/14968

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

Re: RFR: 8310929: Optimization for Integer.toString [v13]

[温绍锦]

On Thu, 31 Aug 2023 02:36:09 GMT, 温绍锦  wrote:

>> 温绍锦 has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   assert bounds check
>
> @cl4es can you help me to review this PR?

> @wenshao How about of approach used in [James Anhalt's 
> algorithm](https://jk-jeon.github.io/posts/2022/02/jeaiii-algorithm/)?
> 
> It reduces number of multiplications ([and store operations in case of 
> writing by 4-8 byte 
> words](https://github.com/plokhotnyuk/jsoniter-scala/blob/b1020bafa7e2e5b7e8dd87b86a9e860975f4695e/jsoniter-scala-core/jvm/src/main/scala/com/github/plokhotnyuk/jsoniter_scala/core/JsonWriter.scala#L2004-L2023))
>  but increases the total number of instructions for the routine.

If the compiled code size is greater than 325 (FreqInlineSize), it will not be 
inlined and performance will slow down. This algorithm obviously greater than 
325.

different algorithms perform differently on different types of tiny/small/big 
values.

The [first 
commit](https://github.com/openjdk/jdk/pull/14699/files/8bdda26b8bb7f6162592a07445ecd2285e4fabaa)
 of this PR performs better under big values. But I still use the current 
algorithm, with few changes, and performance can be improved in all scenarios.

-

PR Comment: https://git.openjdk.org/jdk/pull/14699#issuecomment-1702637851

Re: RFR: 8310929: Optimization for Integer.toString [v14]

[温绍锦]

> Optimization for:
> Integer.toString
> Long.toString
> StringBuilder#append(int)
> 
> # Benchmark Result
> 
> 
> sh make/devkit/createJMHBundle.sh
> bash configure --with-jmh=build/jmh/jars
> make test TEST="micro:java.lang.Integers.toString*" 
> make test TEST="micro:java.lang.Longs.toString*" 
> make test TEST="micro:java.lang.StringBuilders.toStringCharWithInt8"
> 
> 
> ## 1. 
> [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i)
> * cpu : intel xeon sapphire rapids (x64)
> 
> ``` diff
> -Benchmark   (size)  Mode  Cnt  Score   Error  Units (baseline)
> -Integers.toStringBig   500  avgt   15  6.825 ± 0.023  us/op
> -Integers.toStringSmall 500  avgt   15  4.823 ± 0.023  us/op
> -Integers.toStringTiny  500  avgt   15  3.878 ± 0.101  us/op
> 
> +Benchmark   (size)  Mode  Cnt  Score   Error  Units (PR Update 
> 04 f4aa1989)
> +Integers.toStringBig   500  avgt   15  6.002 ± 0.054  us/op (+13.71%)
> +Integers.toStringSmall 500  avgt   15  4.025 ± 0.020  us/op (+19.82%)
> +Integers.toStringTiny  500  avgt   15  3.874 ± 0.067  us/op (+0.10%)
> 
> -Benchmark(size)  Mode  Cnt  Score   Error  Units (baseline)
> -Longs.toStringBig   500  avgt   15  9.224 ± 0.021  us/op
> -Longs.toStringSmall 500  avgt   15  4.621 ± 0.087  us/op
> 
> +Benchmark(size)  Mode  Cnt  Score   Error  Units (PR Update 04 
> f4aa1989)
> +Longs.toStringBig   500  avgt   15  7.483 ± 0.018  us/op (+23.26%)
> +Longs.toStringSmall 500  avgt   15  4.020 ± 0.016  us/op (+14.95%)
> 
> -Benchmark   Mode  Cnt ScoreError  Units 
> (baseline)
> -StringBuilders.toStringCharWithInt8 avgt   1589.327 ±  0.733  ns/op
> 
> +BenchmarkMode  Cnt   Score   Error  Units (PR 
> Update 04 f4aa1989)
> +StringBuilders.toStringCharWithInt8  avgt   15  36.639 ± 0.422  ns/op 
> (+143.80%)
> 
> 
> 
> ## 2. 
> [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a)
> * cpu : amd epc genoa (x64)
> 
> ``` diff
> -Benchmark   (size)  Mode  Cnt  Score   Error  Units (baseline)
> -Integers.toStringBig   500  avgt   15  6.753 ± 0.007  us/op
> -Integers.toStringSmall 500  avgt   15  4.470 ± 0.005  us/op
> -Integers.toStringTiny  500  avgt   15  2.764 ± 0.020  us/op
> 
> +Benchmark   (size)  Mode  Cnt  Score   Error  Units (PR Update 
> 04 f4aa1989)
> +Integers.toStringBig   500  avgt   15  5.036 ± 0.005  us/op (+34.09%)
> +Integers.toStringSmall 500  avgt   15  3.491 ± 0.025  us/op (+28.04%)
> +Integers.toStringTiny  5...

温绍锦 has updated the pull request incrementally with one additional commit since 
the last revision:

  UTF16 & Latin1 use same lookup table

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14699/files
  - new: https://git.openjdk.org/jdk/pull/14699/files/a69f6d2a..d8a3d676

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14699=13
 - incr: https://webrevs.openjdk.org/?repo=jdk=14699=12-13

  Stats: 59 lines in 1 file changed: 23 ins; 31 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/14699.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14699/head:pull/14699

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

Re: RFR: 8288899: java/util/concurrent/ExecutorService/CloseTest.java failed with "InterruptedException: sleep interrupted" [v19]

[Doug Lea]

> Addresses Jdk 8288899 : java/util/concurrent/ExecutorService/CloseTest.java 
> failed with "InterruptedException: sleep interrupted" and related issues.
> 
> This is a major ForkJoin update (and hard to review -- sorry) that finally 
> addresses incompatibilities between ExecutorService and ForkJoinPool (which 
> claims to implement it), with the goal of avoiding continuing bug reports and 
> incompatibilities. Doing this required reworking internal control to use 
> phaser/seqlock-style versioning schemes (affecting nearly every method) that 
> ensure consistent data structures and actions without requiring global 
> synchronization or locking on every task execution that would massively 
> degrade performance. The previous lack of a solution to this was the main 
> reason for these incompatibilities.

Doug Lea has updated the pull request incrementally with one additional commit 
since the last revision:

  Fix tests, undo workarounds

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14301/files
  - new: https://git.openjdk.org/jdk/pull/14301/files/4b12ddc9..53599566

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14301=18
 - incr: https://webrevs.openjdk.org/?repo=jdk=14301=17-18

  Stats: 45 lines in 2 files changed: 15 ins; 5 del; 25 mod
  Patch: https://git.openjdk.org/jdk/pull/14301.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14301/head:pull/14301

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

Re: RFR: 8315373: Change VirtualThread to unmount after freezing, re-mount before thawing

[Markus Grönlund]

On Fri, 1 Sep 2023 11:26:48 GMT, Alan Bateman  wrote:

>> Ok. If the thread can run native code as part of the mount / unmount, and 
>> the sampler can see the intermediary state there too, the check is needed.
>
> Thanks. One other thing that I see when more testing with generational ZGC is 
> that ZPageAllocator::alloc_page will record an event and this can happen 
> during a mount transition. So I think JfrStackTrace::record also needs to 
> test if the current thread is a virtual thread without a continuation mounted.

That is not good. Such a check would be attributed to all event sites, making 
them all slower.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15492#discussion_r1312915249

Re: RFR: 8310929: Optimization for Integer.toString [v13]

[温绍锦]

On Thu, 31 Aug 2023 19:53:17 GMT, Claes Redestad  wrote:

>> 温绍锦 has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   assert bounds check
>
> src/java.base/share/classes/java/lang/StringUTF16.java line 1585:
> 
>> 1583: buf,
>> 1584: Unsafe.ARRAY_BYTE_BASE_OFFSET + (charPos << 1),
>> 1585: PACKED_DIGITS_UTF16[r]);
> 
> What performance would you get if you used the same lookup table as the other 
> implementations, but inflate the value to UTF-16 on the fly?
> 
> 
> int packed = (int)Integer.PACKED_DIGITS[r];
> int inflated = ((packed & 0xFF00) << 8) | (packed & 0xFF));
> UNSAFE.putIntUnaligned(
> buf,
> Unsafe.ARRAY_BYTE_BASE_OFFSET + (charPos << 1),
> inflated);
> 
> 
> This would avoid juggling more lookup table data around than before, 
> alleviating some of the concerns voiced in this PR comment thread.

Good suggestion, using the same lookup table, we can get similar performance.


  int packed = (int) Integer.PACKED_DIGITS[-i];
  int inflated = ((packed & 0xFF00) << 8) | (packed & 0xFF);

  charPos -= 2;
  assert charPos >= 0 && charPos < buf.length : "Trusted caller missed bounds 
check";
  UNSAFE.putIntUnaligned(
  buf,
  Unsafe.ARRAY_BYTE_BASE_OFFSET + (charPos << 1),
  inflated, 
  false);


The performance comparison data is as follows:


-Benchmark Mode  Cnt   Score   Error  Units
-StringBuilders.toStringCharWithInt8UTF16  avgt   15  26.812 ± 0.095  ns/op

+Benchmark Mode  Cnt   Score   Error  Units  
(use same lookup table)
+StringBuilders.toStringCharWithInt8UTF16  avgt   15  27.807 ± 0.046  ns/op

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14699#discussion_r1312910616

Re: RFR: 8315373: Change VirtualThread to unmount after freezing, re-mount before thawing

[Alan Bateman]

On Fri, 1 Sep 2023 10:47:25 GMT, Markus Grönlund  wrote:

>> Just to add that  Patricio suggested today to run the stress tests with 
>> -Xint and that does lead to triggering the assert quickly when the thread is 
>> sampled in native. There are several native methods that are 
>> @IntrinsicCandidate that are invoked after the thread identity has changed 
>> to the virtual thread and before the continuation is mounted. Probably less 
>> likely on the unmount as the only native method is the one that reverts the 
>> thread identity.
>
> Ok. If the thread can run native code as part of the mount / unmount, and the 
> sampler can see the intermediary state there too, the check is needed.

Thanks. One other thing that I see when more testing with generational ZGC is 
that ZPageAllocator::alloc_page will record an event and this can happen during 
a mount transition. So I think JfrStackTrace::record also needs to test if the 
current thread is a virtual thread without a continuation mounted.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15492#discussion_r1312910082

Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v30]

[iaroslavski]

On Wed, 30 Aug 2023 15:10:45 GMT, Srinivas Vamsi Parasa  
wrote:

>>> Hi Vladimir, Just verified that the test/jdk/java/util/Arrays/Sorting.java 
>>> is triggering the intrinsic without additional flags
>> 
>> Just to add that Sorting.java has short and long run modes. The default when 
>> running with jtreg or make run-test is the short run so that it doesn't take 
>> too long.  It might be useful to try it without -shortrun to give the 
>> intrinsic a better work out.
>
>> @AlanBateman If it helps, the changes made by @vamsi-parasa to 
>> DualPivotQuickSort.java don't change the logic as written in Java. They only 
>> carve out the functionality into separate Java methods retaining the meaning 
>> exactly as before. These Java methods are then optimized through a stub.
> 
> Hi Alan,
> 
> As Sandhya (@sviswa7) mentioned, this PR does not make any logic changes to 
> the DualPivotQuickSort.java. 
> 
> The code was refactored to separate out the partitioning logic (dual pivot 
> and single pivot) into separate methods. These methods are being intrinsified 
> using AVX512 to do vectorized partitioning preserving the logic of Java side. 
> Similarly, the methods to sort small arrays 
> (insertionSort/mixedInsertionSort) are being intrinsified as well to use 
> AVX512 sort while preserving the logic on Java side.
> 
> Could you please go through the changes and provide your comments and 
> suggestions?
> 
> Thanks,
> Vamsi

Hi Vamsi (@vamsi-parasa)!

Please see my few questions/suggestions related to method arraySort() and other 
code.

**1.** If size < MIN_FAST_SMALL_ARRAY_SORT_SIZE (=16), you don't go into 
intrinsic arraySort(), method
mixedInsertionSort is invoked instead.

`if (size < MAX_MIXED_INSERTION_SORT_SIZE + bits && (bits & 1) > 0) {`
`int last  = high - 3 * ((size >> 5) << 3);`
`if (size < MIN_FAST_SMALL_ARRAY_SORT_SIZE) mixedInsertionSort(a, low, last 
, high);`
`else arraySort(int.class, a, baseOffset, low, high, last);`
`return;`
`}`

Is Java mixedInsertionSort actually faster than intrinsic arraySort() on small 
(< 16) arrays?
What if you try to invoke arraySort() on all small arrays and don't use 
constant MIN_FAST_SMALL_ARRAY_SORT_SIZE at all?
Like this:

`if (size < MAX_MIXED_INSERTION_SORT_SIZE + bits && (bits & 1) > 0) {`
`int last  = high - 3 * ((size >> 5) << 3);`
`arraySort(int.class, a, baseOffset, low, high, last);`
`return;`
`}`

Could you please check and benchmark it?

**2.** On the next step you apply the same approach when you invoke 
insertionSort() on the small leftmost part.

`if (size < MAX_INSERTION_SORT_SIZE) {`
`if (size < MIN_FAST_SMALL_ARRAY_SORT_SIZE) insertionSort(a, low, high);`
`else arraySort(int.class, a, baseOffset, low, high, -1);`
`return;`
`}`

But this invocation happens only once, on the leftmost part only. I think we 
can invoke Java code and
don't go to intrinsic arraySort(). I guess we will not have profit with 
intrinsic method here. See:

`if (size < MAX_INSERTION_SORT_SIZE) {`
`insertionSort(a, low, high);`
`return;`
`}`

Could you please try this case without intrinsic and benchmark it?

**3.** You introduce constant int baseOffset = Unsafe.ARRAY_INT_BASE_OFFSET 
inside the loop.
What if we pass the constant directly? For example:

`arraySort(int.class, a, Unsafe.ARRAY_INT_BASE_OFFSET, low, high, last);`

**4.** You define 'int[] pivotIndices;' at the beginning of the loop, but the 
indices will be used
later (or not used at all). My proposal to declare pivotIndices in 
if-then-else, see:

`int[] pivotIndices = new int[] {e1, e5};`

**5.** Method arrayPartition has argument isDualPivot, I think we can avoid it.
If isDualPivot is true, pivot indices are different. If indices are equal, it 
means single pivot partitioning.
So, changes are:

`private static void arrayPartition(Class elemType, Object array, long 
offset, int low, int high, int[] pivotIndices) {`
`if (pivotIndices[0] == pivotIndices[1]) partitionSinglePivot(array, low, 
high, pivotIndices);`
`else partitionDualPivot(array, low, high, pivotIndices);`
`}`

**6.** Please add brackets for 'then' and 'else' statements according to common 
Java style:

`if (pivotIndices[0] == pivotIndices[1]) {`
`partitionSinglePivot(array, low, high, pivotIndices);`
`} else {`
`partitionDualPivot(array, low, high, pivotIndices);`
`};`

Thank you,
Vladimir

-

PR Comment: https://git.openjdk.org/jdk/pull/14227#issuecomment-1702565544

Re: RFR: 8315454: Add an immutable BitSet

[Claes Redestad]

On Fri, 1 Sep 2023 08:21:13 GMT, Per Minborg  wrote:

> This PR proposes adding a new method to BitSet that provides an immutable 
> snapshot of the set in the form of an `IntPredicate`.
> 
> The predicate is eligible for constant folding.
> 
> Here are some classes in the JDK that would benefit directly from 
> constant-folding of BitSets:
> 
> PoolReader (6)
> URLEncoder (1) - Updated in this PR
> HtmlTree (2)
> 
> More over, the implementation of the predicate is @ValueBased and this would 
> provide additional benefits in the future.
> 
> Initial benchmarks with the URLEncoder show encouraging results:
> 
> 
> Name   (encodeChars) (maxLength) (unchanged) Cnt  
> Base   Error   Test   Error  Unit  Diff%
> URLEncodeDecode.testEncodeUTF8 61024   0  15 
> 2,371 ± 0,016  2,073 ± 0,184 ms/op  12,6% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024  75  15 
> 1,772 ± 0,013  1,387 ± 0,032 ms/op  21,8% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024 100  15 
> 1,230 ± 0,009  1,140 ± 0,011 ms/op   7,3% (p = 0,000*)

While I think this minimal API could well prove to be a useful addition to many 
projects, this was conceived on a whim and while it's sufficient for 
`URLEncoder` and a few internal users, there's no evidence that this API fits 
the needs of third-party code. I'd be happy with an internal implementation 
initially.

-

PR Comment: https://git.openjdk.org/jdk/pull/15530#issuecomment-1702576346

Re: RFR: 8315373: Change VirtualThread to unmount after freezing, re-mount before thawing

[Markus Grönlund]

On Wed, 30 Aug 2023 13:56:42 GMT, Alan Bateman  wrote:

> In the virtual thread implementation, thread identity switches to the carrier 
> before freezing and switches back to the virtual thread after thawing. This 
> was a forced move due to issues getting JVMTI to work with virtual threads. 
> JVMTI can now hide events during transitions so we can invert the sequence 
> back to mounting before running the continuation, unmounting after freezing, 
> and re-mounting after thawing. This sequence is important for future changes 
> that will initiate the freezing from the VM.
> 
> The change requires an update to the JFR thread sampler to skip sampling when 
> it samples during a transition.
> 
> Testing: tier1-5

Marked as reviewed by mgronlun (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/15492#pullrequestreview-1606720202

Re: RFR: 8315373: Change VirtualThread to unmount after freezing, re-mount before thawing

[Markus Grönlund]

On Thu, 31 Aug 2023 19:28:50 GMT, Alan Bateman  wrote:

>> There are some native methods that we execute during mount/unmount 
>> transitions. From what I see they all seem to be defined as 
>> `@IntrinsicCandidate`, but if for some reason we don't execute the intrinsic 
>> version (interp only mode for instance) then we would call a normal native 
>> method.
>
> Just to add that  Patricio suggested today to run the stress tests with -Xint 
> and that does lead to triggering the assert quickly when the thread is 
> sampled in native. There are several native methods that are 
> @IntrinsicCandidate that are invoked after the thread identity has changed to 
> the virtual thread and before the continuation is mounted. Probably less 
> likely on the unmount as the only native method is the one that reverts the 
> thread identity.

Ok. If the thread can run native code as part of the mount / unmount, and the 
sampler can see the intermediary state there too, the check is needed.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15492#discussion_r1312875905

Re: RFR: 8315454: Add an immutable BitSet

[Viktor Klang]

On Fri, 1 Sep 2023 08:21:13 GMT, Per Minborg  wrote:

> This PR proposes adding a new method to BitSet that provides an immutable 
> snapshot of the set in the form of an `IntPredicate`.
> 
> The predicate is eligible for constant folding.
> 
> Here are some classes in the JDK that would benefit directly from 
> constant-folding of BitSets:
> 
> PoolReader (6)
> URLEncoder (1) - Updated in this PR
> HtmlTree (2)
> 
> More over, the implementation of the predicate is @ValueBased and this would 
> provide additional benefits in the future.
> 
> Initial benchmarks with the URLEncoder show encouraging results:
> 
> 
> Name   (encodeChars) (maxLength) (unchanged) Cnt  
> Base   Error   Test   Error  Unit  Diff%
> URLEncodeDecode.testEncodeUTF8 61024   0  15 
> 2,371 ± 0,016  2,073 ± 0,184 ms/op  12,6% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024  75  15 
> 1,772 ± 0,013  1,387 ± 0,032 ms/op  21,8% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024 100  15 
> 1,230 ± 0,009  1,140 ± 0,011 ms/op   7,3% (p = 0,000*)

src/java.base/share/classes/java/util/BitSet.java line 1412:

> 1410:  * 
> 1411:  * Returned predicates are threadsafe and can be used without 
> external synchronisation.  Furthermore,
> 1412:  * they are eligible for constant-folding optimization by the VM.

>  Furthermore,
 * they are eligible for constant-folding optimization by the VM.

I'd probably skip this unless there's prior art to describe the 
constant-fold-eligibility of returned values from methods. :)

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15530#discussion_r1312826139

Re: RFR: 8315454: Add an immutable BitSet

[Maurizio Cimadamore]

On Fri, 1 Sep 2023 09:07:34 GMT, Per Minborg  wrote:

> > Maybe it would make sense to start with a JDK-internal variant before 
> > exploring potential public API?
> 
> This is certainly one alternative.

When looking at this I thought about that too. It seems like the particular 
case where this optimization is applicable is very specific, and I'm on the 
fence as to whether this deserves a public API. At the same time, there might 
be near-misses that could be difficult to address once the API is set in stone. 
I'd suggest to let it bake (inside the JDK) for some time, and later on decide 
whether it should be exposed as a public API.

All that said, the idea of exposing a fast predicate over an immutable snapshot 
is very cute and clever :-)

-

PR Comment: https://git.openjdk.org/jdk/pull/15530#issuecomment-1702462814

Re: RFR: 8315454: Add an immutable BitSet

[Per Minborg]

On Fri, 1 Sep 2023 09:00:32 GMT, Hannes Greule  wrote:

> Maybe it would make sense to start with a JDK-internal variant before 
> exploring potential public API?

This is certainly one alternative.

-

PR Comment: https://git.openjdk.org/jdk/pull/15530#issuecomment-1702421871

Re: RFR: 8315454: Add an immutable BitSet

[Per Minborg]

On Fri, 1 Sep 2023 08:50:42 GMT, Chen Liang  wrote:

> What about the missing functionalities, such as ranged get, cardinality, 
> next/previousSet/ClearBit?

This was explored in https://github.com/openjdk/jdk/pull/15493

-

PR Comment: https://git.openjdk.org/jdk/pull/15530#issuecomment-1702419694

Re: RFR: 8315454: Add an immutable BitSet

[Hannes Greule]

On Fri, 1 Sep 2023 08:21:13 GMT, Per Minborg  wrote:

> This PR proposes adding a new method to BitSet that provides an immutable 
> snapshot of the set in the form of an `IntPredicate`.
> 
> The predicate is eligible for constant folding.
> 
> Here are some classes in the JDK that would benefit directly from 
> constant-folding of BitSets:
> 
> PoolReader (6)
> URLEncoder (1) - Updated in this PR
> HtmlTree (2)
> 
> More over, the implementation of the predicate is @ValueBased and this would 
> provide additional benefits in the future.
> 
> Initial benchmarks with the URLEncoder show encouraging results:
> 
> 
> Name   (encodeChars) (maxLength) (unchanged) Cnt  
> Base   Error   Test   Error  Unit  Diff%
> URLEncodeDecode.testEncodeUTF8 61024   0  15 
> 2,371 ± 0,016  2,073 ± 0,184 ms/op  12,6% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024  75  15 
> 1,772 ± 0,013  1,387 ± 0,032 ms/op  21,8% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024 100  15 
> 1,230 ± 0,009  1,140 ± 0,011 ms/op   7,3% (p = 0,000*)

Maybe it would make sense to start with a JDK-internal variant before exploring 
potential public API?

-

PR Comment: https://git.openjdk.org/jdk/pull/15530#issuecomment-1702412714

Re: RFR: 8315454: Add an immutable BitSet

[Chen Liang]

On Fri, 1 Sep 2023 08:21:13 GMT, Per Minborg  wrote:

> This PR proposes adding a new method to BitSet that provides an immutable 
> snapshot of the set in the form of an `IntPredicate`.
> 
> The predicate is eligible for constant folding.
> 
> Here are some classes in the JDK that would benefit directly from 
> constant-folding of BitSets:
> 
> PoolReader (6)
> URLEncoder (1) - Updated in this PR
> HtmlTree (2)
> 
> More over, the implementation of the predicate is @ValueBased and this would 
> provide additional benefits in the future.
> 
> Initial benchmarks with the URLEncoder show encouraging results:
> 
> 
> Name   (encodeChars) (maxLength) (unchanged) Cnt  
> Base   Error   Test   Error  Unit  Diff%
> URLEncodeDecode.testEncodeUTF8 61024   0  15 
> 2,371 ± 0,016  2,073 ± 0,184 ms/op  12,6% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024  75  15 
> 1,772 ± 0,013  1,387 ± 0,032 ms/op  21,8% (p = 0,000*)
> URLEncodeDecode.testEncodeUTF8 61024 100  15 
> 1,230 ± 0,009  1,140 ± 0,011 ms/op   7,3% (p = 0,000*)

What about the missing functionalities, such as ranged get, cardinality, 
next/previousSet/ClearBit?

-

PR Comment: https://git.openjdk.org/jdk/pull/15530#issuecomment-1702400105

Re: RFR: 8313983: jmod create --target-platform should replace existing ModuleTarget attribute

[Adam Sotona]

On Thu, 31 Aug 2023 16:47:07 GMT, Mandy Chung  wrote:

> Would you consider documenting in the javadoc of XXXAttribute in the 
> ClassFile API if it allows multiple? It will make clear to the readers.

Yes, I'll add that information as a part of javadoc update #14968.
Thanks!

-

PR Comment: https://git.openjdk.org/jdk/pull/15514#issuecomment-1702375563

Re: RFR: 8314094: java/lang/ProcessHandle/InfoTest.java fails on Windows when run as user with Administrator privileges [v2]

[Christoph Langer]

> On Windows, the test java/lang/ProcessHandle/InfoTest.java can fail when run 
> as user that is member of the Administrators group. In that case new files 
> are not owned by the user but instead by BUILTIN\ADMINISTRATORS. This breaks 
> the assumptions of the test's whoami check. My suggestion is to cater for 
> this case and don't fail the test but write a warning message to stdout that 
> a whoami check is not correctly possible.

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 three additional 
commits since the last revision:

 - Use System.getProperty(user.name) for the Windows Adminstrators case
 - Merge branch 'master' into JDK-8314094
 - JDK-8314094

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/15222/files
  - new: https://git.openjdk.org/jdk/pull/15222/files/4e5cbf82..db6d3d14

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=15222=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=15222=00-01

  Stats: 27267 lines in 894 files changed: 17633 ins; 3604 del; 6030 mod
  Patch: https://git.openjdk.org/jdk/pull/15222.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15222/head:pull/15222

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

Re: RFR: 8314094: java/lang/ProcessHandle/InfoTest.java fails on Windows when run as user with Administrator privileges [v2]

[Christoph Langer]

On Thu, 31 Aug 2023 15:08:34 GMT, Roger Riggs  wrote:

>> The problem with the environment variables is, that jtreg only passes very 
>> few of them down to the testee process  -  USERDOMAIN and USERNAME are not 
>> part of these as far as I know.
>
> ok, more overhead than value in the suggestion. 
> If its windows, strip off the domain (before "/") and compare.
> If that doesn't work then just drop the username compare on windows.

After verifying that System.getenv yields empty results for USERDOMAIN and 
USERNAME, I updated the change to use System.getProperty("user.name") in the 
Windows Administrators case. Let's see how testing goes.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/15222#discussion_r1312738642

Re: RFR: 8315004: Runtime.halt() debug logging

[Alan Bateman]

On Fri, 25 Aug 2023 09:49:20 GMT, Alan Bateman  wrote:

>> I want to add a log output similar to JDK-8301627 to Runtime.halt().
>> To avoid double logging of Runtime.exit(), add a flag to indicate whether 
>> logging was done, and fix it so that logging is done only once.
>> Could someone please review this fix?
>
> I think you may have missed the comment in the JBS issue. Logging means 
> running potentially arbitrary code, doing this at Runtime.halt time is 
> problematic. I thought the conclusion from the work on Runtime.exit was not 
> to log in Runtime.halt?

> @AlanBateman Sorry for missing your comment on JBS. I can't find any 
> discussion of the need for logs in Runtime.halt in JDK-8301627, so I'm not 
> sure if it was intentional that no logging output was added to Runtime.halt.
> However, if Runtime.halt is overlooked without discussion, I think it should 
> be added after considering the need.
> I think it's the same problem as Runtime.exit when it comes to executing 
> arbitrary code.
> Are there any issues specific to Runtime.halt?

The purpose of the halt method is to terminate immediately, it doesn't initiate 
the shutdown sequence. My recollection of the System.exit logging is that it 
was deliberate to not change the halt method. Changing halt to log means it 
would not terminate immediately. Also I think there were concerns with logging 
libraries that needed the shutdown hook to execute in order to fush/close logs. 
@RogerRiggs or @stuart-marks may be able to say more about this.

-

PR Comment: https://git.openjdk.org/jdk/pull/15426#issuecomment-1702371738

RFR: 8315454: Add an immutable BitSet

[Per Minborg]

This PR proposes adding a new method to BitSet that provides an immutable 
snapshot of the set in the form of an `IntPredicate`.

The predicate is eligible for constant folding.

Here are some classes in the JDK that would benefit directly from 
constant-folding of BitSets:

PoolReader (6)
URLEncoder (1) - Updated in this PR
HtmlTree (2)

More over, the implementation of the predicate is @ValueBased and this would 
provide additional benefits in the future.

Initial benchmarks with the URLEncoder show encouraging results:


Name   (encodeChars) (maxLength) (unchanged) Cnt  Base  
 Error   Test   Error  Unit  Diff%
URLEncodeDecode.testEncodeUTF8 61024   0  15 2,371 
± 0,016  2,073 ± 0,184 ms/op  12,6% (p = 0,000*)
URLEncodeDecode.testEncodeUTF8 61024  75  15 1,772 
± 0,013  1,387 ± 0,032 ms/op  21,8% (p = 0,000*)
URLEncodeDecode.testEncodeUTF8 61024 100  15 1,230 
± 0,009  1,140 ± 0,011 ms/op   7,3% (p = 0,000*)

-

Commit messages:
 - Add test and remove updates for some JDK classes

Changes: https://git.openjdk.org/jdk/pull/15530/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=15530=00
  Issue: https://bugs.openjdk.org/browse/JDK-8315454
  Stats: 153 lines in 3 files changed: 140 ins; 1 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/15530.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15530/head:pull/15530

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

Integrated: 8313983: jmod create --target-platform should replace existing ModuleTarget attribute

[Adam Sotona]

On Thu, 31 Aug 2023 12:13:40 GMT, Adam Sotona  wrote:

> ModuleTarget and ModuleResolution attributes were flagged as 'allow multiple' 
> in the Classfile API.
> This patch removed the flags and allows at most one instance of each 
> attribute.
> 
> Please review.
> 
> Thanks,
> Adam

This pull request has now been integrated.

Changeset: c2e01eba
Author:Adam Sotona 
URL:   
https://git.openjdk.org/jdk/commit/c2e01eba5a537acd573b7d2e6d41811c415c3f68
Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod

8313983: jmod create --target-platform should replace existing ModuleTarget 
attribute

Reviewed-by: alanb, mchung

-

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

Re: RFR: 8315004: Runtime.halt() debug logging

[Masanori Yano]

On Fri, 25 Aug 2023 09:49:20 GMT, Alan Bateman  wrote:

>> I want to add a log output similar to JDK-8301627 to Runtime.halt().
>> To avoid double logging of Runtime.exit(), add a flag to indicate whether 
>> logging was done, and fix it so that logging is done only once.
>> Could someone please review this fix?
>
> I think you may have missed the comment in the JBS issue. Logging means 
> running potentially arbitrary code, doing this at Runtime.halt time is 
> problematic. I thought the conclusion from the work on Runtime.exit was not 
> to log in Runtime.halt?

@AlanBateman Sorry for missing your comment on JBS. I can't find any discussion 
of the need for logs in Runtime.halt in JDK-8301627, so I'm not sure if it was 
intentional that no logging output was added to Runtime.halt.
However, if Runtime.halt is overlooked without discussion, I think it should be 
added after considering the need.
I think it's the same problem as Runtime.exit when it comes to executing 
arbitrary code.
Are there any issues specific to Runtime.halt?

-

PR Comment: https://git.openjdk.org/jdk/pull/15426#issuecomment-1702358854

Re: RFR: 8268829: Provide an optimized way to walk the stack with Class object only [v9]

[Alan Bateman]

On Thu, 31 Aug 2023 17:09:40 GMT, Mandy Chung  wrote:

>> 8268829: Provide an optimized way to walk the stack with Class object only
>> 
>> `StackWalker::walk` creates one `StackFrame` per frame and the current 
>> implementation
>> allocates one `StackFrameInfo` and one `MemberName` objects per frame. Some 
>> frameworks
>> like logging may only interest in the Class object but not the method name 
>> nor the BCI,
>> for example, filters out its implementation classes to find the caller 
>> class.  It's
>> similar to `StackWalker::getCallerClass` but allows a predicate to filter 
>> out the element.
>> 
>> This PR proposes to add `Option::DROP_METHOD_INFO` enum that requests to 
>> drop the method information.  If no method information is needed, a 
>> `StackWalker` with `DROP_METHOD_INFO`
>> can be used instead and such stack walker will save the overhead of 
>> extracting the method information
>> and the memory used for the stack walking.
>> 
>> New factory methods to take a parameter to specify the kind of stack walker 
>> to be created are defined.
>> This provides a simple way for existing code, for example logging 
>> frameworks, to take advantage of
>> this enhancement with the least change as it can keep the existing function 
>> for traversing
>> `StackFrame`s.
>> 
>> For example: to find the first caller filtering a known list of 
>> implementation class,
>> existing code can create a stack walker instance with `DROP_METHOD_INFO` 
>> option:
>> 
>> 
>>  StackWalker walker = StackWalker.getInstance(Option.DROP_METHOD_INFO, 
>> Option.RETAIN_CLASS_REFERENCE);
>>  Optional> callerClass = walker.walk(s ->
>>  s.map(StackFrame::getDeclaringClass)
>>   .filter(Predicate.not(implClasses::contains))
>>   .findFirst());
>> 
>> 
>> If method information is accessed on the `StackFrame`s produced by this 
>> stack walker such as
>> `StackFrame::getMethodName`, then `UnsupportedOperationException` will be 
>> thrown.
>> 
>>  Javadoc & specdiff
>> 
>> https://cr.openjdk.org/~mchung/api/java.base/java/lang/StackWalker.html
>> https://cr.openjdk.org/~mchung/jdk22/specdiff/overview-summary.html
>> 
>>  Alternatives Considered
>> One alternative is to provide a new API:
>> ` T walkClass(Function, ? extends T> function)`
>> 
>> In this case, the caller would need to pass a function that takes a stream
>> of `Class` object instead of `StackFrame`.  Existing code would have to
>> modify calls to the `walk` method to `walkClass` and the function body.
>> 
>> ### Implementation Details
>> 
>> A `StackWalker` configured with `DROP_METHOD_INFO` ...
>
> Mandy Chung has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 29 commits:
> 
>  - Merge
>  - Remove the new getInstance method taking varargs
>  - update mode to be int rather than long
>  - update tests
>  - Review feedback on javadoc
>  - Revised the API change.  Add Option::DROP_METHOD_INFO
>  - Review feedback from Remi
>  - fixup javadoc
>  - Review feedback: move JLIA to ClassFrameInfo
>  - review feedback and javadoc clean up
>  - ... and 19 more: https://git.openjdk.org/jdk/compare/c8acab1d...111661bc

The API changes in the the current update (111661bc) look good.

-

PR Comment: https://git.openjdk.org/jdk/pull/15370#issuecomment-1702328042

RFR: JDK-8314121: test tools/jpackage/share/RuntimePackageTest.java#id0 fails on RHEL8

[Matthias Baesken]

on some RHEL Linux 8.X machines , we run into errors in test 
tools/jpackage/share/RuntimePackageTest.java#id0 , error can be seen below.
It looks like these errors occur when running the jtreg tests with higher 
concurrency, I did not see them when running just the single test.

When adding some test output , we see the 2 top install dirs below (1 is 
expected, not 2) :
./test/unpacked-rpm/opt
./test/unpacked-rpm/usr

error output :

java.lang.AssertionError: Expected [1]. Actual [2]: Check the package has 1 top 
installation directories
   at jdk.jpackage.test.TKit.error(TKit.java:273)
   at jdk.jpackage.test.TKit.assertEquals(TKit.java:576)
   at 
jdk.jpackage.test.PackageTest$Handler.verifyRootCountInUnpackedPackage(PackageTest.java:705)
   at 
jdk.jpackage.test.PackageTest$Handler.lambda$verifyPackageInstalled$3(PackageTest.java:635)
   at java.base/java.util.Optional.ifPresent(Optional.java:178)
   at 
jdk.jpackage.test.PackageTest$Handler.verifyPackageInstalled(PackageTest.java:633)
   at jdk.jpackage.test.PackageTest$Handler.accept(PackageTest.java:592)
   at jdk.jpackage.test.PackageTest$2.accept(PackageTest.java:504)
   at jdk.jpackage.test.PackageTest$2.accept(PackageTest.java:411)
   at 
jdk.jpackage.test.Functional$ThrowingConsumer.lambda$toConsumer$0(Functional.java:41)
   at java.base/java.lang.Iterable.forEach(Iterable.java:75)
   at 
jdk.jpackage.test.PackageTest.lambda$runActions$24(PackageTest.java:381)
   at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
   at 
jdk.jpackage.test.PackageTest.lambda$runActions$25(PackageTest.java:380)
   at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
   at jdk.jpackage.test.PackageTest.runActions(PackageTest.java:379)
   at jdk.jpackage.test.RunnablePackageTest.run(RunnablePackageTest.java:58)
   at RuntimePackageTest.test(RuntimePackageTest.java:83)
   at 
java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
   at java.base/java.lang.reflect.Method.invoke(Method.java:580)
   at jdk.jpackage.test.MethodCall.accept(MethodCall.java:145)
   at jdk.jpackage.test.TestInstance.run(TestInstance.java:230)
   at jdk.jpackage.test.TKit.lambda$ignoreExceptions$5(TKit.java:141)
   at jdk.jpackage.test.TKit.lambda$runTests$3(TKit.java:126)
   at 
java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
   at 
java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762)
   at jdk.jpackage.test.TKit.lambda$runTests$4(TKit.java:123)
   at 
jdk.jpackage.test.Functional$ThrowingRunnable.lambda$toRunnable$0(Functional.java:105)
   at jdk.jpackage.test.TKit.withExtraLogStream(TKit.java:109)
   at jdk.jpackage.test.TKit.runTests(TKit.java:122)
   at jdk.jpackage.test.Main.runTests(Main.java:79)
   at jdk.jpackage.test.Main.lambda$main$2(Main.java:75)
   at 
jdk.jpackage.test.Functional$ThrowingRunnable.lambda$toRunnable$0(Functional.java:105)
   at jdk.jpackage.test.TKit.withExtraLogStream(TKit.java:109)
   at jdk.jpackage.test.Main.main(Main.java:75)
   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:1570)

Looks like the additional dir /usr/lib/.build-id  is coming from automatically 
adding /usr/lib/.build-id to package; this seems to be a feature of the 
rpmbuild according to https://bugzilla.redhat.com/show_bug.cgi?id=1724153   and 
must be configured in template.spec if it is unwanted.

-

Commit messages:
 - JDK-8314121

Changes: https://git.openjdk.org/jdk/pull/15528/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=15528=00
  Issue: https://bugs.openjdk.org/browse/JDK-8314121
  Stats: 8 lines in 2 files changed: 7 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/15528.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15528/head:pull/15528

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

Re: RFR: 8315383: jlink SystemModulesPlugin incorrectly parses the options [v2]

[Christoph]

On Wed, 30 Aug 2023 19:30:37 GMT, Mandy Chung  wrote:

>> Oliver Kopp has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove obsolete imports
>
> Looks good.   Thanks for catching this.
> 
> There are a few unused imports in JLinkDedupTestBatchSizeOne.java.  Can you 
> remove them as you are in that file?

@mlchung Could you please do the backport for us? We don't have the rights. 
Thanks in advance!

-

PR Comment: https://git.openjdk.org/jdk/pull/15495#issuecomment-1702268892

Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v9]

[Alan Bateman]

On Thu, 31 Aug 2023 14:29:41 GMT, iaroslavski  wrote:

> Alan, you mentioned that DualPivotQuicksort will need detailed review. Can we 
> go ahead and start reviewing? Laurent checked performance, JMH results look 
> fine.

As before, I think the main question with this change is whether adding radix 
sort to the mix is worth the complexity and additional code to maintain. Also 
as we discussed in the previous PR, the additional memory needed for the radix 
sort may have an effect on other things that are going on concurrently. I know 
it has been updated to handle OOME but I think potential reviewers would need 
to be comfortable with that part.

-

PR Comment: https://git.openjdk.org/jdk/pull/13568#issuecomment-1702220527