Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]

2024-06-10 Thread Per Minborg
On Fri, 17 May 2024 09:31:33 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops/us
>> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
>> 704.893  ops/us
>> StableListElementBenchmark...
>
> Per Minborg has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add benchmarks for memoized IntFunction and Function
>  - Add benchmark for memoized supplier

A new PR will be made available shortly.

-

PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2158140833


Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]

2024-05-21 Thread Chen Liang
On Fri, 17 May 2024 09:31:33 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops/us
>> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
>> 704.893  ops/us
>> StableListElementBenchmark...
>
> Per Minborg has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add benchmarks for memoized IntFunction and Function
>  - Add benchmark for memoized supplier

Thanks for the insights. Also, I wonder what is a good amount of metadata you 
are considering, as original stable values only take one possible 
representation out of all to indicate a mutable state, much lighter in weight 
compared to the current implementation, which takes many bits; this might 
disencourage StableValue use.

-

PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2122714875


Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]

2024-05-21 Thread Per Minborg
On Fri, 17 May 2024 09:31:33 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops/us
>> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
>> 704.893  ops/us
>> StableListElementBenchmark...
>
> Per Minborg has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add benchmarks for memoized IntFunction and Function
>  - Add benchmark for memoized supplier

We are considering another implementation with less complexity. So, for now, 
thank you for all the feedback so far. We will try to make sure to carry over 
them to a new PR.

-

PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2122569580


Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]

2024-05-20 Thread Chen Liang
On Mon, 20 May 2024 09:50:17 GMT, Jens Lidestrom  wrote:

>> Per Minborg has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Add benchmarks for memoized IntFunction and Function
>>  - Add benchmark for memoized supplier
>
> src/java.base/share/classes/jdk/internal/lang/StableArray.java line 66:
> 
>> 64:  * @throws IllegalArgumentException if the provided {@code length} 
>> is {@code < 0}
>> 65:  */
>> 66: static  StableArray of(int length) {
> 
> I interpret the method name `of` as a method that creates an object that 
> contains the argument as some kind of member, in the way that `List.of` and 
> friends work.
> 
> My intuitive interpretation of `StableArray.of(10)` is that it returns an 
> array with the single element 10.
> 
> I think a method like this should be named `empty`, or `emptyOfLength` or 
> something like that.

Stable arrays aren't supposed to be initialized with values, so I think your 
point is moot.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1606708916


Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]

2024-05-20 Thread Jens Lidestrom
On Fri, 17 May 2024 09:31:33 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> > src="https://github.com/openjdk/jdk/assets/7457876/db4b22a1-af87-4914-adac-b05a87e7cb42;
>>  width=20% height=20%>
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops...
>
> Per Minborg has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add benchmarks for memoized IntFunction and Function
>  - Add benchmark for memoized supplier

src/java.base/share/classes/jdk/internal/lang/StableArray.java line 66:

> 64:  * @throws IllegalArgumentException if the provided {@code length} is 
> {@code < 0}
> 65:  */
> 66: static  StableArray of(int length) {

I interpret the method name `of` as a method that creates an object that 
contains the argument as some kind of member, in the way that `List.of` and 
friends work.

My intuitive interpretation of `StableArray.of(10)` is that it returns an array 
with the single element 10.

I think a method like this should be named `empty`, or `emptyOfLength` or 
something like that.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1606529054


Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]

2024-05-17 Thread Per Minborg
On Fri, 17 May 2024 09:31:33 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops/us
>> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
>> 704.893  ops/us
>> StableListElementBenchmark...
>
> Per Minborg has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add benchmarks for memoized IntFunction and Function
>  - Add benchmark for memoized supplier

Here are some results of a recently added benchmark that uses a memorized 
function (with 0 and 1 as input values):

![image](https://github.com/openjdk/jdk/assets/7457876/f2fd5b5a-ac89-483b-acb5-bc5de215417a)

See 
[test/micro/org/openjdk/bench/java/lang/stable/MemoizedFunctionBenchmark.java 
for 
details](https://github.com/minborg/jdk/blob/stable-value/test/micro/org/openjdk/bench/java/lang/stable/MemoizedFunctionBenchmark.java)

-

PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2117143526


Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]

2024-05-17 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

 - Add benchmarks for memoized IntFunction and Function
 - Add benchmark for memoized supplier

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/dd0ceaf0..7beab36a

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

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

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v19]

2024-05-17 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Update ofList and ofMap docs

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/0f798a70..dd0ceaf0

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

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

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v18]

2024-05-17 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Simplify StableList

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/35c9252d..0f798a70

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

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

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v17]

2024-05-17 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Declare field final

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/ec7c92cd..35c9252d

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

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

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v16]

2024-05-17 Thread Per Minborg
On Thu, 16 May 2024 12:48:24 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops/us
>> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
>> 704.893  ops/us
>> StableListElementBenchmark...
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix copyringht issues

> _Mailing list message from [Olexandr 
> Rotan](mailto:rotanolexandr...@gmail.com) on 
> [compiler-dev](mailto:compiler-...@mail.openjdk.org):_
> 
> Is it possible to make stable values and collections Serializable? I see 
> various applications for this feature in entity classes as a way to preserve 
> immutability of entity fields and at the same time not break current JPA 
> specifications. It is a *very* common task in commercial development. Current 
> workarounds lack either thread safety or performance, and this looks like a 
> really good solution for both of those problems. However, unless StableValue 
> is serializable, it is really unlikely that JPA will adopt them (we have 
> precedent with Optional).
> 
> On Thu, May 16, 2024 at 5:07?PM Per Minborg  wrote:
> 
> -- next part -- An HTML attachment was scrubbed... 
> URL: 
> 

`Serializable` is on the list to explore.

-

PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2116840097


Re: RFR: 8330465: Stable Values and Collections (Internal) [v16]

2024-05-16 Thread Olexandr Rotan
Is it possible to make stable values and collections Serializable? I see
various applications for this feature in entity classes as a way to
preserve immutability of entity fields and at the same time not break
current JPA specifications. It is a *very* common task in commercial
development. Current workarounds lack either thread safety or performance,
and this looks like a really good solution for both of those problems.
However, unless StableValue is serializable, it is really unlikely that JPA
will adopt them (we have precedent with Optional).

On Thu, May 16, 2024 at 5:07 PM Per Minborg  wrote:

> On Thu, 16 May 2024 12:48:24 GMT, Per Minborg 
> wrote:
>
> >> # Stable Values & Collections (Internal)
> >>
> >> ## Summary
> >> This PR proposes to introduce an internal _Stable Values & Collections_
> API, which provides immutable value holders where elements are initialized
> _at most once_. Stable Values & Collections offer the performance and
> safety benefits of final fields while offering greater flexibility as to
> the timing of initialization.
> >>
> >> ## Goals
> >>  * Provide an easy and intuitive API to describe value holders that can
> change at most once.
> >>  * Decouple declaration from initialization without significant
> footprint or performance penalties.
> >>  * Reduce the amount of static initializer and/or field initialization
> code.
> >>  * Uphold integrity and consistency, even in a multi-threaded
> environment.
> >>
> >> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> >>
> >> ## Performance
> >> Performance compared to instance variables using two `AtomicReference`
> and two protected by double-checked locking under concurrent access by all
> threads:
> >>
> >>
> >> Benchmark  Mode  Cnt  Score
>   Error   Units
> >> StableBenchmark.atomicthrpt   10259.478 ?
>  36.809  ops/us
> >> StableBenchmark.dcl   thrpt   10225.710 ?
>  26.638  ops/us
> >> StableBenchmark.stablethrpt   10   4382.478 ?
> 1151.472  ops/us <- StableValue significantly faster
> >>
> >>
> >> Performance compared to static variables protected by
> `AtomicReference`, class-holder idiom holder, and double-checked locking
> (all threads):
> >>
> >>
> >> Benchmark  Mode  Cnt  Score
>   Error   Units
> >> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?
> 385.639  ops/us
> >> StableStaticBenchmark.dcl thrpt   10   6605.239 ?
> 210.610  ops/us
> >> StableStaticBenchmark.stable  thrpt   10  14338.239 ?
> 1426.874  ops/us
> >> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ?
> 1839.651  ops/us
> >>
> >>
> >> Performance for stable lists (thread safe) in both instance and static
> contexts whereby we access a single value compared to `ArrayList` instances
> (which are not thread-safe) (all threads):
> >>
> >>
> >> Benchmark  Mode  Cnt  Score
>   Error   Units
> >> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ?
> 1169.730  ops/us
> >> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?
> 704.893  ops/us
> >> StableListElementBenchmark...
> >
> > Per Minborg has updated the pull request incrementally with one
> additional commit since the last revision:
> >
> >   Fix copyringht issues
>
> Here are some updated benchmark graphs where we sum two instance variables
> of different sorts (higher is better):
>
> ![image](
> https://github.com/openjdk/jdk/assets/7457876/d82561d6-e803-4345-b6d2-6b0402e60211
> )
>
> -
>
> PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2115343828
>


Re: RFR: 8330465: Stable Values and Collections (Internal) [v16]

2024-05-16 Thread Per Minborg
On Thu, 16 May 2024 12:48:24 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops/us
>> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
>> 704.893  ops/us
>> StableListElementBenchmark...
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix copyringht issues

Here are some updated benchmark graphs where we sum two instance variables of 
different sorts (higher is better):

![image](https://github.com/openjdk/jdk/assets/7457876/d82561d6-e803-4345-b6d2-6b0402e60211)

-

PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2115343828


Re: RFR: 8330465: Stable Values and Collections (Internal) [v16]

2024-05-16 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Fix copyringht issues

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/2af0168e..ec7c92cd

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

  Stats: 206 lines in 13 files changed: 200 ins; 1 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/18794.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18794/head:pull/18794

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v15]

2024-05-16 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Simplify background thread handling

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/3e1ab5e9..2af0168e

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

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

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v14]

2024-05-16 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Improve toString and simplify offset calculations

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/dbbefea5..3e1ab5e9

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

  Stats: 51 lines in 2 files changed: 18 ins; 27 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/18794.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18794/head:pull/18794

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v13]

2024-05-16 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

 - Cleanup switch rakes
 - Update null benchmarks

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/b40ebfad..dbbefea5

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

  Stats: 19 lines in 4 files changed: 3 ins; 6 del; 10 mod
  Patch: https://git.openjdk.org/jdk/pull/18794.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18794/head:pull/18794

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Per Minborg
On Thu, 16 May 2024 11:14:16 GMT, Chen Liang  wrote:

>> The idea here is to have the most likely value in the middle... Not sure if 
>> that motivates the added complexity though.
>
> Is there any refernce on how/why the middle entry in a tableswitch 
> instruction is the fastest?

It is only in a _lookupswitch_ that this becomes relevant. The above code will 
generate a *tableswitch* so I think it is safe to simplify the code and remove 
the DUMMY.


  private V orThrowVolatile();
descriptor: ()Ljava/lang/Object;
flags: (0x0002) ACC_PRIVATE
Code:
  stack=1, locals=1, args_size=1
 0: aload_0
 1: invokevirtual #15 // Method stateVolatile:()I
 4: tableswitch   { // 0 to 4
   0: 40
   1: 44
   2: 46
   3: 51
   4: 56
 default: 60
}
...

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1603214742


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Per Minborg
On Thu, 16 May 2024 11:13:24 GMT, Chen Liang  wrote:

