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

Reviewed-by: alanb

Author: Andrew Haley

src/java.base/share/classes/java/lang/ScopedValue.java

@@ -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");
         }
@@ -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;
     }
 
     /**

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.*;
 
 /**
@@ -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.
@@ -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 {
@@ -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);
+    }
+
+
 }