On Wed, 7 May 2025 09:23:45 GMT, Per Minborg <pminb...@openjdk.org> wrote:
>> This sketch shows how "Stable Updaters" can be used to create stable >> computations of `@Stable` fields. Only one updater is needed per class, >> similar to `AtomicIntegerFieldUpdater`. > > Per Minborg has updated the pull request incrementally with two additional > commits since the last revision: > > - Reformat > - Revert changes in public classes Starting to look nicely idiomatic. If I saw this I would however want to add a thin wrapper specific for hash codes where the "zero replacement" and generic types are more hidden. * HashCoder.forIntField(Foo.class, "hash", ...) * HashCoder.forLongField(...) I like that there's an annotation on the field now though! test/jdk/java/lang/StableValue/StableFieldUpdaterExampleTest.java line 83: > 81: > 82: > 83: static // Intentionally in unblessed order to allow the example to > look nice Huh, not seen this before ... interesting. test/jdk/java/lang/StableValue/StableFieldUpdaterExampleTest.java line 117: > 115: private static final ToIntFunction<LazyFoo> HASH_UPDATER = > 116: StableFieldUpdater.ofInt(LazyFoo.class, "hash", > 117: l -> Objects.hash(l.bar, l.baz), -1); As someone unfamiliar with this, I'm given to ask "What's the -1 for?" I *think* I can intuit that it's a value to be used *if* the hashcode actually ends up calculating zero (so any random bit pattern would (should?) be equally useful? In example code if feels like this is not quite explaining itself enough to teach someone about its usage. For example (if it's what I think) it can't actually be passed as zero. test/jdk/java/lang/StableValue/StableFieldUpdaterExampleTest.java line 149: > 147: private static final ToLongFunction<LazySpecifiedFoo> > HASH_UPDATER = > 148: StableFieldUpdater.ofLong(LazySpecifiedFoo.class, "hash", > 149: l -> (l.bar == null && l.baz == null) ? 0 : > Objects.hash(l.bar, l.baz), 1L << 32); Maybe for an example, use a (private?) static method reference here `calculateHash`. It helps visually distinguish plumbing and business logic IMO. ------------- PR Review: https://git.openjdk.org/jdk/pull/25040#pullrequestreview-2821463165 PR Review Comment: https://git.openjdk.org/jdk/pull/25040#discussion_r2077457809 PR Review Comment: https://git.openjdk.org/jdk/pull/25040#discussion_r2077457604 PR Review Comment: https://git.openjdk.org/jdk/pull/25040#discussion_r2077460209