>> It seems reasonable to assume `null` values are not constant-folded. For 
>> straight-out-of-the-box usage, there is no apparent significant difference 
>> as indicated by a new benchmark I just added:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  Error   
>> Units
>> StableStaticBenchmark.atomic  thrpt   10   5729.683 ?  502.023  
>> ops/us
>> StableStaticBenchmark.dcl thrpt   10   6069.222 ?  951.784  
>> ops/us
>> StableStaticBenchmark.dclHolder   thrpt   10   5502.102 ? 1630.627  
>> ops/us
>> StableStaticBenchmark.stable  thrpt   10  12737.158 ? 1746.456  
>> ops/us <- Non-null benchmark
>> StableStaticBenchmark.stableHolderthrpt   10  12053.978 ? 1421.527  
>> ops/us
>> StableStaticBenchmark.stableList  thrpt   10  12443.870 ? 2084.607  
>> ops/us
>> StableStaticBenchmark.stableNull  thrpt   10  13164.232 ?  591.284  
>> ops/us <- Added null benchmark
>> StableStaticBenchmark.stableRecordHolder  thrpt   10  13638.893 ? 1250.895  
>> ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13639.220 ? 1190.922  
>> ops/us
>> 
>> 
>> If the `null` value participates in a much larger constant-folding tree, 
>> there might be a significant difference. I am afraid moving the order would 
>> have detrimental effects on instance performance:
>> 
>> Checking value first:
>> 
>> 
>> Benchmark   Mode  Cnt Score  Error   Units
>> StableBenchmark.atomic thrpt   10   246.460 ?   75.417  ops/us
>> StableBenchmark.dclthrpt   10   243.481 ?   35.021  ops/us
>> StableBenchmark.stable thrpt   10  4977.693 ?  675.926  ops/us  
>> <- Non-null
>> StableBenchmark.stableHoldingList  thrpt   10  3614.460 ?  275.140  ops/us
>> StableBenchmark.stableList thrpt   10  3328.155 ?  898.202  ops/us
>> StableBenchmark.stableListStored   thrpt   10  3842.174 ?  535.902  ops/us
>> StableBenchmark.stableNull thrpt   10  6217.737 ?  840.376  ops/us 
>> <- null
>> StableBenchmark.supplier   thrpt   10  9369.934 ? 1449.182  ops/us
>> 
>> 
>> Checking null first:
>> 
>> 
>> Benchmark   Mode  Cnt Score  Error   Units
>> StableBenchmark.atomic thrpt   10   287.640 ?   17.858  ops/us
>> StableBenchmark.dclthrpt   10   250.398 ?   20.874  ops/us
>> StableBenchmark.stable thrpt   10  3745.885 ? 1040.534  ops/us 
>> <- Non-null
>> StableBenchmark.stableHoldingList  thrpt   10  2982.129 ?  503.492  ops/us
>> StableBenchmar...
>
> I think the result would be more convincing if the stable case is changed to 
> `sum += (stable.orThrow() == null ? 0 : 1) + (stable2.orThrow() == null ? 0 : 
> 1);` as adding by 1 might be somewhat better optimized by JIT.

I have instead changed parts of the `stableNull()` body to:


sum += (stableNull.orThrow() == null ? VALUE : VALUE2) + (stableNull2.orThrow() 
== null ? VALUE : VALUE2);

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1603205135


Re: RFR: 8330465: Stable Values and Collections (Internal) [v6]

2024-05-16 Thread Chen Liang
On Thu, 16 May 2024 07:29:21 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops/us
>> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
>> 704.893  ops/us
>> StableListElementBenchmark...
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Simplify exception handling and add benchmarks

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 
400:

> 398: @Override
> 399: public void run() {
> 400: stable.computeIfUnset(supplier);

We can just declare this runnable as a capturing lambda (or an anonymous class 
if you fear initialization issues) and leave this comment there. The thread 
field can be removed.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1603177748


Re: RFR: 8330465: Stable Values and Collections (Internal) [v12]

2024-05-16 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Clean up

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/923e1877..b40ebfad

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

  Stats: 64 lines in 3 files changed: 14 ins; 21 del; 29 mod
  Patch: https://git.openjdk.org/jdk/pull/18794.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18794/head:pull/18794

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Chen Liang
On Thu, 16 May 2024 07:11:20 GMT, Per Minborg  wrote:

>> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java 
>> line 139:
>> 
>>> 137: case NON_NULL: { return valueVolatile(); }
>>> 138: case ERROR:{ throw StableUtil.error(this); }
>>> 139: case DUMMY:{ throw shouldNotReachHere(); }
>> 
>> Redundant branch?
>
> The idea here is to have the most likely value in the middle... Not sure if 
> that motivates the added complexity though.

Is there any refernce on how/why the middle entry in a tableswitch instruction 
is the fastest?

>> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java 
>> line 236:
>> 
>>> 234: } catch (Throwable t) {
>>> 235: putState(ERROR);
>>> 236: putMutex(t.getClass());
>> 
>> Should we cache the exception instance so we can rethrow it in future ERROR 
>> state `orThrow` calls?
>
> We considered recording the entire exception instance but for security 
> reasons, we ended up just recording the type of exception. I will add a 
> comment explaining this in the code.

Thanks for this clarification. Makes sense.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1603149806
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1603150592


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Chen Liang
On Thu, 16 May 2024 06:54:26 GMT, Per Minborg  wrote:

>> Maybe the `state == NULL` check should be moved before `v != null`, as the 
>> **JIT** doesn’t constant‑fold `null` [`@Stable`] values:
>> https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L41-L44
>>   
>> https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L64-L71
>> 
>> [`@Stable`]: 
>> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java
>
> It seems reasonable to assume `null` values are not constant-folded. For 
> straight-out-of-the-box usage, there is no apparent significant difference as 
> indicated by a new benchmark I just added:
> 
> 
> Benchmark  Mode  Cnt  Score  Error   
> Units
> StableStaticBenchmark.atomic  thrpt   10   5729.683 ?  502.023  
> ops/us
> StableStaticBenchmark.dcl thrpt   10   6069.222 ?  951.784  
> ops/us
> StableStaticBenchmark.dclHolder   thrpt   10   5502.102 ? 1630.627  
> ops/us
> StableStaticBenchmark.stable  thrpt   10  12737.158 ? 1746.456  
> ops/us <- Non-null benchmark
> StableStaticBenchmark.stableHolderthrpt   10  12053.978 ? 1421.527  
> ops/us
> StableStaticBenchmark.stableList  thrpt   10  12443.870 ? 2084.607  
> ops/us
> StableStaticBenchmark.stableNull  thrpt   10  13164.232 ?  591.284  
> ops/us <- Added null benchmark
> StableStaticBenchmark.stableRecordHolder  thrpt   10  13638.893 ? 1250.895  
> ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13639.220 ? 1190.922  
> ops/us
> 
> 
> If the `null` value participates in a much larger constant-folding tree, 
> there might be a significant difference. I am afraid moving the order would 
> have detrimental effects on instance performance:
> 
> Checking value first:
> 
> 
> Benchmark   Mode  Cnt Score  Error   Units
> StableBenchmark.atomic thrpt   10   246.460 ?   75.417  ops/us
> StableBenchmark.dclthrpt   10   243.481 ?   35.021  ops/us
> StableBenchmark.stable thrpt   10  4977.693 ?  675.926  ops/us  
> <- Non-null
> StableBenchmark.stableHoldingList  thrpt   10  3614.460 ?  275.140  ops/us
> StableBenchmark.stableList thrpt   10  3328.155 ?  898.202  ops/us
> StableBenchmark.stableListStored   thrpt   10  3842.174 ?  535.902  ops/us
> StableBenchmark.stableNull thrpt   10  6217.737 ?  840.376  ops/us <- 
> null
> StableBenchmark.supplier   thrpt   10  9369.934 ? 1449.182  ops/us
> 
> 
> Checking null first:
> 
> 
> Benchmark   Mode  Cnt Score  Error   Units
> StableBenchmark.atomic thrpt   10   287.640 ?   17.858  ops/us
> StableBenchmark.dclthrpt   10   250.398 ?   20.874  ops/us
> StableBenchmark.stable thrpt   10  3745.885 ? 1040.534  ops/us <- 
> Non-null
> StableBenchmark.stableHoldingList  thrpt   10  2982.129 ?  503.492  ops/us
> StableBenchmark.stableList thrpt   10  3125.045 ?  416.792  ops/us
> StableBenchmark.sta...

I think the result would be more convincing if the stable case is changed to 
`sum += (stable.orThrow() == null ? 0 : 1) + (stable2.orThrow() == null ? 0 : 
1);` as adding by 1 might be somewhat better optimized by JIT.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1603148915


Re: RFR: 8330465: Stable Values and Collections (Internal) [v11]

2024-05-16 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Update src/java.base/share/classes/jdk/internal/lang/stable/StableAccess.java
  
  Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/80b7e081..923e1877

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

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

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v10]

2024-05-16 Thread ExE Boss
On Thu, 16 May 2024 09:01:22 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops/us
>> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
>> 704.893  ops/us
>> StableListElementBenchmark...
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use byte for storing state and compute flags

src/java.base/share/classes/jdk/internal/lang/stable/StableAccess.java line 63:

> 61:Function extends R> original) {
> 62: return new MemoizedFunction<>(stableMap, original);
> 63: }

Maybe also rename these?
Suggestion:

public static  Supplier memoizedSupplier(StableValue stable,
   Supplier 
original) {
return new MemoizedSupplier<>(stable, original);
}

public static  IntFunction memoizedIntFunction(List> 
stableList,
 IntFunction original) {
return new MemoizedIntFunction<>(stableList, original);
}

public static  Function memoizedFunction(Map> 
stableMap,
 Function original) {
return new MemoizedFunction<>(stableMap, original);
}

src/jdk.unsupported/share/classes/sun/misc/Unsafe.java line 729:

> 727: }
> 728: }
> 729: }

Given that the [`Class​::forName​(String, boolean, ClassLoader)`] method 
doesn’t care about whether the requested class is actually exported to the 
caller, it’s possible to do the following:
Suggestion:

final class Holder {
static final Class TRUSTED_FIELD_TYPE;
static {
PrivilegedAction getPlatformClassLoader = 
ClassLoader::getPlatformClassLoader;
@SuppressWarnings("removal")
ClassLoader platformClassLoader = 
AccessController.doPrivileged(getPlatformClassLoader);

try {
TRUSTED_FIELD_TYPE = 
Class.forName("jdk.internal.lang.stable.TrustedFieldType",
false, platformClassLoader);
} catch (ClassNotFoundException e) {
throw new AssertionError(e);
}
}
}

