Skip to content

8361497: Scoped Values: orElse and orElseThrow do not access the cache #26164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 16 additions & 13 deletions src/java.base/share/classes/java/lang/ScopedValue.java
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ public T get() {

@SuppressWarnings("unchecked")
private T slowGet() {
var value = findBinding();
Object value = scopedValueBindings().find(this);
if (value == Snapshot.NIL) {
throw new NoSuchElementException("ScopedValue not bound");
}
Expand All @@ -581,32 +581,35 @@ private T slowGet() {
}

/**
* {@return {@code true} if this scoped value is bound in the current thread}
* Return the value of the scoped value or NIL if not bound.
* Consult the cache, and only if the value is not found there
* search the list of bindings. Update the cache if the binding
* was found.
*/
public boolean isBound() {
private Object findBinding() {
Object[] objects = scopedValueCache();
if (objects != null) {
int n = (hash & Cache.Constants.SLOT_MASK) * 2;
if (objects[n] == this) {
return true;
return objects[n + 1];
}
n = ((hash >>> Cache.INDEX_BITS) & Cache.Constants.SLOT_MASK) * 2;
if (objects[n] == this) {
return true;
return objects[n + 1];
}
}
var value = findBinding();
boolean result = (value != Snapshot.NIL);
if (result) Cache.put(this, value);
return result;
Object value = scopedValueBindings().find(this);
boolean found = (value != Snapshot.NIL);
if (found) Cache.put(this, value);
return value;
}

/**
* Return the value of the scoped value or NIL if not bound.
* {@return {@code true} if this scoped value is bound in the current thread}
*/
private Object findBinding() {
Object value = scopedValueBindings().find(this);
return value;
public boolean isBound() {
Object obj = findBinding();
return obj != Snapshot.NIL;
}

/**
Expand Down
86 changes: 84 additions & 2 deletions test/micro/org/openjdk/bench/java/lang/ScopedValues.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.openjdk.jmh.annotations.*;
import org.openjdk.jmh.infra.Blackhole;

import static java.lang.ScopedValue.where;
import static org.openjdk.bench.java.lang.ScopedValuesData.*;

/**
Expand Down Expand Up @@ -102,6 +103,26 @@ public int thousandMaybeGets(Blackhole bh) throws Exception {
return result;
}

@Benchmark
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public int thousandUnboundOrElses(Blackhole bh) throws Exception {
int result = 0;
for (int i = 0; i < 1_000; i++) {
result += ScopedValuesData.unbound.orElse(1);
}
return result;
}

@Benchmark
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public int thousandBoundOrElses(Blackhole bh) throws Exception {
int result = 0;
for (int i = 0; i < 1_000; i++) {
result += ScopedValuesData.sl1.orElse(1);
}
return result;
}

// Test 2: stress the ScopedValue cache.
// The idea here is to use a bunch of bound values cyclically, which
// stresses the ScopedValue cache.
Expand Down Expand Up @@ -137,12 +158,12 @@ public int sixValues_ThreadLocal() throws Exception {
@Benchmark
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public int CreateBindThenGetThenRemove_ScopedValue() throws Exception {
return ScopedValue.where(sl1, THE_ANSWER).call(sl1::get);
return where(sl1, THE_ANSWER).call(sl1::get);
}


// Create a Carrier ahead of time: might be slightly faster
private static final ScopedValue.Carrier HOLD_42 = ScopedValue.where(sl1, 42);
private static final ScopedValue.Carrier HOLD_42 = where(sl1, 42);
@Benchmark
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public int bindThenGetThenRemove_ScopedValue() throws Exception {
Expand Down Expand Up @@ -230,4 +251,65 @@ public Object newInstance() {
ScopedValue<Integer> val = ScopedValue.newInstance();
return val;
}

// Test 6: Performance with a large number of bindings
static final long deepCall(ScopedValue<Integer> outer, long n) {
long result = 0;
if (n > 0) {
ScopedValue<Long> sv = ScopedValue.newInstance();
return where(sv, n).call(() -> deepCall(outer, n - 1));
} else {
for (int i = 0; i < 1_000_000; i++) {
result += outer.orElse(12);
}
}
return result;
}

@Benchmark
@OutputTimeUnit(TimeUnit.MILLISECONDS)
public long deepBindingTest1() {
return deepCall(ScopedValuesData.unbound, 1000);
}

@Benchmark
@OutputTimeUnit(TimeUnit.MILLISECONDS)
public long deepBindingTest2() {
return deepCall(ScopedValuesData.sl1, 1000);
}


// Test 7: Performance with a large number of bindings
// Different from Test 6 in that we recursively build a very long
// list of Carriers and invoke Carrier.call() only once.
static final long deepCall2(ScopedValue<Integer> outer, ScopedValue.Carrier carrier, long n) {
long result = 0;
if (n > 0) {
ScopedValue<Long> sv = ScopedValue.newInstance();
return deepCall2(outer, carrier.where(sv, n), n - 1);
} else {
result = carrier.call(() -> {
long sum = 0;
for (int i = 0; i < 1_000_000; i++) {
sum += outer.orElse(12);
}
return sum;
});
}
return result;
}

@Benchmark
@OutputTimeUnit(TimeUnit.MILLISECONDS)
public long deepBindingTest3() {
return deepCall2(ScopedValuesData.unbound, where(ScopedValuesData.sl2,0), 1000);
}

@Benchmark
@OutputTimeUnit(TimeUnit.MILLISECONDS)
public long deepBindingTest4() {
return deepCall2(ScopedValuesData.sl1, where(ScopedValuesData.sl2, 0), 1000);
}


}