From bdc1a67a7239748c303478a1dd95e67fe21adb32 Mon Sep 17 00:00:00 2001 From: Andrew Haley Date: Mon, 7 Jul 2025 14:24:10 +0100 Subject: [PATCH 1/2] 8361497: Scoped Values: orElse and orElseThrow do not access the cache --- .../share/classes/java/lang/ScopedValue.java | 30 +++++++++---------- .../openjdk/bench/java/lang/ScopedValues.java | 20 +++++++++++++ 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/java.base/share/classes/java/lang/ScopedValue.java b/src/java.base/share/classes/java/lang/ScopedValue.java index 206c81d5238ec..590e230e46963 100644 --- a/src/java.base/share/classes/java/lang/ScopedValue.java +++ b/src/java.base/share/classes/java/lang/ScopedValue.java @@ -566,7 +566,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"); } @@ -575,32 +575,32 @@ 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. */ - public boolean isBound() { + private Object orElseNil() { Object[] objects = scopedValueCache(); if (objects != null) { int n = (hash & Cache.SLOT_MASK) * 2; if (objects[n] == this) { - return true; + return objects[n + 1]; } n = ((hash >>> Cache.INDEX_BITS) & Cache.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 = orElseNil(); + return obj != Snapshot.NIL; } /** @@ -612,7 +612,7 @@ private Object findBinding() { */ public T orElse(T other) { Objects.requireNonNull(other); - Object obj = findBinding(); + Object obj = orElseNil(); if (obj != Snapshot.NIL) { @SuppressWarnings("unchecked") T value = (T) obj; @@ -633,7 +633,7 @@ public T orElse(T other) { */ public T orElseThrow(Supplier exceptionSupplier) throws X { Objects.requireNonNull(exceptionSupplier); - Object obj = findBinding(); + Object obj = orElseNil(); if (obj != Snapshot.NIL) { @SuppressWarnings("unchecked") T value = (T) obj; diff --git a/test/micro/org/openjdk/bench/java/lang/ScopedValues.java b/test/micro/org/openjdk/bench/java/lang/ScopedValues.java index 70c97d5755116..4a0787bcb00df 100644 --- a/test/micro/org/openjdk/bench/java/lang/ScopedValues.java +++ b/test/micro/org/openjdk/bench/java/lang/ScopedValues.java @@ -92,6 +92,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. From 44b542e900834fb66ac73d54cf73324cabc16540 Mon Sep 17 00:00:00 2001 From: Andrew Haley Date: Mon, 7 Jul 2025 17:02:59 +0100 Subject: [PATCH 2/2] 8361497: Scoped Values: orElse and orElseThrow do not access the cache --- .../share/classes/java/lang/ScopedValue.java | 11 ++-- .../openjdk/bench/java/lang/ScopedValues.java | 66 ++++++++++++++++++- 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/java.base/share/classes/java/lang/ScopedValue.java b/src/java.base/share/classes/java/lang/ScopedValue.java index a9bd92ac9dfa6..0d5da3e0e2a81 100644 --- a/src/java.base/share/classes/java/lang/ScopedValue.java +++ b/src/java.base/share/classes/java/lang/ScopedValue.java @@ -582,8 +582,11 @@ private T slowGet() { /** * 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. */ - private Object orElseNil() { + private Object findBinding() { Object[] objects = scopedValueCache(); if (objects != null) { int n = (hash & Cache.Constants.SLOT_MASK) * 2; @@ -605,7 +608,7 @@ private Object orElseNil() { * {@return {@code true} if this scoped value is bound in the current thread} */ public boolean isBound() { - Object obj = orElseNil(); + Object obj = findBinding(); return obj != Snapshot.NIL; } @@ -618,7 +621,7 @@ public boolean isBound() { */ public T orElse(T other) { Objects.requireNonNull(other); - Object obj = orElseNil(); + Object obj = findBinding(); if (obj != Snapshot.NIL) { @SuppressWarnings("unchecked") T value = (T) obj; @@ -639,7 +642,7 @@ public T orElse(T other) { */ public T orElseThrow(Supplier exceptionSupplier) throws X { Objects.requireNonNull(exceptionSupplier); - Object obj = orElseNil(); + Object obj = findBinding(); if (obj != Snapshot.NIL) { @SuppressWarnings("unchecked") T value = (T) obj; diff --git a/test/micro/org/openjdk/bench/java/lang/ScopedValues.java b/test/micro/org/openjdk/bench/java/lang/ScopedValues.java index 47601ee9c4156..710cf87e72fa1 100644 --- a/test/micro/org/openjdk/bench/java/lang/ScopedValues.java +++ b/test/micro/org/openjdk/bench/java/lang/ScopedValues.java @@ -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.*; /** @@ -157,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 { @@ -250,4 +251,65 @@ public Object newInstance() { ScopedValue val = ScopedValue.newInstance(); return val; } + + // Test 6: Performance with a large number of bindings + static final long deepCall(ScopedValue outer, long n) { + long result = 0; + if (n > 0) { + ScopedValue 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 outer, ScopedValue.Carrier carrier, long n) { + long result = 0; + if (n > 0) { + ScopedValue 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); + } + + }