Class declaringClass = f.getDeclaringClass();
if (declaringClass.isHidden()) {
throw new UnsupportedOperationException("can't get base address on 
a hidden class: " + f);
  

Re: RFR: 8330465: Stable Values and Collections (Internal) [v10]

2024-05-16 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Use byte for storing state and compute flags

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/058cfddf..80b7e081

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

  Stats: 42 lines in 2 files changed: 11 ins; 17 del; 14 mod
  Patch: https://git.openjdk.org/jdk/pull/18794.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18794/head:pull/18794

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Per Minborg
On Wed, 15 May 2024 16:26:57 GMT, Chen Liang  wrote:

>> Per Minborg has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Switch to monomorphic StableValue and use lazy arrays
>
> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java 
> line 75:
> 
>> 73:  */
>> 74: @Stable
>> 75: private int state;
> 
> Can we change this to be a byte, so state and supplying fields can be packed 
> together in 4 bytes in some layouts?

We had `byte` before but converted to `int` as the `byte` will get promoted to 
an `int` anyhow in the code logic. However, one idea would be to go back to 
using a `byte` again and also use a `byte` for the flag of computation 
invocation. This would reduce the footprint for these two fields from 8 bytes 
to 2 bytes.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602800304


Re: RFR: 8330465: Stable Values and Collections (Internal) [v4]

2024-05-16 Thread Per Minborg
On Wed, 15 May 2024 15:49:22 GMT, Chen Liang  wrote:

>> Maybe there is a better home for this?
>
> I don't think we should publish this API; this will soon be phased out by 
> strict final fields (written only before super constructor calls) introduced 
> by Valhalla, as strict final fields are never modifiable and can be safely 
> trusted.

Let's keep it internal.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602804071


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Per Minborg
On Wed, 15 May 2024 16:29:08 GMT, Chen Liang  wrote:

>> Per Minborg has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Switch to monomorphic StableValue and use lazy arrays
>
> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java 
> line 236:
> 
>> 234: } catch (Throwable t) {
>> 235: putState(ERROR);
>> 236: putMutex(t.getClass());
> 
> Should we cache the exception instance so we can rethrow it in future ERROR 
> state `orThrow` calls?

We considered recording the entire exception instance but for security reasons, 
we ended up just recording the type of exception. I will add a comment 
explaining this in the code.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602793832


Re: RFR: 8330465: Stable Values and Collections (Internal) [v9]

2024-05-16 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

 - Add comment on security precaution
 - Share code paths

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/d8875db7..058cfddf

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18794=08
 - incr: https://webrevs.openjdk.org/?repo=jdk=18794=07-08

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

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Per Minborg
On Wed, 15 May 2024 16:31:15 GMT, Chen Liang  wrote:

>> Per Minborg has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Switch to monomorphic StableValue and use lazy arrays
>
> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java 
> line 256:
> 
>> 254: 
>> 255: @ForceInline
>> 256: private  V computeIfUnsetShared(Object provider, K key) {
> 
> Can we let suppliers share this path too, with a null key? I see this path 
> supports suppliers but supplier code path doesn't call this path.

Good suggestion!

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602787248


Re: RFR: 8330465: Stable Values and Collections (Internal) [v8]

2024-05-16 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Rework the way compute invocation is recorded

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/b845c589..d8875db7

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18794=07
 - incr: https://webrevs.openjdk.org/?repo=jdk=18794=06-07

  Stats: 40 lines in 1 file changed: 3 ins; 12 del; 25 mod
  Patch: https://git.openjdk.org/jdk/pull/18794.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18794/head:pull/18794

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Per Minborg
On Wed, 15 May 2024 19:07:26 GMT, Chen Liang  wrote:

>> Yes, according to the `@Stable` annotation’s JavaDoc, this is UB:
>> https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L74-L80
>
> Fyi what usually happens is that if a stable field or similarly 
> constant-folded field is promoted to constant, the constant promotion can 
> happen to any of the previous valid values written.
> 
> MethodHandle optimisitically sets a trusted final field this way:
> https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/java/lang/invoke/MethodHandle.java#L1868-L1870
> 
> Also a similar example in user code targeting older Java releases, before JDK 
> 16's strong encapsulation so that enums could have been added by reflection:
> https://github.com/MinecraftForge/MinecraftForge/issues/3885#issuecomment-355602542

Somehow the `@Stable` annotation sneaked into the `supplying` field. But 
actually keeping it that way and just set the value once would be better.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602781188


Re: RFR: 8330465: Stable Values and Collections (Internal) [v7]

2024-05-16 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Rename memoized factories

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/befb2751..b845c589

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18794=06
 - incr: https://webrevs.openjdk.org/?repo=jdk=18794=05-06

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

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v6]

2024-05-16 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using two `AtomicReference` and 
> two protected by double-checked locking under concurrent access by all 
> threads:
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableBenchmark.atomicthrpt   10259.478 ?   
> 36.809  ops/us
> StableBenchmark.dcl   thrpt   10225.710 ?   
> 26.638  ops/us
> StableBenchmark.stablethrpt   10   4382.478 ? 
> 1151.472  ops/us <- StableValue significantly faster
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
> 385.639  ops/us
> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
> 210.610  ops/us
> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
> 1426.874  ops/us
> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
> 1839.651  ops/us
> 
> 
> Performance for stable lists (thread safe) in both instance and static 
> contexts whereby we access a single value compared to `ArrayList` instances 
> (which are not thread-safe) (all threads):
> 
> 
> Benchmark  Mode  Cnt  Score  
> Error   Units
> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
> 1169.730  ops/us
> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
> 704.893  ops/us
> StableListElementBenchmark.staticArrayListthrpt   10   7614.741 ?  
> 564.777  ops/us
> StableListElementBe...

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

  Simplify exception handling and add benchmarks

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/2b840e06..befb2751

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18794=05
 - incr: https://webrevs.openjdk.org/?repo=jdk=18794=04-05

  Stats: 30 lines in 3 files changed: 19 ins; 6 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/18794.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18794/head:pull/18794

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Per Minborg
On Thu, 16 May 2024 07:19:54 GMT, Per Minborg  wrote:

>> src/java.base/share/classes/jdk/internal/lang/StableValue.java line 384:
>> 
>>> 382:  * @param   the memoized type
>>> 383:  */
>>> 384: static  Supplier ofSupplier(Supplier original) {
>> 
>> `ofSupplier` sounds like this method returns a `StableValue` from a 
>> `Supplier`. I recommend another name, such as `stableSupplier`, 
>> `wrapSupplier`, or `memoize`, to better associate with the method's types.
>
> One alternative would be to expose the types `StableList` and `StableMap`. 
> This would allow detection of these types if declared. This would also allow 
> us to expose the methods `computeIfUnset` as instance methods and remove the 
> somewhat strange static counterparts. I agree strict finals are better in the 
> long run.

We had other names for the memoized factories before but some people did not 
like names like `asMemoized`. 

Maybe `ofSupplier` -> `memoizedSupplier` etc. ?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602754675


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Per Minborg
On Wed, 15 May 2024 16:25:04 GMT, Chen Liang  wrote:

>> Per Minborg has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Switch to monomorphic StableValue and use lazy arrays
>
> src/java.base/share/classes/jdk/internal/lang/StableValue.java line 384:
> 
>> 382:  * @param   the memoized type
>> 383:  */
>> 384: static  Supplier ofSupplier(Supplier original) {
> 
> `ofSupplier` sounds like this method returns a `StableValue` from a 
> `Supplier`. I recommend another name, such as `stableSupplier`, 
> `wrapSupplier`, or `memoize`, to better associate with the method's types.

One alternative would be to expose the types `StableList` and `StableMap`. This 
would allow detection of these types if declared. This would also allow us to 
expose the methods `computeIfUnset` as instance methods and remove the somewhat 
strange static counterparts. I agree strict finals are better in the long run.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602749467


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Per Minborg
On Wed, 15 May 2024 16:19:05 GMT, Chen Liang  wrote:

>> Per Minborg has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Switch to monomorphic StableValue and use lazy arrays
>
> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java 
> line 403:
> 
>> 401: stable.computeIfUnset(supplier);
>> 402: } catch (Throwable throwable) {
>> 403: final Thread.UncaughtExceptionHandler 
>> uncaughtExceptionHandler =
> 
> Does this exception handling differ from the default one for threads? If not, 
> I think we can safely remove this catch block, as all exceptions are just 
> propagated and computeIfUnset doesn't declare any checked exception.

Nice catch. This will reduce complexity:


@Override
public void run() {
stable.computeIfUnset(supplier);
// Exceptions are implicitly captured by the tread's
// uncaught exception handler.
}

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602743924


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Per Minborg
On Wed, 15 May 2024 16:11:56 GMT, Chen Liang  wrote:

>> Per Minborg has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Switch to monomorphic StableValue and use lazy arrays
>
> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java 
> line 139:
> 
>> 137: case NON_NULL: { return valueVolatile(); }
>> 138: case ERROR:{ throw StableUtil.error(this); }
>> 139: case DUMMY:{ throw shouldNotReachHere(); }
> 
> Redundant branch?

The idea here is to have the most likely value in the middle... Not sure if 
that motivates the added complexity though.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602739126


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-16 Thread Per Minborg
On Wed, 15 May 2024 18:45:16 GMT, ExE Boss  wrote:

>> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java 
>> line 104:
>> 
>>> 102: // Optimistically try plain semantics first
>>> 103: final V v = value;
>>> 104: if (v != null) {
>> 
>> If `value == null && state == NULL`, can the path still be constant folded? 
>> I doubt it because `value` in this case may not be promoted to constant.
>
> Maybe the `state == NULL` check should be moved before `v != null`, as the 
> **JIT** doesn’t constant‑fold `null` [`@Stable`] values:
> https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L41-L44
>   
> https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L64-L71
> 
> [`@Stable`]: 
> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java

It seems reasonable to assume `null` values are not constant-folded. For 
straight-out-of-the-box usage, there is no apparent significant difference as 
indicated by a new benchmark I just added:


Benchmark  Mode  Cnt  Score  Error   
Units
StableStaticBenchmark.atomic  thrpt   10   5729.683 ?  502.023  
ops/us
StableStaticBenchmark.dcl thrpt   10   6069.222 ?  951.784  
ops/us
StableStaticBenchmark.dclHolder   thrpt   10   5502.102 ? 1630.627  
ops/us
StableStaticBenchmark.stable  thrpt   10  12737.158 ? 1746.456  
ops/us <- Non-null benchmark
StableStaticBenchmark.stableHolderthrpt   10  12053.978 ? 1421.527  
ops/us
StableStaticBenchmark.stableList  thrpt   10  12443.870 ? 2084.607  
ops/us
StableStaticBenchmark.stableNull  thrpt   10  13164.232 ?  591.284  
ops/us <- Added null benchmark
StableStaticBenchmark.stableRecordHolder  thrpt   10  13638.893 ? 1250.895  
ops/us
StableStaticBenchmark.staticCHI   thrpt   10  13639.220 ? 1190.922  
ops/us


If the `null` value participates in a much larger constant-folding tree, there 
might be a significant difference. I was afraid moving the order would have 
detrimental effects on instance performance but that does not seem to be the 
case:

Checking value first:


Benchmark   Mode  Cnt Score  Error   Units
StableBenchmark.atomic thrpt   10   246.460 ?   75.417  ops/us
StableBenchmark.dclthrpt   10   243.481 ?   35.021  ops/us
StableBenchmark.stable thrpt   10  4977.693 ?  675.926  ops/us  <- 
Non-null
StableBenchmark.stableHoldingList  thrpt   10  3614.460 ?  275.140  ops/us
StableBenchmark.stableList thrpt   10  3328.155 ?  898.202  ops/us
StableBenchmark.stableListStored   thrpt   10  3842.174 ?  535.902  ops/us
StableBenchmark.stableNull thrpt   10  6217.737 ?  840.376  ops/us <- 
null
StableBenchmark.supplier   thrpt   10  9369.934 ? 1449.182  ops/us


Checking null first:


Benchmark   Mode  Cnt Score  Error   Units
StableBenchmark.atomic thrpt   10   275.952 ?   39.480  ops/us
StableBenchmark.dclthrpt   10   252.697 ?   18.645  ops/us
StableBenchmark.stable thrpt   10  5211.552 ?  315.307  ops/us <- 
Non-null
StableBenchmark.stableHoldingList  thrpt   10  3764.202 ?  224.325  ops/us
StableBenchmark.stableList thrpt   10  3689.870 ?  419.858  ops/us
StableBenchmark.stableListStored   thrpt   10  3676.182 ?  938.485  ops/us
StableBenchmark.stableNull thrpt   10  6046.935 ? 1512.391  ops/us <- 
null
StableBenchmark.supplier   thrpt   10  9202.202 ? 1479.950  ops/us


So, swapping order seems to be the right move.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602719002


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-15 Thread Chen Liang
On Wed, 15 May 2024 18:49:49 GMT, ExE Boss  wrote:

>> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java 
>> line 240:
>> 
>>> 238: }
>>> 239: } finally {
>>> 240: supplying = false;
>> 
>> Resetting a stable field is a bad idea. I recommend renaming this to 
>> `supplierCalled` or `supplied` so we never transition this false -> true
>
> Yes, according to the `@Stable` annotation’s JavaDoc, this is UB:
> https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L74-L80

Fyi what usually happens is that if a stable field or similarly constant-folded 
field is promoted to constant, the constant promotion can happen to any of the 
previous valid values written.

MethodHandle optimisitically sets a trusted final field this way:
https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/java/lang/invoke/MethodHandle.java#L1868-L1870

Also a similar example in user code targeting older Java releases, before JDK 
16's strong encapsulation so that enums could have been added by reflection:
https://github.com/MinecraftForge/MinecraftForge/issues/3885#issuecomment-355602542

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602125255


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-15 Thread ExE Boss
On Wed, 15 May 2024 16:10:06 GMT, Chen Liang  wrote:

>> Per Minborg has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Switch to monomorphic StableValue and use lazy arrays
>
> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java 
> line 104:
> 
>> 102: // Optimistically try plain semantics first
>> 103: final V v = value;
>> 104: if (v != null) {
> 
> If `value == null && state == NULL`, can the path still be constant folded? I 
> doubt it because `value` in this case may not be promoted to constant.

Maybe the `state == NULL` check should be moved before `v != null`, as the 
**JIT** doesn’t constant‑fold `null` [`@Stable`] values:
https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L41-L44
https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L64-L71

[`@Stable`]: 
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java

> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java 
> line 240:
> 
>> 238: }
>> 239: } finally {
>> 240: supplying = false;
> 
> Resetting a stable field is a bad idea. I recommend renaming this to 
> `supplierCalled` or `supplied` so we never transition this false -> true

Yes, according to the `@Stable` annotation’s JavaDoc, this is UB:
https://github.com/openjdk/jdk/blob/8a4315f833f3700075d65fae6bc566011c837c07/src/java.base/share/classes/jdk/internal/vm/annotation/Stable.java#L74-L80

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602101301
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602106099


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-15 Thread ExE Boss
On Wed, 15 May 2024 15:27:34 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops/us
>> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
>> 704.893  ops/us
>> StableListElementBenchmark...
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Switch to monomorphic StableValue and use lazy arrays

src/java.base/share/classes/jdk/internal/lang/stable/StableUtil.java line 152:

> 150: StableValueImpl witness = (StableValueImpl)
> 151: Holder.UNSAFE.compareAndExchangeReference(elements, 
> offset, null, stable);
> 152: return witness == null ? stable: witness;

Suggestion:

return witness == null ? stable : witness;

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1602094801


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-15 Thread Chen Liang
On Wed, 15 May 2024 15:27:34 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops/us
>> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
>> 704.893  ops/us
>> StableListElementBenchmark...
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Switch to monomorphic StableValue and use lazy arrays

src/java.base/share/classes/jdk/internal/lang/StableValue.java line 384:

> 382:  * @param   the memoized type
> 383:  */
> 384: static  Supplier ofSupplier(Supplier original) {

`ofSupplier` sounds like this method returns a `StableValue` from a `Supplier`. 
I recommend another name, such as `stableSupplier`, `wrapSupplier`, or 
`memoize`, to better associate with the method's types.

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 
75:

> 73:  */
> 74: @Stable
> 75: private int state;

Can we change this to be a byte, so state and supplying fields can be packed 
together in 4 bytes in some layouts?

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 
104:

> 102: // Optimistically try plain semantics first
> 103: final V v = value;
> 104: if (v != null) {

If `value == null && state == NULL`, can the path still be constant folded? I 
doubt it because `value` in this case may not be promoted to constant.

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 
139:

> 137: case NON_NULL: { return valueVolatile(); }
> 138: case ERROR:{ throw StableUtil.error(this); }
> 139: case DUMMY:{ throw shouldNotReachHere(); }

Redundant branch?

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 
236:

> 234: } catch (Throwable t) {
> 235: putState(ERROR);
> 236: putMutex(t.getClass());

Should we cache the exception instance so we can rethrow it in future ERROR 
state `orThrow` calls?

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 
240:

> 238: }
> 239: } finally {
> 240: supplying = false;

Resetting a stable field is a bad idea. I recommend renaming this to 
`supplierCalled` or `supplied` so we never transition this false -> true

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 
256:

> 254: 
> 255: @ForceInline
> 256: private  V 

Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-15 Thread Chen Liang
On Mon, 6 May 2024 19:31:43 GMT, Per Minborg  wrote:

>> src/java.base/share/classes/jdk/internal/lang/StableArray.java line 25:
>> 
>>> 23:  * @since 23
>>> 24:  */
>>> 25: public sealed interface StableArray
>> 
>> Do we have a use case for StableArray beyond those of StableList?
>
> I am trying to model multi-dimensional arrays that also provide flattening. 
> Let's see if it becomes useful.

I think this StableArray can be used as an explicit field type to block 
reflection modifications and enforce constant-folding; the List interface 
cannot. Though in long term I still believe strict final fields are better.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601931752


Re: RFR: 8330465: Stable Values and Collections (Internal) [v4]

2024-05-15 Thread Chen Liang
On Wed, 15 May 2024 15:20:58 GMT, Per Minborg  wrote:

>> At some point in the future, 'jdk.unsupported' will be removed
>
> Maybe there is a better home for this?

I don't think we should publish this API; this will soon be phased out by 
strict final fields (written only before super constructor calls) introduced by 
Valhalla, as strict final fields are never modifiable and can be safely trusted.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601887921


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-15 Thread Chen Liang
On Tue, 14 May 2024 11:12:39 GMT, Per Minborg  wrote:

>> src/hotspot/share/ci/ciField.cpp line 262:
>> 
>>> 260: const char* stable_array3d_klass_name = 
>>> "jdk/internal/lang/StableArray3D";
>>> 261: 
>>> 262: static bool trust_final_non_static_fields_of_type(Symbol* signature) {
>> 
>> Is there a better way of doing this?
>
> How do we check if the type implements `TrustedFieldType` in C?

Is it possible for us to just look at strict fields from valhalla, so we can 
reliably constant-fold those strict final fields? 
https://cr.openjdk.org/~jrose/jls/constructive-classes.html

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601876058


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-15 Thread Per Minborg
On Wed, 15 May 2024 15:27:34 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops/us
>> StableListElementBenchmark.instanceList   thrpt   10   4818.643 ?  
>> 704.893  ops/us
>> StableListElementBenchmark...
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Switch to monomorphic StableValue and use lazy arrays

I have reworked the stable collections so that we create StableValues on demand 
and store them in a lazily populated backing array. This improved performance 
significantly as well as gave us improved startup times (only one array needs 
to be created upfront). Also, StableValue is now monomorphic. On the flip side 
is the fact that slightly more memory is needed.

-

PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2112886060


Re: RFR: 8330465: Stable Values and Collections (Internal) [v4]

2024-05-15 Thread Per Minborg
On Wed, 15 May 2024 08:53:32 GMT, ExE Boss  wrote:

>> Per Minborg has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Revise docs for ofBackground()
>
> src/java.base/share/classes/jdk/internal/lang/StableValue.java line 1:
> 
>> 1: /*
> 
> Maybe also add `StableValue​::⁠ofLazy​(Supplier)` which 
> behaves more like the original **Computed Constants** JEP draft?

There is a method `StableValue::asSupplier` that is similar to the former 
ComputedConstant behavior.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601840547


Re: RFR: 8330465: Stable Values and Collections (Internal) [v5]

2024-05-15 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using an `AtomicReference` and one 
> protected by double-checked locking under concurrent access by 8 threads:
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
> StableValue (~40% faster than DCL)
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (8 threads):
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
> StableValue ( > 2x faster than `AtomicInteger` and DCL)
> 
> 
> Performance for stable lists in both instance and static contexts whereby the 
> sum of random contents is calculated for stable lists (which are thread-safe) 
> compared to `ArrayList` instances (which are not thread-safe) (under single 
> thread access):
> 
> 
> Benchmark Mode  Cnt  Score   Error  Units
> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  ns/op 
> <- Stable list
> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? 0.002  ns/op
> StableListSumBenchmark.staticList avgt   10  0.356 ? 0.00...

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

  Switch to monomorphic StableValue and use lazy arrays

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/c92b16c4..2b840e06

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

  Stats: 471 lines in 9 files changed: 126 ins; 306 del; 39 mod
  Patch: https://git.openjdk.org/jdk/pull/18794.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18794/head:pull/18794

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v4]

2024-05-15 Thread Per Minborg
On Wed, 15 May 2024 12:26:34 GMT, Rémi Forax  wrote:

>> Given that `TrustedFieldType` is more generic than stable values, it could 
>> be moved to `jdk.internal.misc` or `jdk.internal.reflect`, then 
>> `jdk.unsupported` could use it without exporting new packages to 
>> `jdk.unsupported`.
>
> At some point in the future, 'jdk.unsupported' will be removed

Maybe there is a better home for this?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601843499


Re: RFR: 8330465: Stable Values and Collections (Internal) [v4]

2024-05-15 Thread Rémi Forax
On Wed, 15 May 2024 11:27:04 GMT, ExE Boss  wrote:

>>> Maybe export this interface to `jdk.unsupported`?
>> 
>> I don't we should do that. In general, we need jdk.unsupported to go away in 
>> the long term. Also integrity of the platform depends on java.base being 
>> very stingy and not exporting internal packages to other modules.
>
> Given that `TrustedFieldType` is more generic than stable values, it could be 
> moved to `jdk.internal.misc` or `jdk.internal.reflect`, then 
> `jdk.unsupported` could use it without exporting new packages to 
> `jdk.unsupported`.

At some point in the future, 'jdk.unsupported' will be removed

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601548492


Re: RFR: 8330465: Stable Values and Collections (Internal) [v4]

2024-05-15 Thread ExE Boss
On Wed, 15 May 2024 10:43:52 GMT, Alan Bateman  wrote:

>> src/java.base/share/classes/jdk/internal/lang/stable/TrustedFieldType.java 
>> line 14:
>> 
>>> 12:  * operations.
>>> 13:  */
>>> 14: public sealed interface TrustedFieldType
>> 
>> Maybe export this interface to `jdk.unsupported`?
>
>> Maybe export this interface to `jdk.unsupported`?
> 
> I don't we should do that. In general, we need jdk.unsupported to go away in 
> the long term. Also integrity of the platform depends on java.base being very 
> stingy and not exporting internal packages to other modules.

Given that `TrustedFieldType` is more generic than stable values, it could be 
moved to `jdk.internal.misc` or `jdk.internal.reflect`, then `jdk.unsupported` 
could use it without exporting new packages to `jdk.unsupported`.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601455889


Re: RFR: 8330465: Stable Values and Collections (Internal) [v4]

2024-05-15 Thread Alan Bateman
On Wed, 15 May 2024 08:49:46 GMT, ExE Boss  wrote:

> Maybe export this interface to `jdk.unsupported`?

I don't we should do that. In general, we need jdk.unsupported to go away in 
the long term. Also integrity of the platform depends on java.base being very 
stingy and not exporting internal packages to other modules.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601399452


Re: RFR: 8330465: Stable Values and Collections (Internal) [v4]

2024-05-15 Thread ExE Boss
On Wed, 15 May 2024 07:48:42 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using an `AtomicReference` and 
>> one protected by double-checked locking under concurrent access by 8 threads:
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
>> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
>> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
>> StableValue (~40% faster than DCL)
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (8 threads):
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
>> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
>> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
>> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
>> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
>> StableValue ( > 2x faster than `AtomicInteger` and DCL)
>> 
>> 
>> Performance for stable lists in both instance and static contexts whereby 
>> the sum of random contents is calculated for stable lists (which are 
>> thread-safe) compared to `ArrayList` instances (which are not thread-safe) 
>> (under single thread access):
>> 
>> 
>> Benchmark Mode  Cnt  Score   Error  Units
>> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
>> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  
>> ns/op <- Stable list
>> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? ...
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Revise docs for ofBackground()

src/java.base/share/classes/jdk/internal/lang/StableValue.java line 1:

> 1: /*

Maybe also add `StableValue​::⁠ofLazy​(Supplier)` which behaves 
more like the original **Computed Constants** JEP draft?

src/java.base/share/classes/jdk/internal/lang/stable/TrustedFieldType.java line 
14:

> 12:  * operations.
> 13:  */
> 14: public sealed interface TrustedFieldType

Maybe export this interface to `jdk.unsupported`?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601229245
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601223866


Re: RFR: 8330465: Stable Values and Collections (Internal) [v4]

2024-05-15 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using an `AtomicReference` and one 
> protected by double-checked locking under concurrent access by 8 threads:
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
> StableValue (~40% faster than DCL)
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (8 threads):
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
> StableValue ( > 2x faster than `AtomicInteger` and DCL)
> 
> 
> Performance for stable lists in both instance and static contexts whereby the 
> sum of random contents is calculated for stable lists (which are thread-safe) 
> compared to `ArrayList` instances (which are not thread-safe) (under single 
> thread access):
> 
> 
> Benchmark Mode  Cnt  Score   Error  Units
> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  ns/op 
> <- Stable list
> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? 0.002  ns/op
> StableListSumBenchmark.staticList avgt   10  0.356 ? 0.00...

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

  Revise docs for ofBackground()

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/7db1101c..c92b16c4

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

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

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v3]

2024-05-15 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using an `AtomicReference` and one 
> protected by double-checked locking under concurrent access by 8 threads:
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
> StableValue (~40% faster than DCL)
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (8 threads):
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
> StableValue ( > 2x faster than `AtomicInteger` and DCL)
> 
> 
> Performance for stable lists in both instance and static contexts whereby the 
> sum of random contents is calculated for stable lists (which are thread-safe) 
> compared to `ArrayList` instances (which are not thread-safe) (under single 
> thread access):
> 
> 
> Benchmark Mode  Cnt  Score   Error  Units
> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  ns/op 
> <- Stable list
> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? 0.002  ns/op
> StableListSumBenchmark.staticList avgt   10  0.356 ? 0.00...

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

  Add delegation to the thread's exception handler

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/d7c31585..7db1101c

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

  Stats: 70 lines in 3 files changed: 62 ins; 0 del; 8 mod
  Patch: https://git.openjdk.org/jdk/pull/18794.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18794/head:pull/18794

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


Re: RFR: 8330465: Stable Values and Collections (Internal) [v2]

2024-05-15 Thread Per Minborg
On Tue, 14 May 2024 16:02:41 GMT, Viktor Klang  wrote:

>> Per Minborg has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove text in public class that references an internal class
>
> src/java.base/share/classes/jdk/internal/lang/StableValue.java line 171:
> 
>> 169: /**
>> 170:  * {@return a fresh stable value with an unset value where the 
>> returned stable's
>> 171:  * value is computed in a separate background thread (created via 
>> the provided
> 
> Suggestion:
> 
>  * {@return a fresh stable value with an unset value where the returned 
> stable
>  * value is computed in a separate background thread (created via the 
> provided

It is a bit confusing with "stable value" and the "stable value's value".  
Maybe something like:


 * {@return a fresh stable value with an unset value where its
 * value is computed in a separate background thread (created via the 
provided ...


?

> src/java.base/share/classes/jdk/internal/lang/StableValue.java line 175:
> 
>> 173:  * 
>> 174:  * If the supplier throws an (unchecked) exception, the exception 
>> is ignored, and no
>> 175:  * value is set.
> 
> Is it likely that users will want to be made aware of failures? If so, 
> perhaps it would make sense to make sure that the Exception hits the 
> UncaughtExceptionHandler? 樂

Good suggestion. An alternative would be to provide an exception listener 
getting invoked upon hitting an exception.

> src/java.base/share/classes/jdk/internal/lang/stable/StableArray3DImpl.java 
> line 33:
> 
>> 31: Objects.checkIndex(i1, dim1);
>> 32: Objects.checkIndex(i2, dim2);
>> 33: final int index = i0 * dim1 * dim2 + i1 * dim2 + i2;
> 
> Might be worth doing some overflow checking here?

At construction, we assert the invariant; the product of `dim0, dim1, and dim2` 
can be fit in an `int`:


private StableArray3DImpl(int dim0, int dim1, int dim2) {
this(dim0, dim1, dim2, Math.multiplyExact(Math.multiplyExact(dim0, 
dim1), dim2));
}

The three `checkIndex(iN, dimN)` ensures each iN is greater than zero and less 
than dimN. This means, by induction, that the operation `final int index = i0 * 
dim1 * dim2 + i1 * dim2 + i2;` will never overflow.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601035645
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601028336
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1601025953


Re: RFR: 8330465: Stable Values and Collections (Internal) [v2]

2024-05-14 Thread Viktor Klang
On Tue, 14 May 2024 14:51:20 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using an `AtomicReference` and 
>> one protected by double-checked locking under concurrent access by 8 threads:
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
>> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
>> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
>> StableValue (~40% faster than DCL)
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (8 threads):
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
>> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
>> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
>> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
>> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
>> StableValue ( > 2x faster than `AtomicInteger` and DCL)
>> 
>> 
>> Performance for stable lists in both instance and static contexts whereby 
>> the sum of random contents is calculated for stable lists (which are 
>> thread-safe) compared to `ArrayList` instances (which are not thread-safe) 
>> (under single thread access):
>> 
>> 
>> Benchmark Mode  Cnt  Score   Error  Units
>> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
>> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  
>> ns/op <- Stable list
>> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? ...
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove text in public class that references an internal class

src/java.base/share/classes/jdk/internal/lang/stable/StableArray3DImpl.java 
line 33:

> 31: Objects.checkIndex(i1, dim1);
> 32: Objects.checkIndex(i2, dim2);
> 33: final int index = i0 * dim1 * dim2 + i1 * dim2 + i2;

Might be worth doing some overflow checking here?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1600313603


Re: RFR: 8330465: Stable Values and Collections (Internal) [v2]

2024-05-14 Thread Viktor Klang
On Tue, 14 May 2024 14:51:20 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using an `AtomicReference` and 
>> one protected by double-checked locking under concurrent access by 8 threads:
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
>> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
>> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
>> StableValue (~40% faster than DCL)
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (8 threads):
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
>> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
>> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
>> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
>> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
>> StableValue ( > 2x faster than `AtomicInteger` and DCL)
>> 
>> 
>> Performance for stable lists in both instance and static contexts whereby 
>> the sum of random contents is calculated for stable lists (which are 
>> thread-safe) compared to `ArrayList` instances (which are not thread-safe) 
>> (under single thread access):
>> 
>> 
>> Benchmark Mode  Cnt  Score   Error  Units
>> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
>> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  
>> ns/op <- Stable list
>> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? ...
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove text in public class that references an internal class

src/java.base/share/classes/jdk/internal/lang/StableValue.java line 171:

> 169: /**
> 170:  * {@return a fresh stable value with an unset value where the 
> returned stable's
> 171:  * value is computed in a separate background thread (created via 
> the provided

Suggestion:

 * {@return a fresh stable value with an unset value where the returned 
stable
 * value is computed in a separate background thread (created via the 
provided

src/java.base/share/classes/jdk/internal/lang/StableValue.java line 175:

> 173:  * 
> 174:  * If the supplier throws an (unchecked) exception, the exception is 
> ignored, and no
> 175:  * value is set.

Is it likely that users will want to be made aware of failures? If so, perhaps 
it would make sense to make sure that the Exception hits the 
UncaughtExceptionHandler? 樂

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1600296959
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1600298211


Re: RFR: 8330465: Stable Values and Collections (Internal) [v2]

2024-05-14 Thread Per Minborg
On Tue, 14 May 2024 14:19:44 GMT, Alan Bateman  wrote:

>> Per Minborg has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove text in public class that references an internal class
>
> src/java.base/share/classes/java/lang/reflect/AccessibleObject.java line 195:
> 
>> 193:  * final fields of a type that implements
>> 194:  * {@linkplain jdk.internal.lang.stable.TrustedFieldType}
>> 195:  * (e.g {@linkplain StableValue StableValue})
> 
> The API docs for a standard method can't reference a JDK internal annotation. 
> It would be possible for the API docs to admit to an implementation specific 
> way to do this but I think we should try to avoid this for now.

I will remove it. Once some of the classes are public, we could add back a 
message of this type.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1600179776


Re: RFR: 8330465: Stable Values and Collections (Internal) [v2]

2024-05-14 Thread Per Minborg
> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using an `AtomicReference` and one 
> protected by double-checked locking under concurrent access by 8 threads:
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
> StableValue (~40% faster than DCL)
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (8 threads):
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
> StableValue ( > 2x faster than `AtomicInteger` and DCL)
> 
> 
> Performance for stable lists in both instance and static contexts whereby the 
> sum of random contents is calculated for stable lists (which are thread-safe) 
> compared to `ArrayList` instances (which are not thread-safe) (under single 
> thread access):
> 
> 
> Benchmark Mode  Cnt  Score   Error  Units
> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  ns/op 
> <- Stable list
> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? 0.002  ns/op
> StableListSumBenchmark.staticList avgt   10  0.356 ? 0.00...

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

  Remove text in public class that references an internal class

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18794/files
  - new: https://git.openjdk.org/jdk/pull/18794/files/5d5dcced..d7c31585

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

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

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


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Alan Bateman
On Tue, 16 Apr 2024 11:47:23 GMT, Per Minborg  wrote:

> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using an `AtomicReference` and one 
> protected by double-checked locking under concurrent access by 8 threads:
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
> StableValue (~40% faster than DCL)
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (8 threads):
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
> StableValue ( > 2x faster than `AtomicInteger` and DCL)
> 
> 
> Performance for stable lists in both instance and static contexts whereby the 
> sum of random contents is calculated for stable lists (which are thread-safe) 
> compared to `ArrayList` instances (which are not thread-safe) (under single 
> thread access):
> 
> 
> Benchmark Mode  Cnt  Score   Error  Units
> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  ns/op 
> <- Stable list
> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? 0.002  ns/op
> StableListSumBenchmark.staticList avgt   10  0.356 ? 0.00...

src/java.base/share/classes/java/lang/reflect/AccessibleObject.java line 195:

> 193:  * final fields of a type that implements
> 194:  * {@linkplain jdk.internal.lang.stable.TrustedFieldType}
> 195:  * (e.g {@linkplain StableValue StableValue})

The API docs for a standard method can't reference a JDK internal annotation. 
It would be possible for the API docs to admit to an implementation specific 
way to do this but I think we should try to avoid this for now.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1600132293


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Per Minborg
On Mon, 22 Apr 2024 09:34:39 GMT, Per Minborg  wrote:

>> src/java.base/share/classes/jdk/internal/lang/stable/StableValueElement.java 
>> line 116:
>> 
>>> 114: public V computeIfUnset(Supplier supplier) {
>>> 115: // Todo: This creates a lambda
>>> 116: return computeIfUnsetShared(supplier, Supplier::get);
>> 
>> Suggestion:
>> 
>> return computeIfUnsetShared(supplier, supplierExtractor());
>
> Yes. This is a work in progress. I will explore using a `BiFunction` instead.

I've converted to pattern matching instead.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1576155645


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Per Minborg
On Fri, 19 Apr 2024 09:32:56 GMT, ExE Boss  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using an `AtomicReference` and 
>> one protected by double-checked locking under concurrent access by 8 threads:
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
>> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
>> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
>> StableValue (~40% faster than DCL)
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (8 threads):
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
>> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
>> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
>> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
>> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
>> StableValue ( > 2x faster than `AtomicInteger` and DCL)
>> 
>> 
>> Performance for stable lists in both instance and static contexts whereby 
>> the sum of random contents is calculated for stable lists (which are 
>> thread-safe) compared to `ArrayList` instances (which are not thread-safe) 
>> (under single thread access):
>> 
>> 
>> Benchmark Mode  Cnt  Score   Error  Units
>> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
>> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  
>> ns/op <- Stable list
>> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? ...
>
> src/java.base/share/classes/jdk/internal/lang/stable/StableValueElement.java 
> line 116:
> 
>> 114: public V computeIfUnset(Supplier supplier) {
>> 115: // Todo: This creates a lambda
>> 116: return computeIfUnsetShared(supplier, Supplier::get);
> 
> Suggestion:
> 
> return computeIfUnsetShared(supplier, supplierExtractor());

Yes. This is a work in progress. I will explore using a `BiFunction` instead.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1574446126


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Chen Liang
On Wed, 17 Apr 2024 13:23:53 GMT, Maurizio Cimadamore  
wrote:

>> I see what you mean. Initially, I thought it would be easy to create 
>> memorized functions but it turned out, that was not the case if one wants to 
>> retain easy debugability etc. So, I have added a couple of factory methods 
>> including this:
>> 
>> 
>> /**
>>  * {@return a new memoized {@linkplain Function } backed by an 
>> internal
>>  * stable map with the provided {@code inputs} keys where the provided
>>  * {@code original} Function will only be invoked at most once per 
>> distinct input}
>>  *
>>  * @param original the original Function to convert to a memoized 
>> Function
>>  * @param inputs   the potential input values to the Function
>>  * @param   the type of input values
>>  * @param   the return type of the function
>>  */
>> static  Function ofFunction(Set inputs,
>> Function 
>> original) {
>> Objects.requireNonNull(inputs);
>> Objects.requireNonNull(original);
>> ...
>> }
>
> I agree that these method appear to be confusing. We have:
> 
> 
> StableValue::of()
> StableValue::ofList(int)
> StableValue::ofMap(Set)
> 
> 
> These methods are clearly primitives, because they are used to create a 
> wrapper around a stable value/array. (Actually, if you squint, the primitive 
> is really the `ofMap` factory, since that one can be used to derive the other 
> two as well, but that's mostly a sophism).
> 
> Everything else falls in the "helper" bucket. That is, we could have:
> 
> 
> StableValue::ofList(IntFunction) -> List // similar to 
> StableValue::ofList(int)
> StableValue::ofMap(Function) -> Map // similar to 
> StableValue::ofMap(Set)
> 
> 
> Or, we could have:
> 
> 
> StableValue::ofSupplier(Supplier) -> Supplier // similar to 
> StableValue::of()
> StableValue::ofIntFunction(IntFunction) -> IntFunction // similar to 
> StableValue::ofList(int)
> StableValue::ofFunction(Function) -> Function // similar to 
> StableValue::ofMap(Set)
> 
> 
> IMHO, having both sets feel slightly redundant. That is, if you have a Map V>, you also have a function from K, V - namely, map::get. And, conversely, 
> if a client wants a List of fixed size, which is populated lazily, using a 
> memoized IntFunction is, effectively, the same thing.

I prefer these:

StableValue::ofSupplier(Supplier) -> StableValue
StableValue::ofIntFunction(keys, IntFunction) -> IntFunction>
StableValue::ofFunction(keys, Function) -> Function>

These still expose StableValue so users can set the values if they need. In 
addition, the List/Map functionalites are mostly useless so a getter/function 
suffices for the most part.

These APIs are less error-prone to accidental context capture compared to the 
individual use-site ones, as use-site leaks means each access involves an 
allocation, but the allocation for construction site is shared.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1568895552


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Maurizio Cimadamore
On Wed, 17 Apr 2024 11:12:37 GMT, Per Minborg  wrote:

>> Yes, consider the 3 capture scenarios:
>> | API | Capture frequency | Capture Impact | Code Convenience | Flexibility |
>> |-|---||--|-|
>> | `StableValue.ofMap(map, k -> ...)` | By accident | single capture is 
>> reused | OK | One generator for all keys |
>> | `StableValue.computeIfUnset(map, key, k -> ...)` | By accident | capture 
>> happens for all access sites | somewhat ugly | Different generator for 
>> different keys |
>> | `map.get(k).computeIfUnset(() -> ...)` | Always | capture happens for all 
>> access sites | OK | Different generator for different keys |
>> 
>> Notice the `ofMap` factory is the most tolerant to faulty captures: even if 
>> it captures, the single capturing lambda is reused for all map stables, 
>> avoiding capture overheads at access sites. Given Java compiler doesn't tell 
>> user anything about captures during compilation, I think `ofMap` is a better 
>> factory to avoid accidentally writing capturing lambdas.
>
> I see what you mean. Initially, I thought it would be easy to create 
> memorized functions but it turned out, that was not the case if one wants to 
> retain easy debugability etc. So, I have added a couple of factory methods 
> including this:
> 
> 
> /**
>  * {@return a new memoized {@linkplain Function } backed by an 
> internal
>  * stable map with the provided {@code inputs} keys where the provided
>  * {@code original} Function will only be invoked at most once per 
> distinct input}
>  *
>  * @param original the original Function to convert to a memoized Function
>  * @param inputs   the potential input values to the Function
>  * @param   the type of input values
>  * @param   the return type of the function
>  */
> static  Function ofFunction(Set inputs,
> Function 
> original) {
> Objects.requireNonNull(inputs);
> Objects.requireNonNull(original);
> ...
> }

I agree that these method appear to be confusing. We have:


StableValue::of()
StableValue::ofList(int)
StableValue::ofMap(Set)


These methods are clearly primitives, because they are used to create a wrapper 
around a stable value/array. (Actually, if you squint, the primitive is really 
the `ofMap` factory, since that one can be used to derive the other two as 
well, but that's mostly a sophism).

Everything else falls in the "helper" bucket. That is, we could have:


StableValue::ofList(IntFunction) -> List // similar to 
StableValue::ofList(int)
StableValue::ofMap(Function) -> Map // similar to StableValue::ofMap(Set)


Or, we could have:


StableValue::ofSupplier(Supplier) -> Supplier // similar to StableValue::of()
StableValue::ofIntFunction(IntFunction) -> IntFunction // similar to 
StableValue::ofList(int)
StableValue::ofFunction(Function) -> Function // similar to 
StableValue::ofMap(Set)


IMHO, having both sets feel slightly redundant. That is, if you have a Map, you also have a function from K, V - namely, map::get. And, conversely, if 
a client wants a List of fixed size, which is populated lazily, using a 
memoized IntFunction is, effectively, the same thing.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1568840269


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Per Minborg
On Mon, 22 Apr 2024 17:09:39 GMT, Chen Liang  wrote:

>> src/java.base/share/classes/jdk/internal/lang/StableValue.java line 130:
>> 
>>> 128:  * } else {
>>> 129:  * V newValue = supplier.get();
>>> 130:  * stable.setOrThrow(newValue);
>> 
>> If ::computeIfUnset allows racy sets, then it isn't equivalent to this code 
>> as ::setOrThrow will throw on a race, correct?
>
> Indeed, this if-else should be guarded by a synchronized block, except the 
> lock is on the internal mutex which is not publicly exposed.

`computeIfUnset()` is indeed guarded by a synchronized block, only it sits on 
the method declaration of `computeIfUnsetVolatile0`. I think we should have an 
internal mutex. This will also correspond to the stable collections which have 
internal mutexes for each index/key.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1575972396


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Chen Liang
On Mon, 22 Apr 2024 16:16:39 GMT, Dan Heidinga  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using an `AtomicReference` and 
>> one protected by double-checked locking under concurrent access by 8 threads:
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
>> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
>> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
>> StableValue (~40% faster than DCL)
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (8 threads):
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
>> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
>> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
>> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
>> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
>> StableValue ( > 2x faster than `AtomicInteger` and DCL)
>> 
>> 
>> Performance for stable lists in both instance and static contexts whereby 
>> the sum of random contents is calculated for stable lists (which are 
>> thread-safe) compared to `ArrayList` instances (which are not thread-safe) 
>> (under single thread access):
>> 
>> 
>> Benchmark Mode  Cnt  Score   Error  Units
>> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
>> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  
>> ns/op <- Stable list
>> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? ...
>
> src/java.base/share/classes/jdk/internal/lang/StableValue.java line 130:
> 
>> 128:  * } else {
>> 129:  * V newValue = supplier.get();
>> 130:  * stable.setOrThrow(newValue);
> 
> If ::computeIfUnset allows racy sets, then it isn't equivalent to this code 
> as ::setOrThrow will throw on a race, correct?

Indeed, this if-else should be guarded by a synchronized block, except the lock 
is on the internal mutex which is not publicly exposed.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1575101750


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Per Minborg
On Wed, 17 Apr 2024 14:24:59 GMT, Maurizio Cimadamore  
wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using an `AtomicReference` and 
>> one protected by double-checked locking under concurrent access by 8 threads:
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
>> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
>> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
>> StableValue (~40% faster than DCL)
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (8 threads):
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
>> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
>> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
>> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
>> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
>> StableValue ( > 2x faster than `AtomicInteger` and DCL)
>> 
>> 
>> Performance for stable lists in both instance and static contexts whereby 
>> the sum of random contents is calculated for stable lists (which are 
>> thread-safe) compared to `ArrayList` instances (which are not thread-safe) 
>> (under single thread access):
>> 
>> 
>> Benchmark Mode  Cnt  Score   Error  Units
>> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
>> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  
>> ns/op <- Stable list
>> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? ...
>
> src/java.base/share/classes/java/util/ImmutableCollections.java line 1505:
> 
>> 1503: }
>> 1504: 
>> 1505: static final class StableMap
> 
> Question: can stable maps be implemented in terms of stable lists? After all, 
> you need a stable backing array - and then you need to have a way to go from 
> a key to an index in the stable array. The logic that does key->index 
> conversion belongs to Map, but after that we should be able to just 
> "delegate" to StableList?

This is partially done but we could pull more on this string and unify the 
implementations.

Incidentally, it is also possible to unify the two implementation classes of 
`StableValue` so it becomes monomorphic.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1570837425


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Chen Liang
On Tue, 23 Apr 2024 12:18:53 GMT, Per Minborg  wrote:

>> Good idea.
>
> Ahh. I thought you meant pattern matching in another place (which actually 
> turned out to be a really good idea). Here, however, we also need to get the 
> type parameters correct:
> 
>  src="https://github.com/openjdk/jdk/assets/7457876/b2a66033-4c74-42bc-a9de-a976c441ca61;>

Would you still need a cast if you declare `HasComputeIfUnset` with `extends 
Map>`?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1581828542


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Maurizio Cimadamore
On Tue, 16 Apr 2024 11:47:23 GMT, Per Minborg  wrote:

> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using an `AtomicReference` and one 
> protected by double-checked locking under concurrent access by 8 threads:
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
> StableValue (~40% faster than DCL)
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (8 threads):
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
> StableValue ( > 2x faster than `AtomicInteger` and DCL)
> 
> 
> Performance for stable lists in both instance and static contexts whereby the 
> sum of random contents is calculated for stable lists (which are thread-safe) 
> compared to `ArrayList` instances (which are not thread-safe) (under single 
> thread access):
> 
> 
> Benchmark Mode  Cnt  Score   Error  Units
> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  ns/op 
> <- Stable list
> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? 0.002  ns/op
> StableListSumBenchmark.staticList avgt   10  0.356 ? 0.00...

src/java.base/share/classes/java/util/ImmutableCollections.java line 1505:

> 1503: }
> 1504: 
> 1505: static final class StableMap

Question: can stable maps be implemented in terms of stable lists? After all, 
you need a stable backing array - and then you need to have a way to go from a 
key to an index in the stable array. The logic that does key->index conversion 
belongs to Map, but after that we should be able to just "delegate" to 
StableList?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1568940281


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Dan Heidinga
On Tue, 23 Apr 2024 12:22:25 GMT, Per Minborg  wrote:

>> src/java.base/share/classes/java/lang/reflect/AccessibleObject.java line 193:
>> 
>>> 191:  * final fields declared in a {@linkplain Class#isHidden() 
>>> hidden class}
>>> 192:  * final fields declared in a {@linkplain Class#isRecord() 
>>> record}
>>> 193:  * final fields of type {@linkplain StableValue 
>>> StableValue}
>> 
>> In Valhalla, we've been looking at adding "strict" final fields to support 
>> value classes (which must be strongly immutable) which are fields that are 
>> unmodifiable.  Most of the existing unmodifiable field cases can be covered 
>> by "strict" fields.  This one can't though so I'm a little saddened to see 
>> this list grow.
>
> Maybe we could introduce a special marker interface (e.g. `TrustedFieldType`) 
> that signals this behavior. This might only take effect if loaded via the 
> boot loader.

Thinking on this more, hidden classes & records & value classes can all be 
dealt with by the introduction of strict fields.  Adding a new type - 
TrustedFieldType - when we'll eventually only have 1 type here - StableValue - 
seems like an unnecessary tradeoff.

If we ever have to add a second type here, then it's probably worth revisiting 
this idea.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1584921294


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Per Minborg
On Mon, 13 May 2024 12:18:28 GMT, Chen Liang  wrote:

>> As we need the array in both cases, how would such a solution look like 
>> without duplicating code?
>
> I was thinking about changing the StableEnumMap factory to directly take an 
> EnumSet/BitSet indicating the indices without conversions to full objects and 
> arrays.

Sounds like a good idea. I will fix this.

>> I see what you mean with distinct initialization logic. This is not the 
>> intended use though. 
>> 
>> The reason these methods exist is to avoid lambda capturing. Let's say we 
>> have a `Function` we want to apply to a `Map>`. 
>> Then, retrieving a `stable = StableValue` and applying 
>> `stable.computeIfUnset(() -> function.apply(key))` would capture a new 
>> `Supplier`. Another alternative would be to write imperative code similar to 
>> what is already in these methods.
>> 
>> What we could do is provide factories for memorized functions (the latter 
>> described in the draft JEP at the end (https://openjdk.org/jeps/8312611) ) 
>> even though these are easy to write.
>> 
>> I think what you are proposing is something like this?
>> 
>> 
>> Map> map = StableValue.ofMap(keys, k -> createV(k));
>> 
>> 
>> or perhaps even:
>> 
>> 
>> Map map = StableValue.ofMap(keys, k -> createV(k));
>
> Yes, consider the 3 capture scenarios:
> | API | Capture frequency | Capture Impact | Code Convenience | Flexibility |
> |-|---||--|-|
> | `StableValue.ofMap(map, k -> ...)` | By accident | single capture is reused 
> | OK | One generator for all keys |
> | `StableValue.computeIfUnset(map, key, k -> ...)` | By accident | capture 
> happens for all access sites | somewhat ugly | Different generator for 
> different keys |
> | `map.get(k).computeIfUnset(() -> ...)` | Always | capture happens for all 
> access sites | OK | Different generator for different keys |
> 
> Notice the `ofMap` factory is the most tolerant to faulty captures: even if 
> it captures, the single capturing lambda is reused for all map stables, 
> avoiding capture overheads at access sites. Given Java compiler doesn't tell 
> user anything about captures during compilation, I think `ofMap` is a better 
> factory to avoid accidentally writing capturing lambdas.

I see what you mean. Initially, I thought it would be easy to create memorized 
functions but it turned out, that was not the case if one wants to retain easy 
debugability etc. So, I have added a couple of factory methods including this:


/**
 * {@return a new memoized {@linkplain Function } backed by an 
internal
 * stable map with the provided {@code inputs} keys where the provided
 * {@code original} Function will only be invoked at most once per distinct 
input}
 *
 * @param original the original Function to convert to a memoized Function
 * @param inputs   the potential input values to the Function
 * @param   the type of input values
 * @param   the return type of the function
 */
static  Function ofFunction(Set inputs,
Function 
original) {
Objects.requireNonNull(inputs);
Objects.requireNonNull(original);
...
}

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1600035236
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1568671241


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Per Minborg
On Tue, 23 Apr 2024 09:17:29 GMT, Per Minborg  wrote:

>> src/java.base/share/classes/java/util/ImmutableCollections.java line 183:
>> 
>>> 181:K key,
>>> 182:Function>> extends V> mapper) {
>>> 183: if (map instanceof HasComputeIfUnset) {
>> 
>> Can we use pattern matching instanceof here?
>> 
>> if (map instance HasComputeIfUnset uc) {
>
> Good idea.

Ahh. I thought you meant pattern matching in another place (which actually 
turned out to be a really good idea). Here, however, we also need to get the 
type parameters correct:

https://github.com/openjdk/jdk/assets/7457876/b2a66033-4c74-42bc-a9de-a976c441ca61;>

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1576154547


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Per Minborg
On Tue, 14 May 2024 11:07:00 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using an `AtomicReference` and 
>> one protected by double-checked locking under concurrent access by 8 threads:
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
>> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
>> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
>> StableValue (~40% faster than DCL)
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (8 threads):
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
>> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
>> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
>> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
>> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
>> StableValue ( > 2x faster than `AtomicInteger` and DCL)
>> 
>> 
>> Performance for stable lists in both instance and static contexts whereby 
>> the sum of random contents is calculated for stable lists (which are 
>> thread-safe) compared to `ArrayList` instances (which are not thread-safe) 
>> (under single thread access):
>> 
>> 
>> Benchmark Mode  Cnt  Score   Error  Units
>> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
>> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  
>> ns/op <- Stable list
>> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? ...
>
> src/hotspot/share/ci/ciField.cpp line 262:
> 
>> 260: const char* stable_array3d_klass_name = 
>> "jdk/internal/lang/StableArray3D";
>> 261: 
>> 262: static bool trust_final_non_static_fields_of_type(Symbol* signature) {
> 
> Is there a better way of doing this?

How do we check if the type implements `TrustedFieldType` in C?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1599836419


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Per Minborg
On Mon, 22 Apr 2024 16:31:15 GMT, Dan Heidinga  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using an `AtomicReference` and 
>> one protected by double-checked locking under concurrent access by 8 threads:
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
>> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
>> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
>> StableValue (~40% faster than DCL)
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (8 threads):
>> 
>> 
>> Benchmark   Mode  Cnt  Score   Error  Units
>> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
>> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
>> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
>> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
>> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
>> StableValue ( > 2x faster than `AtomicInteger` and DCL)
>> 
>> 
>> Performance for stable lists in both instance and static contexts whereby 
>> the sum of random contents is calculated for stable lists (which are 
>> thread-safe) compared to `ArrayList` instances (which are not thread-safe) 
>> (under single thread access):
>> 
>> 
>> Benchmark Mode  Cnt  Score   Error  Units
>> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
>> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  
>> ns/op <- Stable list
>> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? ...
>
> src/java.base/share/classes/java/lang/reflect/AccessibleObject.java line 193:
> 
>> 191:  * final fields declared in a {@linkplain Class#isHidden() 
>> hidden class}
>> 192:  * final fields declared in a {@linkplain Class#isRecord() 
>> record}
>> 193:  * final fields of type {@linkplain StableValue 
>> StableValue}
> 
> In Valhalla, we've been looking at adding "strict" final fields to support 
> value classes (which must be strongly immutable) which are fields that are 
> unmodifiable.  Most of the existing unmodifiable field cases can be covered 
> by "strict" fields.  This one can't though so I'm a little saddened to see 
> this list grow.

Maybe we could introduce a special marker interface (e.g. `TrustedFieldType`) 
that signals this behavior. This might only take effect if loaded via the boot 
loader.

> src/java.base/share/classes/java/util/ImmutableCollections.java line 183:
> 
>> 181:K key,
>> 182:Function> extends V> mapper) {
>> 183: if (map instanceof HasComputeIfUnset) {
> 
> Can we use pattern matching instanceof here?
> 
> if (map instance HasComputeIfUnset uc) {

Good idea.

> src/java.base/share/classes/jdk/internal/lang/StableArray.java line 25:
> 
>> 23:  * @since 23
>> 24:  */
>> 25: public sealed interface StableArray
> 
> Do we have a use case for StableArray beyond those of StableList?

I am trying to model multi-dimensional arrays that also provide flattening. 
Let's see if it becomes useful.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1576158929
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1575929756
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1591465182


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Dan Heidinga
On Tue, 16 Apr 2024 11:47:23 GMT, Per Minborg  wrote:

> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using an `AtomicReference` and one 
> protected by double-checked locking under concurrent access by 8 threads:
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
> StableValue (~40% faster than DCL)
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (8 threads):
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
> StableValue ( > 2x faster than `AtomicInteger` and DCL)
> 
> 
> Performance for stable lists in both instance and static contexts whereby the 
> sum of random contents is calculated for stable lists (which are thread-safe) 
> compared to `ArrayList` instances (which are not thread-safe) (under single 
> thread access):
> 
> 
> Benchmark Mode  Cnt  Score   Error  Units
> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  ns/op 
> <- Stable list
> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? 0.002  ns/op
> StableListSumBenchmark.staticList avgt   10  0.356 ? 0.00...

src/java.base/share/classes/java/lang/reflect/AccessibleObject.java line 193:

> 191:  * final fields declared in a {@linkplain Class#isHidden() 
> hidden class}
> 192:  * final fields declared in a {@linkplain Class#isRecord() 
> record}
> 193:  * final fields of type {@linkplain StableValue StableValue}

In Valhalla, we've been looking at adding "strict" final fields to support 
value classes (which must be strongly immutable) which are fields that are 
unmodifiable.  Most of the existing unmodifiable field cases can be covered by 
"strict" fields.  This one can't though so I'm a little saddened to see this 
list grow.

src/java.base/share/classes/java/lang/reflect/Field.java line 179:

> 177: AccessibleObject.checkPermission();
> 178: if (flag) {
> 179: if (StableValue.class.isAssignableFrom(type) && 
> Modifier.isFinal(modifiers)) {

Should this check be done regardless of the value of "flag"?  If it failed 
always when calling ::setAccessible on a StableValue, we'd make it easier to 
find bugs and the contract for users would be clearer

src/java.base/share/classes/java/lang/reflect/Field.java line 181:

> 179: if (StableValue.class.isAssignableFrom(type) && 
> Modifier.isFinal(modifiers)) {
> 180: throw newInaccessibleObjectException(
> 181: "Unable to make field " + this + " accessable: " 
> +

Suggestion:

"Unable to make field " + this + " accessible: " +

src/java.base/share/classes/java/util/ImmutableCollections.java line 183:

> 181:K key,
> 182:Function extends V> mapper) {
> 183: if (map instanceof HasComputeIfUnset) {

Can we use pattern matching instanceof here?

if (map instance HasComputeIfUnset uc) {

src/java.base/share/classes/jdk/internal/lang/StableArray.java line 25:

> 23:  * @since 23
> 24:  */
> 25: public sealed interface StableArray

Do we have a use case for StableArray beyond those of StableList?

src/java.base/share/classes/jdk/internal/lang/StableValue.java line 47:

> 45:  * An atomic, thread-safe, stable value holder for which the value can be 
> set at most once.
> 46:  * 
> 47:  * Stable values are eligible 

Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Per Minborg
On Tue, 16 Apr 2024 11:47:23 GMT, Per Minborg  wrote:

> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using an `AtomicReference` and one 
> protected by double-checked locking under concurrent access by 8 threads:
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
> StableValue (~40% faster than DCL)
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (8 threads):
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
> StableValue ( > 2x faster than `AtomicInteger` and DCL)
> 
> 
> Performance for stable lists in both instance and static contexts whereby the 
> sum of random contents is calculated for stable lists (which are thread-safe) 
> compared to `ArrayList` instances (which are not thread-safe) (under single 
> thread access):
> 
> 
> Benchmark Mode  Cnt  Score   Error  Units
> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  ns/op 
> <- Stable list
> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? 0.002  ns/op
> StableListSumBenchmark.staticList avgt   10  0.356 ? 0.00...

I've run some benchmarks on various platforms for static fields (higher is 
better):

https://github.com/openjdk/jdk/assets/7457876/074f45f5-e577-4da0-97ab-2e7023cc6550;>

Here are some figures for various platforms where we compare `AtomicReference`, 
double-checked locking holder, and `StableValue` using instance variables and 
where we iterate and sum 20 values from said constructs:

https://github.com/openjdk/jdk/assets/7457876/6554233c-8238-483f-8a1b-e51f832243a3;>

Note: The figures should be taken with a grain of salt pending a deeper 
analysis.

src/hotspot/share/ci/ciField.cpp line 262:

> 260: const char* stable_array3d_klass_name = 
> "jdk/internal/lang/StableArray3D";
> 261: 
> 262: static bool trust_final_non_static_fields_of_type(Symbol* signature) {

Is there a better way of doing this?

-

PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2075121059
PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2076586960
PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1599826991


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Per Minborg
On Wed, 17 Apr 2024 15:17:52 GMT, Per Minborg  wrote:

>> Also, I want to mention a few important differences between `@Stable` and 
>> Stable Values:
>> 
>> Patterns:
>> 1. Benign race (does not exist in StableValue API): multiple threads can 
>> create an instance and upload, any non-null instance is functionally 
>> equivalent so race is ok (seen in most of JDK)
>> 2. compareAndSet (setIfUnset): multiple threads can create instance, only 
>> one will succeed in uploading; usually for when the instance computation is 
>> cheap but we want single witness.
>> 3. atomic computation (computeIfUnset): only one thread can create instance 
>> which will be witnessed by other threads; this pattern ensures correctness 
>> and prevents wasteful computation by other threads at the cost of locking 
>> and lambda creation.
>> 
>> Allocation in objects: `@Stable` field is local to an object but 
>> `StableValue` is another object; thus sharing strategy may differ, as stable 
>> fields are copied over but StableValue uses a shared cache (which is even 
>> better for avoiding redundant computation)
>> 
>> Question:
>> 1. Will we ever try to expose the stable benign race model to users?
>> 2. Will we ever try to inline the stable values in object layout like a 
>> stable field?
>
>> Question:
>> 
>> 1. Will we ever try to expose the stable benign race model to users?
>> 2. Will we ever try to inline the stable values in object layout like a 
>> stable field?
> 
> 1. I think there is little or no upside in exposing the benign race. It would 
> also be difficult to assert the invariant, competing objects are functionally 
> equivalent. So, I think no.
> 
> 2. In a static context, the stable value will be inlined (or rather 
> constant-folded). So we are partly already there. For pure instance contexts, 
> I have some ideas about this but it is non-trivial.

> @minborg Just curious, why are you adding holder-in-holder benchmark cases?

I'd like to test the transitive constant folding capabilities.

-

PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2075119450


Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Per Minborg
On Wed, 17 Apr 2024 14:07:05 GMT, Chen Liang  wrote:

> Question:
> 
> 1. Will we ever try to expose the stable benign race model to users?
> 2. Will we ever try to inline the stable values in object layout like a 
> stable field?

1. I think there is little or no upside in exposing the benign race. It would 
also be difficult to assert the invariant, competing objects are functionally 
equivalent. So, I think no.

2. In a static context, the stable value will be inlined (or rather 
constant-folded). So we are partly already there. For pure instance contexts, I 
have some ideas about this but it is non-trivial.

> Just curious, can you test other samples, like `StableValue>` 
> where the contained `List` is an immutable list from `List.of` factories? I 
> think that would be a meaningful case too.

Good suggestion. I've added such a test. It turns out the performance is great 
there too.

> Also on a side note, I just realized there's no equivalent of `@Stable int[]` 
> etc. stable primitive arrays exposed, yet immutable arrays will be useful. Is 
> the Frozen Arrays JEP still active, or will this Stable Values consider 
> expose stable primitive arrays?

Good question. In one of the previous prototypes, we accepted a class literal 
that would enable the use of primitive arrays. We now think that we can achieve 
the same thing once Valhalla is integrated. This will allow not just 
`StableValue` to use primitive flattened arrays but also a large number of 
other constructs like `ArrayList`.

One thing we are considering is adding support for stable multi-dimensional 
reference arrays. For an overwhelming majority of the use cases, we would be 
able to eliminate the second layer of indirection that is there for arrays of 
rank > 1.

> src/java.base/share/classes/java/lang/reflect/Field.java line 179:
> 
>> 177: AccessibleObject.checkPermission();
>> 178: // Always check if the field is a final StableValue
>> 179: if (StableValue.class.isAssignableFrom(type) && 
>> Modifier.isFinal(modifiers)) {
> 
> This doesn't protect the Stable Collections.

I will take a look at having an interface that signals this.

> src/java.base/share/classes/java/util/ImmutableCollections.java line 173:
> 
>> 171: .map(Objects::requireNonNull)
>> 172: .toArray();
>> 173: return keys instanceof EnumSet
> 
> We can move this instanceof check before the stream call.

As we need the array in both cases, how would such a solution look like without 
duplicating code?

> src/java.base/share/classes/java/util/ImmutableCollections.java line 1457:
> 
>> 1455: private final V[] elements;
>> 1456: @Stable
>> 1457: private final AuxiliaryArrays aux;
> 
> Is java.util not trusted package so we need `@Stable`?

That is correct. Hence, there are many @Stable annotations already in this 
class.

> src/java.base/share/classes/java/util/ImmutableCollections.java line 1677:
> 
>> 1675: static final class StableEnumMap, V>
>> 1676: extends AbstractImmutableMap>
>> 1677: implements Map>, HasComputeIfUnset 
>> {
> 
> Note that this might be a navigable map, as enums are comparable.

While that is true, no other immutable collection implements a navigable map. 
The way the API is currently wired, it always returns a `Map`. If we go down 
this route, it would incidentally return a `NaviableMap` if presented with an 
`EnumMap` or, we could have a separate factory for enums that states it returns 
a `NavigableMap`. I think creating all the required views would increase 
complexity significantly and I am not sure it would be used that much. That 
said, let us keep this open for the future.

> src/java.base/share/classes/java/util/ImmutableCollections.java line 1855:
> 
>> 1853: @Override
>> 1854: public boolean equals(Object o) {
>> 1855: return o == this;
> 
> These implementations are violations to the Set contracts; Set's hash code 
> should be its elements' sum (thus an entry set's hash code is equivalent to 
> its map's hash) and equals should check if all elements are present. This 
> also makes two entry sets from two `entrySet()` calls not equal (at least 
> before valhalla)

Good catch. Thank you for finding this!

> src/java.base/share/classes/jdk/internal/lang/StableValue.java line 279:
> 
>> 277: static  V computeIfUnset(List> list,
>> 278: int index,
>> 279: IntFunction mapper) {
> 
> Hmm, these APIs seem unintuitive and error-prone to users. Have you studied 
> the use case where for one stable list/map, there are distinct initialization 
> logics for different indices/keys so that they support different mappers for 
> the same list/map? I cannot recall on top of my head. 
> 
> If we drop said ability and restrict mappers to the list/map creation, the 
> whole thing will be 

Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread ExE Boss
On Tue, 16 Apr 2024 11:47:23 GMT, Per Minborg  wrote:

> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using an `AtomicReference` and one 
> protected by double-checked locking under concurrent access by 8 threads:
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
> StableValue (~40% faster than DCL)
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (8 threads):
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
> StableValue ( > 2x faster than `AtomicInteger` and DCL)
> 
> 
> Performance for stable lists in both instance and static contexts whereby the 
> sum of random contents is calculated for stable lists (which are thread-safe) 
> compared to `ArrayList` instances (which are not thread-safe) (under single 
> thread access):
> 
> 
> Benchmark Mode  Cnt  Score   Error  Units
> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  ns/op 
> <- Stable list
> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? 0.002  ns/op
> StableListSumBenchmark.staticList avgt   10  0.356 ? 0.00...

**Nit:** Inconsistent whitespace:

src/java.base/share/classes/java/lang/reflect/AccessibleObject.java line 393:

> 391: }
> 392: 
> 393: InaccessibleObjectException newInaccessibleObjectException(String 
> msg) {

This internal helper method can be `static`:
Suggestion:

static InaccessibleObjectException newInaccessibleObjectException(String 
msg) {

src/java.base/share/classes/jdk/internal/lang/stable/StableValueElement.java 
line 57:

> 55: return switch (stateVolatile()) {
> 56: case UNSET-> throw new NoSuchElementException(); // No 
> value was set
> 57: case NON_NULL -> orThrowVolatile(); // Race: another thread 
> has set a value

It should be safe to avoid self‑recursion here:
Suggestion:

case NON_NULL -> elements[index]; // Race: another thread has set a 
value

or:
Suggestion:

case NON_NULL -> {
v = elements[index]; // Race: another thread has set a value
if (v != null) {
yield v;
}
throw shouldNotReachHere();
}

src/java.base/share/classes/jdk/internal/lang/stable/StableValueElement.java 
line 63:

> 61: // more compact byte code.
> 62: switch (stateVolatile()) {
> 63: case UNSET:{ throw StableUtil.notSet();}

Suggestion:

case UNSET:{ throw StableUtil.notSet(); }

src/java.base/share/classes/jdk/internal/lang/stable/StableValueElement.java 
line 116:

> 114: public V computeIfUnset(Supplier supplier) {
> 115: // Todo: This creates a lambda
> 116: return computeIfUnsetShared(supplier, Supplier::get);

Suggestion:

return computeIfUnsetShared(supplier, supplierExtractor());

src/java.base/share/classes/jdk/internal/lang/stable/StableValueElement.java 
line 144:

> 142: // more compact byte code.
> 143: switch (stateVolatile()) {
> 144: case UNSET:{ return computeIfUnsetVolatile0(provider, 
> key);}

Suggestion:

case UNSET:{ return computeIfUnsetVolatile0(provider, key); }

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 
116:

> 114: // more compact byte code.
> 115: switch (stateVolatile()) {
> 116: case UNSET:   

Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Chen Liang
On Wed, 17 Apr 2024 15:17:52 GMT, Per Minborg  wrote:

>> Also, I want to mention a few important differences between `@Stable` and 
>> Stable Values:
>> 
>> Patterns:
>> 1. Benign race (does not exist in StableValue API): multiple threads can 
>> create an instance and upload, any non-null instance is functionally 
>> equivalent so race is ok (seen in most of JDK)
>> 2. compareAndSet (setIfUnset): multiple threads can create instance, only 
>> one will succeed in uploading; usually for when the instance computation is 
>> cheap but we want single witness.
>> 3. atomic computation (computeIfUnset): only one thread can create instance 
>> which will be witnessed by other threads; this pattern ensures correctness 
>> and prevents wasteful computation by other threads at the cost of locking 
>> and lambda creation.
>> 
>> Allocation in objects: `@Stable` field is local to an object but 
>> `StableValue` is another object; thus sharing strategy may differ, as stable 
>> fields are copied over but StableValue uses a shared cache (which is even 
>> better for avoiding redundant computation)
>> 
>> Question:
>> 1. Will we ever try to expose the stable benign race model to users?
>> 2. Will we ever try to inline the stable values in object layout like a 
>> stable field?
>
>> Question:
>> 
>> 1. Will we ever try to expose the stable benign race model to users?
>> 2. Will we ever try to inline the stable values in object layout like a 
>> stable field?
> 
> 1. I think there is little or no upside in exposing the benign race. It would 
> also be difficult to assert the invariant, competing objects are functionally 
> equivalent. So, I think no.
> 
> 2. In a static context, the stable value will be inlined (or rather 
> constant-folded). So we are partly already there. For pure instance contexts, 
> I have some ideas about this but it is non-trivial.

@minborg Just curious, why are you adding holder-in-holder benchmark cases?

>> src/java.base/share/classes/java/util/ImmutableCollections.java line 173:
>> 
>>> 171: .map(Objects::requireNonNull)
>>> 172: .toArray();
>>> 173: return keys instanceof EnumSet
>> 
>> We can move this instanceof check before the stream call.
>
> As we need the array in both cases, how would such a solution look like 
> without duplicating code?

I was thinking about changing the StableEnumMap factory to directly take an 
EnumSet/BitSet indicating the indices without conversions to full objects and 
arrays.

>> src/java.base/share/classes/java/util/ImmutableCollections.java line 1677:
>> 
>>> 1675: static final class StableEnumMap, V>
>>> 1676: extends AbstractImmutableMap>
>>> 1677: implements Map>, HasComputeIfUnset>> V> {
>> 
>> Note that this might be a navigable map, as enums are comparable.
>
> While that is true, no other immutable collection implements a navigable map. 
> The way the API is currently wired, it always returns a `Map`. If we go down 
> this route, it would incidentally return a `NaviableMap` if presented with an 
> `EnumMap` or, we could have a separate factory for enums that states it 
> returns a `NavigableMap`. I think creating all the required views would 
> increase complexity significantly and I am not sure it would be used that 
> much. That said, let us keep this open for the future.

Fair enough, `Collections` APIs like `unmodifiableSortedMap` explicitly avoid 
implementing too many interfaces.

>> src/java.base/share/classes/jdk/internal/lang/StableValue.java line 279:
>> 
>>> 277: static  V computeIfUnset(List> list,
>>> 278: int index,
>>> 279: IntFunction mapper) {
>> 
>> Hmm, these APIs seem unintuitive and error-prone to users. Have you studied 
>> the use case where for one stable list/map, there are distinct 
>> initialization logics for different indices/keys so that they support 
>> different mappers for the same list/map? I cannot recall on top of my head. 
>> 
>> If we drop said ability and restrict mappers to the list/map creation, the 
>> whole thing will be much cleaner, and it's a better way to avoid capturing 
>> lambdas as well. Users can still go to individual stable values and use 
>> functional creation if they really, really want that functionality.
>
> I see what you mean with distinct initialization logic. This is not the 
> intended use though. 
> 
> The reason these methods exist is to avoid lambda capturing. Let's say we 
> have a `Function` we want to apply to a `Map>`. Then, 
> retrieving a `stable = StableValue` and applying `stable.computeIfUnset(() 
> -> function.apply(key))` would capture a new `Supplier`. Another alternative 
> would be to write imperative code similar to what is already in these methods.
> 
> What we could do is provide factories for memorized functions (the latter 
> described in the draft JEP at the end (https://openjdk.org/jeps/8312611) 

Re: RFR: 8330465: Stable Values and Collections (Internal)

2024-05-14 Thread Chen Liang
On Tue, 16 Apr 2024 11:47:23 GMT, Per Minborg  wrote:

> # Stable Values & Collections (Internal)
> 
> ## Summary
> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
> which provides immutable value holders where elements are initialized _at 
> most once_. Stable Values & Collections offer the performance and safety 
> benefits of final fields while offering greater flexibility as to the timing 
> of initialization.
> 
> ## Goals
>  * Provide an easy and intuitive API to describe value holders that can 
> change at most once.
>  * Decouple declaration from initialization without significant footprint or 
> performance penalties.
>  * Reduce the amount of static initializer and/or field initialization code.
>  * Uphold integrity and consistency, even in a multi-threaded environment.
>  
> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
> 
> ## Performance 
> Performance compared to instance variables using an `AtomicReference` and one 
> protected by double-checked locking under concurrent access by 8 threads:
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.instanceAtomic  avgt   10  1.576 ? 0.052  ns/op
> StableBenchmark.instanceDCL avgt   10  1.608 ? 0.059  ns/op
> StableBenchmark.instanceStable  avgt   10  0.979 ? 0.023  ns/op <- 
> StableValue (~40% faster than DCL)
> 
> 
> Performance compared to static variables protected by `AtomicReference`, 
> class-holder idiom holder, and double-checked locking (8 threads):
> 
> 
> Benchmark   Mode  Cnt  Score   Error  Units
> StableBenchmark.staticAtomicavgt   10  1.335 ? 0.056  ns/op
> StableBenchmark.staticCHI   avgt   10  0.623 ? 0.086  ns/op
> StableBenchmark.staticDCL   avgt   10  1.418 ? 0.171  ns/op
> StableBenchmark.staticList  avgt   10  0.617 ? 0.024  ns/op
> StableBenchmark.staticStableavgt   10  0.604 ? 0.022  ns/op <- 
> StableValue ( > 2x faster than `AtomicInteger` and DCL)
> 
> 
> Performance for stable lists in both instance and static contexts whereby the 
> sum of random contents is calculated for stable lists (which are thread-safe) 
> compared to `ArrayList` instances (which are not thread-safe) (under single 
> thread access):
> 
> 
> Benchmark Mode  Cnt  Score   Error  Units
> StableListSumBenchmark.instanceArrayList  avgt   10  0.356 ? 0.005  ns/op
> StableListSumBenchmark.instanceList   avgt   10  0.373 ? 0.017  ns/op 
> <- Stable list
> StableListSumBenchmark.staticArrayListavgt   10  0.352 ? 0.002  ns/op
> StableListSumBenchmark.staticList avgt   10  0.356 ? 0.00...

Glad to see this! Some API design remarks.

Also, I want to mention a few important differences between `@Stable` and 
Stable Values:

Patterns:
1. Benign race (does not exist in StableValue API): multiple threads can create 
an instance and upload, any non-null instance is functionally equivalent so 
race is ok (seen in most of JDK)
2. compareAndSet (setIfUnset): multiple threads can create instance, only one 
will succeed in uploading; usually for when the instance computation is cheap 
but we want single witness.
3. atomic computation (computeIfUnset): only one thread can create instance 
which will be witnessed by other threads; this pattern ensures correctness and 
prevents wasteful computation by other threads at the cost of locking and 
lambda creation.

Allocation in objects: `@Stable` field is local to an object but `StableValue` 
is another object; thus sharing strategy may differ, as stable fields are 
copied over but StableValue uses a shared cache (which is even better for 
avoiding redundant computation)

Question:
1. Will we ever try to expose the stable benign race model to users?
2. Will we ever try to inline the stable values in object layout like a stable 
field?

Just curious, can you test other samples, like `StableValue>` 
where the contained `List` is an immutable list from `List.of` factories? I 
think that would be a meaningful case too.

Also on a side note, I just realized there's no equivalent of `@Stable int[]` 
etc. stable primitive arrays exposed, yet immutable arrays will be useful. Is 
the Frozen Arrays JEP still active, or will this Stable Values consider expose 
stable primitive arrays?

src/java.base/share/classes/java/lang/reflect/Field.java line 179:

> 177: AccessibleObject.checkPermission();
> 178: // Always check if the field is a final StableValue
> 179: if (StableValue.class.isAssignableFrom(type) && 
> Modifier.isFinal(modifiers)) {

This doesn't protect the Stable Collections.

src/java.base/share/classes/java/util/ImmutableCollections.java line 173:

> 171: .map(Objects::requireNonNull)
> 172: .toArray();
> 173: return keys instanceof EnumSet

We can move this instanceof check before the stream call.