On Fri, 4 Nov 2022 09:50:10 GMT, Andrew Haley <a...@openjdk.org> wrote:
>> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java
>> line 481:
>>
>>> 479: }
>>> 480: */
>>> 481: return findBinding() != Snapshot.NIL;
>>
>> This should probably call `Cache.put(this, value)` when `findBinding()`
>> isn’t `Snapshot.NIL`, since it’s likely that `isBound()` will most commonly
>> be used in the form of:
>>
>> if (SCOPED_VALUE.isBound()) {
>> final var value = SCOPED_VALUE.get();
>> // do something with `value`
>> }
>>
>>
>> --------------------------------------------------------------------------------
>>
>> Suggestion:
>>
>> var value = findBinding();
>> if (value == Snapshot.NIL) {
>> return false;
>> }
>> Cache.put(this, value);
>> return true;
>
> Probably so, yes. I'll have a look at that along with caching failure.
So I just did the experiment of caching failures and the result of `isBound()`.
This test:
@Benchmark
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public int thousandMaybeGets(Blackhole bh) throws Exception {
int result = 0;
for (int i = 0; i < 1_000; i++) {
if (ScopedValuesData.sl1.isBound()) {
result += ScopedValuesData.sl1.get();
}
}
return result;
}
Before and after:
ScopedValues.thousandMaybeGets avgt 10 13436.112 ± 20.885 ns/op
ScopedValues.thousandMaybeGets avgt 10 56.315 ± 0.583 ns/op
You may have a point. The experiment is on a branch called
`JDK-8286666-cache-queries` in [My personal
repo](https://github.com/theRealAph/jdk).
I'd push it now but it's getting a bit late to make such changes now. WDYT?
-------------
PR: https://git.openjdk.org/jdk/pull/10952
