Re: RFR: 8337712: Wrong javadoc in java.util.Date#toString(): "61" and right parenthesis
On Fri, 2 Aug 2024 07:24:20 GMT, Per Minborg wrote:
>> This trivial PR proposes to add a missing parenthesis in
>> `java.util.Date::toString`.
>
> src/java.base/share/classes/java/util/Date.java line 1015:
>
>> 1013: * {@code 59}), as two decimal digits.
>> 1014: * {@code ss} is the second within the minute ({@code 00}
>> through
>> 1015: * {@code 61}), as two decimal digits.
>
> The doc mentions seconds could be in the range [0, 61]. The normal interval
> should be [0, 59] but maybe there are situations where leap seconds are
> inserted (see https://en.wikipedia.org/wiki/Leap_second). Historically, only
> one second has been used so I wonder why it says "61"?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20439#discussion_r1701429707
RFR: 8337712: Wrong javadoc in java.util.Date#toString(): "61" and right parenthesis
This trivial PR proposes to add a missing parenthesis in `java.util.Date::toString`. - Commit messages: - Update copyright year - Add missing parenthesis in Date::toString Changes: https://git.openjdk.org/jdk/pull/20439/files Webrev: https://webrevs.openjdk.org/?repo=jdk=20439=00 Issue: https://bugs.openjdk.org/browse/JDK-8337712 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/20439.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20439/head:pull/20439 PR: https://git.openjdk.org/jdk/pull/20439
Re: RFR: 8337712: Wrong javadoc in java.util.Date#toString(): "61" and right parenthesis
On Fri, 2 Aug 2024 07:20:39 GMT, Per Minborg wrote:
> This trivial PR proposes to add a missing parenthesis in
> `java.util.Date::toString`.
src/java.base/share/classes/java/util/Date.java line 1015:
> 1013: * {@code 59}), as two decimal digits.
> 1014: * {@code ss} is the second within the minute ({@code 00}
> through
> 1015: * {@code 61}), as two decimal digits.
The doc mentions seconds could be in the range [0, 61]. The normal interval
should be [0, 59] but maybe there are situations where leap seconds are
inserted (see https://en.wikipedia.org/wiki/Leap_second). Historically, only
one second has been used so I wonder why it says "61"?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20439#discussion_r1701412451
Integrated: 8333884: MemorySegment::reinterpret removes read-only property
On Mon, 10 Jun 2024 13:18:43 GMT, Per Minborg wrote: > This PR proposes to retain the read-only state when any of the > `MemorySegment::reinterpret` methods is called. > > Previously, the read-only state was lost and the returned `MemorySegment` was > always writable regardless of the original segment's read-only state. This pull request has now been integrated. Changeset: 6f7f0f1d Author:Per Minborg URL: https://git.openjdk.org/jdk/commit/6f7f0f1de05fdc0f6a88ccd90b806e8a5c5074ef Stats: 38 lines in 4 files changed: 30 ins; 1 del; 7 mod 8333884: MemorySegment::reinterpret removes read-only property Reviewed-by: jvernee, mcimadamore - PR: https://git.openjdk.org/jdk/pull/19629
Re: RFR: 8333884: MemorySegment::reinterpret removes read-only property [v4]
> This PR proposes to retain the read-only state when any of the > `MemorySegment::reinterpret` methods is called. > > Previously, the read-only state was lost and the returned `MemorySegment` was > always writable regardless of the original segment's read-only state. Per Minborg has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits: - Merge master - Add read-only comment on asSegment too - Rework docs - Make reinterpret retain read-only state - Changes: https://git.openjdk.org/jdk/pull/19629/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19629=03 Stats: 38 lines in 4 files changed: 30 ins; 1 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/19629.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19629/head:pull/19629 PR: https://git.openjdk.org/jdk/pull/19629
Integrated: 8333886: Explicitly specify that asSlice and reinterpret return a memory segment backed by the same region of memory.
On Mon, 10 Jun 2024 15:45:07 GMT, Per Minborg wrote: > This PR proposes to explicitly state that returned segments form the > `asSlice` and `reinterpret` method share memory regions with `this` memory > segment. > > Note: The term "subset" means a true subset or the same set. This pull request has now been integrated. Changeset: c80e2eb3 Author:Per Minborg URL: https://git.openjdk.org/jdk/commit/c80e2eb35c4eb03f17a2a31e979e5c369453e203 Stats: 21 lines in 1 file changed: 21 ins; 0 del; 0 mod 8333886: Explicitly specify that asSlice and reinterpret return a memory segment backed by the same region of memory. Reviewed-by: jvernee, mcimadamore - PR: https://git.openjdk.org/jdk/pull/19633
Re: RFR: 8333886: Explicitly specify that asSlice and reinterpret return a memory segment backed by the same region of memory. [v2]
> This PR proposes to explicitly state that returned segments form the > `asSlice` and `reinterpret` method share memory regions with `this` memory > segment. > > Note: The term "subset" means a true subset or the same set. Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Change wording in doc updates - Changes: - all: https://git.openjdk.org/jdk/pull/19633/files - new: https://git.openjdk.org/jdk/pull/19633/files/6cde00c8..13f33e65 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19633=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19633=00-01 Stats: 14 lines in 1 file changed: 0 ins; 0 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/19633.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19633/head:pull/19633 PR: https://git.openjdk.org/jdk/pull/19633
Re: RFR: 8333886: Explicitly specify that asSlice and reinterpret return a memory segment backed by the same region of memory.
On Mon, 10 Jun 2024 15:45:07 GMT, Per Minborg wrote: > This PR proposes to explicitly state that returned segments form the > `asSlice` and `reinterpret` method share memory regions with `this` memory > segment. > > Note: The term "subset" means a true subset or the same set. > In my opinion, any 'sub' terminology is misleading since, as you pointed out, > one can (and often will) `reinterpret` to a _larger_ size. Wouldn't something > like, "The returned memory segment is backed by the same region of memory > that backs this memory segment", be sufficiently accurate? Could even append, > "No memory will be allocated or deallocated", to really hammer the point home. > > Using 'subset' for `asSlice` makes sense. Though possibly unnecessary. If a segment is reinterpreted to be *larger* than `this` segment, then the extra memory is not a part of `this` segment's backing part. So, in this case the total memory is a subset (but not a _proper subset_ but rather the same) of `this` backing memory _plus_ some additional memory. If a segment is reinterpreted to be *smaller* than `this` segment, then the reduced memory is a proper subset. So, strictly the proposed text is correct. The proposition "The returned memory segment is backed by the same region of memory that backs this memory segment" appears to be incorrect if a _smaller_ chunk is carved out? Maybe something more neutral like "The returned memory segment shares a region of backing memory with this segment"? Adding text similar to "No memory will be allocated or deallocated" sounds like a good idea. - PR Comment: https://git.openjdk.org/jdk/pull/19633#issuecomment-2160213978
Re: RFR: 8316493: Remove the caching fields in AbstractMap [v11]
On Fri, 10 Nov 2023 08:17:22 GMT, Per Minborg wrote: >> This PR outlines a solution for making immutable maps `@ValueBased` by >> removing cacheing of certain values in `AbstractMap`. >> >> By removing these caching fields in `AbstractMap`, we can make the immutable >> maps `@ValueBased` and at the same time, performance is likely improved >> because the JVM is probably able to optimize away object creation anyway via >> escape analysis. Also, all maps will occupy less space as we get rid of a >> number of objects and references stored for each map. >> >> We need to benchmark this solution to better understand its implications. > > Per Minborg has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 14 commits: > > - Merge branch 'master' into vb-map2 > - Fix formatting > - Remove caching in TreeMap > - Remove caching from CHM and CSLM > - Move back clone to original position > - Reintroduce AbstractMap::clone > - Add 'fresh' to implSpec > - Remove AbstractMap::clone > - Merge master > - Merge branch 'master' into vb-map2 > - ... and 4 more: https://git.openjdk.org/jdk/compare/9cce9fe0...b1bfcd17 Keep alive - PR Comment: https://git.openjdk.org/jdk/pull/15614#issuecomment-2158838349
RFR: 8333886: Explicitly specify that asSlice and reinterpret return a memory segment backed by the same region of memory.
This PR proposes to explicitly state that returned segments form the `asSlice` and `reinterpret` method share memory regions with `this` memory segment. Note: The term "subset" means a true subset or the same set. - Commit messages: - Add segments share the same backing store in docs Changes: https://git.openjdk.org/jdk/pull/19633/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19633=00 Issue: https://bugs.openjdk.org/browse/JDK-8333886 Stats: 21 lines in 1 file changed: 21 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19633.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19633/head:pull/19633 PR: https://git.openjdk.org/jdk/pull/19633
Re: RFR: 8333884: MemorySegment::reinterpret removes read-only property [v3]
> This PR proposes to retain the read-only state when any of the > `MemorySegment::reinterpret` methods is called. > > Previously, the read-only state was lost and the returned `MemorySegment` was > always writable regardless of the original segment's read-only state. Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Add read-only comment on asSegment too - Changes: - all: https://git.openjdk.org/jdk/pull/19629/files - new: https://git.openjdk.org/jdk/pull/19629/files/538eb2ed..bc0ade8a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19629=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19629=01-02 Stats: 19 lines in 1 file changed: 13 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/19629.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19629/head:pull/19629 PR: https://git.openjdk.org/jdk/pull/19629
Re: RFR: 8333884: MemorySegment::reinterpret removes read-only property [v2]
On Mon, 10 Jun 2024 13:45:16 GMT, Maurizio Cimadamore wrote: > Note that `asSlice` methods also lacks a similar guarantee on read-only (but > the impl works fine there) Even though not mentioned in the original issue, we could add documentation for this here as well. Or maybe via https://bugs.openjdk.org/browse/JDK-8333886 ? - PR Comment: https://git.openjdk.org/jdk/pull/19629#issuecomment-2158628430
Re: RFR: 8333884: MemorySegment::reinterpret removes read-only property [v2]
> This PR proposes to retain the read-only state when any of the > `MemorySegment::reinterpret` methods is called. > > Previously, the read-only state was lost and the returned `MemorySegment` was > always writable regardless of the original segment's read-only state. Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Rework docs - Changes: - all: https://git.openjdk.org/jdk/pull/19629/files - new: https://git.openjdk.org/jdk/pull/19629/files/acebad9b..538eb2ed Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19629=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19629=00-01 Stats: 29 lines in 1 file changed: 9 ins; 0 del; 20 mod Patch: https://git.openjdk.org/jdk/pull/19629.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19629/head:pull/19629 PR: https://git.openjdk.org/jdk/pull/19629
RFR: 8333884: MemorySegment::reinterpret removes read-only property
This PR proposes to retain the read-only state when any of the `MemorySegment::reinterpret` methods is called. Previously, the read-only state was lost and the returned `MemorySegment` was always writable regardless of the original segment's read-only state. - Commit messages: - Make reinterpret retain read-only state Changes: https://git.openjdk.org/jdk/pull/19629/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19629=00 Issue: https://bugs.openjdk.org/browse/JDK-8333884 Stats: 37 lines in 4 files changed: 9 ins; 1 del; 27 mod Patch: https://git.openjdk.org/jdk/pull/19629.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19629/head:pull/19629 PR: https://git.openjdk.org/jdk/pull/19629
Withdrawn: 8330465: Stable Values and Collections (Internal)
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 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... This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/18794
Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]
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: 8310843: Reimplement ByteArray and ByteArrayLittleEndian with Unsafe [v2]
On Mon, 10 Jun 2024 03:51:40 GMT, Glavo wrote: >> Things have changed since https://github.com/openjdk/jdk/pull/14636 was >> closed, so let me reopen it. >> >> https://github.com/openjdk/jdk/pull/15386 confirmed that `VarHandle` in BALE >> caused a startup regression. In order to not have any more revert patches, >> it makes sense to optimize BALE. >> >> The optimization of https://github.com/openjdk/jdk/pull/16245 allows the >> traditional expression to have good performance, but BA and BALE save us >> from having to copy these lengthy expressions everywhere. So it makes sense >> to keep them. >> >> Now here's the question, should I rewrite this PR without `Unsafe`? I'll do >> some research (e.g. does `Unsafe` have better performance during warmup?), >> but I'd also like to take some advice. > > Glavo has updated the pull request incrementally with one additional commit > since the last revision: > > update copyright Would it be possible to add a benchmark for some of the methods here so we can see if there is a difference? Also, it would be interesting to see if startup performance is impacted. - PR Comment: https://git.openjdk.org/jdk/pull/19616#issuecomment-2157489839
Re: RFR: 8333774: Avoid eagerly loading various EmptySpliterator classes [v3]
On Fri, 7 Jun 2024 13:08:24 GMT, Claes Redestad wrote: >> Trivially move a few private static finals to their respective source type >> to avoid eagerly loading a few small classes. > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Update src/java.base/share/classes/java/util/Spliterators.java > > Co-authored-by: Chen Liang Looks good to me. Nice observation, the laziness was for free as existing classes could be reused. - Marked as reviewed by pminborg (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19591#pullrequestreview-2104630412
Re: RFR: 8332249: Micro-optimize Method.hashCode [v2]
On Wed, 5 Jun 2024 13:41:10 GMT, Per Minborg wrote: >> In order to be eligible for constant folding, the benchmark must declare the >> `Method hashCodeMethod;` as `static final`. >> >> It is hard for me to understand why a `@Stable` annotation should have a >> detrimental performance impact on an instance field. >> >> ~~Can we see a benchmark on Arm Neoverse N1 with the field declared >> `@Stable` compared to not declared `@Stable`? ~~ Also, if the field is >> `static final`, how would it look like? > > As a note, If we would have access to the contemplated `StableValue` and > `hash` was declared even as an _instance variable_ `StableValue` in > a regular class, then it would be trusted and would be eligible for constant > folding due to the VM will have special rules for fields of StableValue type. Ahh. There was a benchmark in the initial message. Sorry. - PR Review Comment: https://git.openjdk.org/jdk/pull/19433#discussion_r1627818122
Re: RFR: 8332249: Micro-optimize Method.hashCode [v2]
On Wed, 5 Jun 2024 13:37:15 GMT, Per Minborg wrote: >> Interesting, don't know about hotspot internals so I can't diagnose the >> exact cause of this regression. > > In order to be eligible for constant folding, the benchmark must declare the > `Method hashCodeMethod;` as `static final`. > > It is hard for me to understand why a `@Stable` annotation should have a > detrimental performance impact on an instance field. > > Can we see a benchmark on Arm Neoverse N1 with the field declared `@Stable` > compared to not declared `@Stable`? Also, if the field is `static final`, how > would it look like? As a note, If we would have access to the contemplated `StableValue` and `hash` was declared even as an _instance variable_ `StableValue` in a regular class, then it would be trusted and would be eligible for constant folding due to the VM will have special rules for fields of StableValue type. - PR Review Comment: https://git.openjdk.org/jdk/pull/19433#discussion_r1627810415
Re: RFR: 8332249: Micro-optimize Method.hashCode [v2]
On Wed, 29 May 2024 15:20:15 GMT, Chen Liang wrote: >> This was something that I tried and I observed a performance regression on >> the ARM platform that I was testing. From my understanding, `@Stable` tells >> the compiler to trust the contents of this field which is only given when >> the field holder is a known constant. This is generally true for cases like >> `Enums` but not usually for `Method`. >> >> >> Benchmark Mode Cnt Score Error Units >> # Intel Skylake >> MethodHashCode.benchmarkHashCode avgt5 1.113 ± 1.146 ns/op >> # Arm Neoverse N1 >> MethodHashCode.benchmarkHashCode avgt5 3.204 ± 0.001 ns/op > > Interesting, don't know about hotspot internals so I can't diagnose the exact > cause of this regression. In order to be eligible for constant folding, the benchmark must declare the `Method hashCodeMethod;` as `static final`. It is hard for me to understand why a `@Stable` annotation should have a detrimental performance impact on an instance field. Can we see a benchmark on Arm Neoverse N1 with the field declared `@Stable` compared to not declared `@Stable`? Also, if the field is `static final`, how would it look like? - PR Review Comment: https://git.openjdk.org/jdk/pull/19433#discussion_r1627803876
Re: RFR: 8333236: Test java/foreign/TestAccessModes.java is timing out after passing [v2]
On Fri, 31 May 2024 16:18:33 GMT, Maurizio Cimadamore wrote: >> This PR restores a var handle cache in `Utils::makeSegmentViewVarHandle`. >> The cache was moved to `ValueLayouts::varHandle` as part of >> [pull/19251](https://git.openjdk.org/jdk/pull/19251), on the basis that we >> want to optimize the common case like: >> >> >> ValueLayout layout = ... >> layout.varHandle().get(...) >> >> >> And that caching more complex var handles didn't seem to add value, given >> that, for these var handles, the logic in `LayoutPath` needs to adapt the >> returned var handle anyways. >> >> But, `TestAccessModes` revealed a different picture - w/o any cache in >> `Utils` the test end up allocating 8963 var handle instances (instead of >> just 4), in each of the 4 runs the test includes. While this is admittedly a >> stress test, it seems nice to restore the level of sharing we had before >> [pull/19251](https://git.openjdk.org/jdk/pull/19251). > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comments Would it make sense to add some tests that assert, `VarHandle` objects get cached? - PR Comment: https://git.openjdk.org/jdk/pull/19485#issuecomment-2142604322
Re: RFR: 8292955: Collections.checkedMap Map.merge does not properly check key and value [v3]
On Thu, 7 Mar 2024 05:33:16 GMT, Lei Zhu wrote: >> When the specified key did not associated with a value, should check the >> `key` and `value` type. > > Lei Zhu has updated the pull request incrementally with one additional commit > since the last revision: > > Use testNG builtin functionalities and modify the test function name Adding the checks before remapping seems the right thing to do in order to uphold the checked maps guarantees on types and values. - Marked as reviewed by pminborg (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18141#pullrequestreview-2081881249
Re: RFR: 8292955: Collections.checkedMap Map.merge does not properly check key and value [v3]
On Mon, 27 May 2024 12:40:07 GMT, Chen Liang wrote: > @minborg Is it possible for you to review this collection bugfix? Will do! Thanks for the heads up. - PR Comment: https://git.openjdk.org/jdk/pull/18141#issuecomment-2134446236
Re: RFR: 8328821: Map.of().entrySet() mutators should throw UnsupportedOperationException
On Wed, 27 Mar 2024 17:36:28 GMT, Liam Miller-Cushon wrote:
> This change overrides mutator methods in the implementation returned by
> `Map.of().entrySet()` to throw `UnsupportedOperationException`.
src/java.base/share/classes/java/util/ImmutableCollections.java line 1323:
> 1321: @Override
> 1322: public int hashCode() {
> 1323: return MapN.this.hashCode();
The hash code for a `Set` is defined as the sum of the elements in the `Set`
(hash(`null`) == 0). The `Map. Entry` hash code is defined the same way
`MapN.this.hashCode` operates so this seems right. It would be nice with a
couple of tests that assert this invariant.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18522#discussion_r1615591889
Integrated: 8332749: Broken link in MemorySegment.Scope.html
On Thu, 23 May 2024 11:39:11 GMT, Per Minborg wrote: > This PR proposes to fix a broken link in the `MemorySegment.Scope` class. This pull request has now been integrated. Changeset: 0a9d1f8c Author: Per Minborg URL: https://git.openjdk.org/jdk/commit/0a9d1f8c89e946d99f01549515f6044e53992168 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8332749: Broken link in MemorySegment.Scope.html Reviewed-by: iris - PR: https://git.openjdk.org/jdk/pull/19366
Re: RFR: 8332749: Broken link in MemorySegment.Scope.html
On Thu, 23 May 2024 11:39:11 GMT, Per Minborg wrote: > This PR proposes to fix a broken link in the `MemorySegment.Scope` class. Seams to work after the patch:  .../api/java.base/java/lang/foreign/MemorySegment.html#wrapping-addresses - PR Comment: https://git.openjdk.org/jdk/pull/19366#issuecomment-2126976574
RFR: 8332749: Broken link in MemorySegment.Scope.html
This PR proposes to fix a broken link in the `MemorySegment.Scope` class. - Commit messages: - Fix link to section Changes: https://git.openjdk.org/jdk/pull/19366/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19366=00 Issue: https://bugs.openjdk.org/browse/JDK-8332749 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19366.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19366/head:pull/19366 PR: https://git.openjdk.org/jdk/pull/19366
Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]
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]
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):  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]
> # 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]
> # 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]
> # 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]
> # 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]
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: > <https://mail.openjdk.org/pipermail/compiler-dev/attachments/20240516/7fa869a5/attachment-0001.htm> `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]
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):  - PR Comment: https://git.openjdk.org/jdk/pull/18794#issuecomment-2115343828
Re: RFR: 8331670: Deprecate the Memory-Access Methods in sun.misc.Unsafe for Removal [v2]
On Thu, 16 May 2024 07:14:53 GMT, Alan Bateman wrote: >> This is the implementation changes for JEP 471. >> >> The methods in sun.misc.Unsafe for on-heap and off-heap access are >> deprecated for removal. This means a removal warning at compile time. No >> methods have been removed. A deprecated message is added to each of the >> methods but unlikely to be seen as the JDK does not generate or publish the >> API docs for this class. >> >> A new command line option --sun-misc-unsafe-memory-access=$value is >> introduced to allow or deny access to these methods. The default proposed >> for JDK 23 is "allow" so no change in behavior compared to JDK 22 or >> previous releases. >> >> A new test is added to test the command line option settings. The existing >> micros for FFM that use Unsafe are updated to suppress the removal warning >> at compile time. A new micro is introduced with a small sample of methods to >> ensure the changes doesn't cause any perf regressions. >> >> For now, the changes include the update to the man page for the "java" >> command. It might be that this has to be separated out so that it goes with >> other updates in the release. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains ten additional > commits since the last revision: > > - Add removal to DISABLED_WARNINGS >Fail at startup if bad value specified to option > - Merge > - Update man page > - Update man page > - Fix comment > - Merge > - Merge > - Whitespace > - Initial commit Looks good. It appears the JVM will be able to constant-fold away the `beforeMemoryAcess()` checks but, it would be nice to see the output of the new benchmark for; before PR, `MemoryAccessOption.ALLOW` and `MemoryAccessOption.WARN`. - Marked as reviewed by pminborg (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19174#pullrequestreview-2060709939
Re: RFR: 8330465: Stable Values and Collections (Internal) [v16]
> # 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]
> # 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]
> # 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]
> # 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]
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]
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) [v12]
> # 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: 8331865: Consolidate size and alignment checks in LayoutPath
On Wed, 15 May 2024 15:43:45 GMT, Maurizio Cimadamore
wrote:
> When creating a nested memory access var handle, we ensure that the segment
> is accessed at the correct alignment for the root layout being accessed. But
> we do not ensure that the segment has at least the size of the accessed root
> layout. Example:
>
>
> MemoryLayout LAYOUT = sequenceLayout(2, structLayout(JAVA_INT.withName("x"),
> JAVA_INT.withName("y")));
> VarHandle X_HANDLE = LAYOUT.varHandle(PathElement.sequenceElement(),
> PathElement.groupElement("x"));
>
>
> If I have a memory segment whose size is 12, I can successfully call X_HANDLE
> on it with index 1, like so:
>
>
> MemorySegment segment = Arena.ofAuto().allocate(12);
> int x = (int)X_HANDLE.get(segment, 0, 1);
>
>
> This seems incorrect: after all, the provided segment doesn't have enough
> space to fit _two_ elements of the nested structs.
>
> This PR adds checks to detect this kind of scenario earlier, thus improving
> usability. To achieve this we could, in principle, just tweak
> `LayoutPath::checkEnclosingLayout` and add the required size check.
>
> But doing so will add yet another redundant check on top of the other already
> added by [pull/19124](https://git.openjdk.org/jdk/pull/19124). Instead, this
> PR rethinks how memory segment var handles are created, in order to eliminate
> redundant checks.
>
> The main observation is that size and alignment checks depend on an
> _enclosing_ layout. This layout *might* be the very same value layout being
> accessed (this is the case when e.g. using `JAVA_INT.varHandle()`). But, in
> the general case, the accessed value layout and the enclosing layout might
> differ (e.g. think about accessing a struct field).
>
> Furthermore, the enclosing layout check depends on the _base_ offset at which
> the segment is accessed, _prior_ to any index computation that occurs if the
> accessed layout path has any open elements. It is important to notice that
> this base offset is only available when looking at the var handle that is
> returned to the user. For instance, an indexed var handle with coordinates:
>
>
> (MemorySegment, long /* base */, long /* index 1 */, long /* index 2 */, long
> /* index 3 */)
>
>
> Is obtained through adaptation, by taking a more basic var handle of the form:
>
>
> (MemorySegment, long /* offset */)
>
>
> And then injecting the result of the index multiplication into `offset`. As
> such, we can't add an enclosing layout check inside the var handle returned
> by `VarHandles::memorySegmentViewHandle`, as doing so will end up seeing the
> *wrong* offset (e.g. an offset in which the index part has already been mixed
> in)...
Do we have any performance tests available to see if there are any potential
impacts?
src/java.base/share/classes/java/lang/invoke/VarHandleSegmentViewBase.java line
48:
> 46: final long alignmentMask;
> 47:
> 48: VarHandleSegmentViewBase(VarForm form, boolean be, long
> alignmentMask, boolean exact) {
Nit: Copyright year. This also applies to some other files in this PR.
-
PR Comment: https://git.openjdk.org/jdk/pull/19251#issuecomment-2114955648
PR Review Comment: https://git.openjdk.org/jdk/pull/19251#discussion_r1603156019
Re: RFR: 8330465: Stable Values and Collections (Internal) [v11]
> # 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]
> # 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]
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]
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]
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]
> # 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]
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]
> # 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]
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]
> # 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]
> # 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]
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]
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]
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]
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]
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]
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]
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]
> # 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]
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]
> # 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]
> # 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]
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]
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]
> # 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: 8305457: Implement java.io.IO [v9]
On Mon, 13 May 2024 09:56:35 GMT, Pavel Rappo wrote:
>> Please review this PR which introduces the `java.io.IO` top-level class and
>> three methods to `java.io.Console` for [Implicitly Declared Classes and
>> Instance Main Methods (Third Preview)].
>>
>> This PR has been obtained as `git merge --squash` of a now obsolete [draft
>> PR].
>>
>> [Implicitly Declared Classes and Instance Main Methods (Third Preview)]:
>> https://bugs.openjdk.org/browse/JDK-8323335
>> [draft PR]: https://github.com/openjdk/jdk/pull/18921
>
> Pavel Rappo has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains 17 additional
> commits since the last revision:
>
> - Escape prompt
> - Merge branch 'master' into 8305457-Implement-java.io.IO
> - Clarify input charset
> - Make IO final
> - Fix System.console().readln(null) in jshell
>
>Without it, jshell hangs on me. Will think of a test.
> - Fix typo
> - Merge branch 'master' into 8305457-Implement-java.io.IO
> - Simplify output.exp
> - Cover null prompt in input tests
> - Make input test parametric
> - ... and 7 more: https://git.openjdk.org/jdk/compare/2f44ac18...17100ab8
src/java.base/share/classes/java/io/IO.java line 98:
> 96: * or if an I/O error occurs
> 97: */
> 98: public static String readln(String prompt) {
Did we consider Optional here? Maybe that is to complicated for an
onramp?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19112#discussion_r1600145844
Re: RFR: 8330465: Stable Values and Collections (Internal)
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)
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)
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)
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)
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)
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)
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)
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)
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
RFR: 8330465: Stable Values and Collections (Internal)
# 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.003 ns/op <- Stable list Performance for stable maps in a static context compared to a `ConcurrentHashMap` (under single thread access): Benchmark Mode Cnt Score Error Units StablePropertiesBenchmark.chmRaw avgt 10 3.416 ? 0.031 ns/op StablePropertiesBenchmark.mapRaw avgt 10 2.105 ? 0.012 ns/op <- Stable map (~40% faster) All figures above are from local tests on a Mac M1 laptop and should only be constructed as indicative figures. ## Implementation details There are some noteworthy implementation details in this PR: * A field is _trusted_ if it is _declared_ as a `final StableValue`. Previously, the determination of trustworthiness was connected to the _class in which it was declared_ (e.g. is it a `record` or a hidden class). In order to grant such trust, there are extra restrictions imposed on reflection and `sun.misc.Unsafe` usage for such declared `StableValue` fields. This is similar to how `record` classes are handled. * In order to allow plain memory semantics for read operations across threads (rather than `volatile` semantics which is slower and which is normally required for double-checked-locking access), we perform a _freeze_ operation before an object becomes visible to other threads. This will prevent store-store reordering and hence, we are able to guarantee complete objects are always seen even under plain memory semantics. * In collections with `StableValue` elements/values, a transient `StableValue` view backed by internal arrays is created upon read operations. This improves initialization time, reduces storage requirements, and improves access performance as these transient objects are eliminated by the C2 compiler. - Commit messages: - Merge branch 'master' into stable-value - Rework the creation of StableEnumMaps - Update sun.misc.Unsafe - Fix error in hash code - Add methods to create generic arrays - Change class types - Add a marker interface TrustedFieldType - Improve array test - Clean up tests - Add tests - ... and 162 more: https://git.openjdk.org/jdk/compare/4ba74475...5d5dcced Changes: https://git.openjdk.org/jdk/pull/18794/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18794=00 Issue: https://bugs.openjdk.org/browse/JDK-8330465 Stats: 5733 lines in 39 files changed: 5708 ins; 13 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/18794.diff Fetch: git fetch
Re: RFR: 8330465: Stable Values and Collections (Internal)
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)
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: 8331670: Deprecate the Memory-Access Methods in sun.misc.Unsafe for Removal
On Fri, 10 May 2024 10:06:55 GMT, Alan Bateman wrote: > This is the implementation changes for JEP 471. > > The methods in sun.misc.Unsafe for on-heap and off-heap access are deprecated > for removal. This means a removal warning at compile time. No methods have > been removed. A deprecated message is added to each of the methods but > unlikely to be seen as the JDK does not generate or publish the API docs for > this class. > > A new command line option --sun-misc-unsafe-memory-access=$value is > introduced to allow or deny access to these methods. The default proposed for > JDK 23 is "allow" so no change in behavior compared to JDK 22 or previous > releases. > > A new test is added to test the command line option settings. The existing > micros for FFM that use Unsafe are updated to suppress the removal warning at > compile time. A new micro is introduced with a small sample of methods to > ensure the changes doesn't cause any perf regressions. > > For now, the changes include the update to the man page for the "java" > command. It might be that this has to be separated out so that it goes with > other updates in the release. Some of the deprecated methods are very likely to be run in hot loops (e.g. read/store operations). Unless we set `--sun-misc-unsafe-memory-access=allow`, what would be the performance impact on various platforms for these operations? - PR Comment: https://git.openjdk.org/jdk/pull/19174#issuecomment-2107216975
Re: RFR: 8331670: Deprecate the Memory-Access Methods in sun.misc.Unsafe for Removal
On Fri, 10 May 2024 10:06:55 GMT, Alan Bateman wrote: > This is the implementation changes for JEP 471. > > The methods in sun.misc.Unsafe for on-heap and off-heap access are deprecated > for removal. This means a removal warning at compile time. No methods have > been removed. A deprecated message is added to each of the methods but > unlikely to be seen as the JDK does not generate or publish the API docs for > this class. > > A new command line option --sun-misc-unsafe-memory-access=$value is > introduced to allow or deny access to these methods. The default proposed for > JDK 23 is "allow" so no change in behavior compared to JDK 22 or previous > releases. > > A new test is added to test the command line option settings. The existing > micros for FFM that use Unsafe are updated to suppress the removal warning at > compile time. A new micro is introduced with a small sample of methods to > ensure the changes doesn't cause any perf regressions. > > For now, the changes include the update to the man page for the "java" > command. It might be that this has to be separated out so that it goes with > other updates in the release. Would it make sense to add some verbiage in the JavaDocs for `sun.misc.Unsafe` that indicates the planned direction for said class and the use of the new command line options? - PR Comment: https://git.openjdk.org/jdk/pull/19174#issuecomment-2106795327
Re: RFR: 8305457: Implement java.io.IO [v7]
On Tue, 7 May 2024 20:23:11 GMT, Pavel Rappo wrote: >>> Should this be final? >> >> With only the private constructor, it doesn't matter too much in practice, >> but an explicit `final` would be good documentation if that is the intent. > > If the sole constructor of a class is private, not only does it preclude the > class from being instantiated, but it also precludes the class from being > extended. Mind you, even with the sole private constructor, both > instantiation and extension are still possible from inside the class or its > nested classes. As long as we don't leak such instances to API clients, they > won't seem to be able to observe any difference between "the private > constructor" and "the private constructor of a final class". > > I think that just having that constructor private but the class "non-final" > makes bigger bang for the buck. We can defer the decision to make the class > final. > > If this helps, here are a few well-known similar classes: > > - java.util.Collections > - java.util.concurrent.Executors Although true (and also described in Item 4 in "Effective Java" 3rd ed by JB) I think it is better to also signal the intent by marking the class as `final`. See for example Arrays that we updated some time ago. - PR Review Comment: https://git.openjdk.org/jdk/pull/19112#discussion_r1596377720
Re: RFR: 8305457: Implement java.io.IO [v7]
On Thu, 9 May 2024 18:27:08 GMT, Pavel Rappo wrote:
>> Please review this PR which introduces the `java.io.IO` top-level class and
>> three methods to `java.io.Console` for [Implicitly Declared Classes and
>> Instance Main Methods (Third Preview)].
>>
>> This PR has been obtained as `git merge --squash` of a now obsolete [draft
>> PR].
>>
>> [Implicitly Declared Classes and Instance Main Methods (Third Preview)]:
>> https://bugs.openjdk.org/browse/JDK-8323335
>> [draft PR]: https://github.com/openjdk/jdk/pull/18921
>
> Pavel Rappo has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix System.console().readln(null) in jshell
>
> Without it, jshell hangs on me. Will think of a test.
src/java.base/share/classes/java/io/IO.java line 47:
> 45:
> 46: private IO() {
> 47: throw new Error("no instances");
Is this necessary?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19112#discussion_r1596370177
Re: RFR: 8332003: Clarify javadoc for MemoryLayout::offsetHandle
On Thu, 9 May 2024 15:00:35 GMT, Maurizio Cimadamore
wrote:
> Consider this layout:
>
>
> MemoryLayout SEQ = MemoryLayout.sequenceLayout(5,
> MemoryLayout.sequenceLayout(10, JAVA_INT));
>
>
> And the corresponding offset handle:
>
>
> MethodHandle offsetHandle = SEQ.offsetHandle(PathElement.sequenceLayout(),
> PathElement.sequenceLayout());
>
>
> The resulting method handle takes two additional `long` indices. The
> implementation checks that the dynamically provided indices conform to the
> bound available at construction: that is, the first index must be < 5, while
> the second must be < 10. If this is not true, an `IndexOutOfBoundException`
> is thrown.
>
> However, the javadoc for `MemoryLayout::byteOffsetHandle` doesn't claim
> anything in this direction. There are only some vague claims in the javadoc
> for `PathElement::sequenceElement()` and `PathElement::sequenceElement(long,
> long, long)` which make some claims on which indices are actually allowed,
> but the text seems more in the tone of a discussion, rather than actual
> normative text.
>
> I've tweaked the javadoc for `MemoryLayout::byteOffsetHandle` to actually
> state that the indices will be checked against the "size" of the
> corresponding open path element (this is a new concept that I also have
> defined in the javadoc).
>
> I also added a test for `byteOffsetHandle` as I don't think we had a test for
> that specifically (although this method is tested indirectly, via
> `MemoryLayout::varHandle`).
src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 616:
> 614: *
> 615: * {@snippet lang = "java":
> 616: * O = this.byteOffsetHandle(P).invokeExact(B, I1, I2, ... In);
Good catch
test/jdk/java/foreign/TestLayoutPaths.java line 36:
> 34:
> 35: import java.lang.invoke.MethodHandle;
> 36: import java.lang.invoke.VarHandle;
Nit: Copyright year
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19158#discussion_r1595624831
PR Review Comment: https://git.openjdk.org/jdk/pull/19158#discussion_r1595624353
Re: RFR: 8331734: Atomic MemorySegment VarHandle operations fails for element layouts
On Tue, 7 May 2024 15:42:23 GMT, Maurizio Cimadamore wrote: > This PR fixes an issue that has crept into the FFM API implementation. > > From very early stages, the FFM API used to disable the alignment check on > nested layout elements, in favor of an alignment check against the memory > segment base address. The rationale was that the JIT had issue with > eliminating redundant alignment checks, and accessing nested elements could > never result in alignment issues, since the alignment of the container is > provably bigger than that of the contained element. This means that, when > creating a var handle for a nested layout element, we set the nested layout > alignment to 1 (unaligned), derive its var handle, and then decorate the var > handle with an alignment check for the container. > > At some point in 22, we tweaked the API to throw > `UnsupportedOperationException` when using an access mode incompatible with > the alignment constraint of the accessed layout. That is, a volatile read on > an `int` is only possible if the access occurs at an address that is at least > 4-byte aligned. Otherwise an `UOE` is thrown. > > Unfortunately this change broke the aforementioned optimization: creating a > var handle for an unaligned layout works, but the resulting layout will *not* > support any of the atomic access modes. > > Since this optimization is not really required anymore (proper C2 support to > hoist/eliminate alignment checks has been added since then), it is better to > disable this implementation quirk, and leave optimizations to C2. > > (If we really really wanted to optimize things a bit, we could remove the > container alignment check in the case the accessed value is the first layout > element nested in the container, but this PR doesn't go that far). > > I've run relevant benchmarks before/after and found no differences. In part > this is because `arrayElementVarHandle` is unaffected. But, even after > tweaking the `LoopOverNonConstant` benchmark to add explicit tests for the > code path affected, no significant difference was found, sign that C2 is > indeed able to spot (and remove) the redundant alignment check. Note: if we > know that `aligned_to_N(base)` holds, then it's easy to prove that > `aligned_to_M(base + offset + index * scale)` holds, when `N >= M` and with > `offset` and `scale` known (the latter a power of two). LGTM. As mentioned in the PR notes, additional optimizations could be made and this could be the objective of a future PR. - Marked as reviewed by pminborg (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19124#pullrequestreview-2043680093
Re: RFR: 8316493: Remove the caching fields in AbstractMap [v11]
On Fri, 10 Nov 2023 08:17:22 GMT, Per Minborg wrote: >> This PR outlines a solution for making immutable maps `@ValueBased` by >> removing cacheing of certain values in `AbstractMap`. >> >> By removing these caching fields in `AbstractMap`, we can make the immutable >> maps `@ValueBased` and at the same time, performance is likely improved >> because the JVM is probably able to optimize away object creation anyway via >> escape analysis. Also, all maps will occupy less space as we get rid of a >> number of objects and references stored for each map. >> >> We need to benchmark this solution to better understand its implications. > > Per Minborg has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 14 commits: > > - Merge branch 'master' into vb-map2 > - Fix formatting > - Remove caching in TreeMap > - Remove caching from CHM and CSLM > - Move back clone to original position > - Reintroduce AbstractMap::clone > - Add 'fresh' to implSpec > - Remove AbstractMap::clone > - Merge master > - Merge branch 'master' into vb-map2 > - ... and 4 more: https://git.openjdk.org/jdk/compare/9cce9fe0...b1bfcd17 Keep alive - PR Comment: https://git.openjdk.org/jdk/pull/15614#issuecomment-2096767172
Re: RFR: 8331346: Update PreviewFeature of STREAM_GATHERERS to JEP-473
On Mon, 29 Apr 2024 16:42:05 GMT, Viktor Klang wrote: > This PR finalizes JEP 473 by modifying the PreviewFeature JEP number and > status for Stream Gatherers. LGTM - Marked as reviewed by pminborg (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19003#pullrequestreview-2030731354
Re: RFR: 8331264: Reduce java.lang.constant initialization overhead [v3]
On Mon, 29 Apr 2024 12:39:17 GMT, Claes Redestad wrote:
>> I'm looking at ways at reducing/eliminating startup overheads the classfile
>> API in preparation of #17108, and have pushed a series of enhancements to
>> that effect already. This PR is a collection of minor improvements which add
>> up to a 1.5% reduction in retired instructions - or a 5% reduction in
>> executed bytecode - on a simple lambda startup test.
>
> Claes Redestad has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Simplified void check
src/java.base/share/classes/java/lang/constant/ConstantDescs.java line 178:
> 176:
> 177: private static final ClassDesc[] INDY_BOOTSTRAP_ARGS = {
> 178: CD_MethodHandles_Lookup,
Can these fields be @Stable?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18991#discussion_r1583168392
Integrated: 8314592: Add shortcut to SymbolLookup::find
On Mon, 25 Mar 2024 14:56:23 GMT, Per Minborg wrote: > While `SymbolLookup` correctly uses an `Optional` return to denote whether a > symbol has been found by the lookup or not (which enables composition of > symbol lookups), many clients end up just calling `Optional::get`, or > `Optional::orElseThrow()` on the result. > > This PR proposes to add a convenience method `SymbolLookup::findOrThrow` that > will do a lookup and, if no symbol can be found, throws an > `IllegalArgumentException` with a relevant exception message. This pull request has now been integrated. Changeset: e923dfe4 Author:Per Minborg URL: https://git.openjdk.org/jdk/commit/e923dfe4c51291099d9b7411e6c9f20be79b9a53 Stats: 151 lines in 22 files changed: 88 ins; 0 del; 63 mod 8314592: Add shortcut to SymbolLookup::find Reviewed-by: jvernee, prr - PR: https://git.openjdk.org/jdk/pull/18474
Re: RFR: 8314592: Add shortcut to SymbolLookup::find [v7]
On Mon, 22 Apr 2024 13:46:59 GMT, Maurizio Cimadamore
wrote:
>> Per Minborg has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 12 additional
>> commits since the last revision:
>>
>> - Simplify tests
>> - Add a test for null arg
>> - Add a test for findOrThrow()
>> - Merge branch 'master' into symbol-lookup-findorthrow
>> - Change exception type
>> - Update src/java.base/share/classes/java/lang/foreign/package-info.java
>>
>>Co-authored-by: Jorn Vernee
>> - Update src/java.base/share/classes/java/lang/foreign/SymbolLookup.java
>>
>>Co-authored-by: Maurizio Cimadamore
>> <54672762+mcimadam...@users.noreply.github.com>
>> - Update src/java.base/share/classes/java/lang/foreign/SymbolLookup.java
>>
>>Co-authored-by: Maurizio Cimadamore
>> <54672762+mcimadam...@users.noreply.github.com>
>> - Fix typo
>> - Update after PR comments
>> - ... and 2 more: https://git.openjdk.org/jdk/compare/099a64e8...0e06e0d6
>
> test/jdk/java/foreign/loaderLookup/TestSymbolLookupFindOrThrow.java line 41:
>
>> 39:
>> 40: static {
>> 41: System.loadLibrary("Foo");
>
> Where is this lib defined?
it is defined in the sub-folder `lookup` in the file `libFoo.c`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18474#discussion_r1577755855
Re: RFR: 8314592: Add shortcut to SymbolLookup::find [v8]
> While `SymbolLookup` correctly uses an `Optional` return to denote whether a > symbol has been found by the lookup or not (which enables composition of > symbol lookups), many clients end up just calling `Optional::get`, or > `Optional::orElseThrow()` on the result. > > This PR proposes to add a convenience method `SymbolLookup::findOrThrow` that > will do a lookup and, if no symbol can be found, throws an > `IllegalArgumentException` with a relevant exception message. Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Remove redundant test - Changes: - all: https://git.openjdk.org/jdk/pull/18474/files - new: https://git.openjdk.org/jdk/pull/18474/files/0e06e0d6..31d92589 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18474=07 - incr: https://webrevs.openjdk.org/?repo=jdk=18474=06-07 Stats: 7 lines in 1 file changed: 0 ins; 7 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18474.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18474/head:pull/18474 PR: https://git.openjdk.org/jdk/pull/18474
Re: RFR: 8327858: Improve spliterator and forEach for single-element immutable collections [v2]
On Thu, 21 Mar 2024 18:01:38 GMT, Chen Liang wrote: >> Please review this patch that: >> 1. Implemented `forEach` to optimize for 1 or 2 element collections. >> 2. Implemented `spliterator` to optimize for a single element. >> >> The default implementations for multiple-element immutable collections are >> fine as-is, specializing implementation doesn't provide much benefit. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 13 commits: > > - Use the improved form in forEach > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Null checks should probably be in the beginning... > - mark implicit null checks > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Copyright year, revert changes for non-few element collections > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Merge branch 'feature/imm-coll-stream' of > https://github.com/liachmodded/jdk into feature/imm-coll-stream > - Spliterator for 12, iterate/forEach benchmark > - fix comments > - ... and 3 more: https://git.openjdk.org/jdk/compare/d5b95a0e...69bd0e9c Do we need additional tests or are these modifications already covered by the existing tests? - PR Comment: https://git.openjdk.org/jdk/pull/15834#issuecomment-2072289455
Re: RFR: 8314592: Add shortcut to SymbolLookup::find [v7]
> While `SymbolLookup` correctly uses an `Optional` return to denote whether a > symbol has been found by the lookup or not (which enables composition of > symbol lookups), many clients end up just calling `Optional::get`, or > `Optional::orElseThrow()` on the result. > > This PR proposes to add a convenience method `SymbolLookup::findOrThrow` that > will do a lookup and, if no symbol can be found, throws an > `IllegalArgumentException` with a relevant exception message. Per Minborg has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision: - Simplify tests - Add a test for null arg - Add a test for findOrThrow() - Merge branch 'master' into symbol-lookup-findorthrow - Change exception type - Update src/java.base/share/classes/java/lang/foreign/package-info.java Co-authored-by: Jorn Vernee - Update src/java.base/share/classes/java/lang/foreign/SymbolLookup.java Co-authored-by: Maurizio Cimadamore <54672762+mcimadam...@users.noreply.github.com> - Update src/java.base/share/classes/java/lang/foreign/SymbolLookup.java Co-authored-by: Maurizio Cimadamore <54672762+mcimadam...@users.noreply.github.com> - Fix typo - Update after PR comments - ... and 2 more: https://git.openjdk.org/jdk/compare/76cd531f...0e06e0d6 - Changes: - all: https://git.openjdk.org/jdk/pull/18474/files - new: https://git.openjdk.org/jdk/pull/18474/files/e2f6c4c9..0e06e0d6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18474=06 - incr: https://webrevs.openjdk.org/?repo=jdk=18474=05-06 Stats: 91042 lines in 1455 files changed: 42444 ins; 38886 del; 9712 mod Patch: https://git.openjdk.org/jdk/pull/18474.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18474/head:pull/18474 PR: https://git.openjdk.org/jdk/pull/18474
Re: RFR: 8329760: Add indexOf(Predicate filter) to java..util.List interface [v2]
On Thu, 18 Apr 2024 22:21:11 GMT, Evemose wrote:
>> **Subject**
>> Addition of Predicate-based `indexOf` and `lastIndexOf` methods to
>> `java.util.List`
>>
>> **Motivation**
>> The motivation behind this proposal is to enhance the functionality of the
>> `List` interface by providing a more flexible way to find the index of an
>> element. Currently, the `indexOf` and `lastIndexOf` methods only accept an
>> object as a parameter. This limits the flexibility of these methods as they
>> can only find the index of exact object matches.
>>
>> The proposed methods would accept a `Predicate` as a parameter, allowing
>> users to define a condition that the desired element must meet. This would
>> provide a more flexible and powerful way to find the index of an element in
>> a list.
>>
>> Here is a brief overview of the changes made in this pull request:
>>
>> 1. Added the `indexOf(Predicate filter)` method to the `List`
>> interface.
>> 2. Added the `lastIndexOf(Predicate filter)` method to the `List`
>> interface.
>> 3. Implemented these methods in all non-abstract classes that implement the
>> `List` interface.
>>
>> The changes have been thoroughly tested to ensure they work as expected and
>> do not introduce any regressions. The test cases cover a variety of
>> scenarios to ensure the robustness of the implementation.
>>
>> For example, consider the following test case:
>>
>> List list = new ArrayList<>();
>> list.add("Object one");
>> list.add("NotObject two");
>> list.add("NotObject three");
>>
>> int index1 = list.indexOf(s -> s.contains("ct t"));
>> System.out.println(index1); // Expected output: 1
>> int index2 = list.lastIndexOf(s -> s.startsWith("NotObject"));
>> System.out.println(index2); // Expected output: 2
>>
>>
>> Currently, to achieve the same result, we would have to use a more verbose
>> approach:
>>
>> int index1 = IntStream.range(0, list.size())
>> .filter(i -> list.get(i).contains("ct t"))
>> .findFirst()
>> .orElse(-1);
>> System.out.println(index1); // Output: 1
>> int index2 = IntStream.range(0, list.size())
>> .filter(i -> list.get(i).startsWith("NotObject"))
>> .reduce((first, second) -> second)
>> .orElse(-1);
>> System.out.println(index2); // Output: 2
>>
>>
>> I believe these additions would greatly enhance the functionality and
>> flexibility of the `List` interface, making it more powerful and
>> user-friendly. I look forward to your feedback and am open to making any
>> necessary changes bas...
>
> Evemose has updated the pull request incrementally with one additional commit
> since the last revision:
>
> empty commit to trigger check rerun
Adding (default) methods to `List` would have significant compatibility
ramifications. Also, the PR needs to contain significant testing of the new
proposed methods.
-
PR Comment: https://git.openjdk.org/jdk/pull/18639#issuecomment-2066206801
Re: RFR: 8314592: Add shortcut to SymbolLookup::find [v6]
> While `SymbolLookup` correctly uses an `Optional` return to denote whether a > symbol has been found by the lookup or not (which enables composition of > symbol lookups), many clients end up just calling `Optional::get`, or > `Optional::orElseThrow()` on the result. > > This PR proposes to add a convenience method `SymbolLookup::findOrThrow` that > will do a lookup and, if no symbol can be found, throws an > `IllegalArgumentException` with a relevant exception message. Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Change exception type - Changes: - all: https://git.openjdk.org/jdk/pull/18474/files - new: https://git.openjdk.org/jdk/pull/18474/files/2ebac9fc..e2f6c4c9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18474=05 - incr: https://webrevs.openjdk.org/?repo=jdk=18474=04-05 Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18474.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18474/head:pull/18474 PR: https://git.openjdk.org/jdk/pull/18474
Integrated: 8329997: Add provisions for checking memory segment alignment constraints
On Mon, 15 Apr 2024 07:50:24 GMT, Per Minborg wrote: > This PR proposes to add a new method `MemorySegment::maxByteAlignment` that > returns the maximum byte alignment of a segment (both heap and native > segments). > > Clients can then use this method to determine if a segment is properly > aligned for any given layout (e.g. following a `MemorySegment::reinterpret` > operation). This pull request has now been integrated. Changeset: b648ed0a Author:Per Minborg URL: https://git.openjdk.org/jdk/commit/b648ed0a08b5ed47c1a7d7cbca89d8f389b17013 Stats: 135 lines in 4 files changed: 129 ins; 2 del; 4 mod 8329997: Add provisions for checking memory segment alignment constraints Reviewed-by: jvernee, mcimadamore - PR: https://git.openjdk.org/jdk/pull/18779
