From 9c9670bbfd6cdb3679689627894b9659646f06a7 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 9 Jan 2025 16:44:25 +0100 Subject: [PATCH] Optimize CachedSupplier a little (#119563) This thing shows up a lot in profiling many-shards searches and fields caps. This is mainly due to expensive `initResult` but we can at least get a little better cache behavior here on some architectures by saving one volatile read. --- .../common/util/CachedSupplier.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/util/CachedSupplier.java b/server/src/main/java/org/elasticsearch/common/util/CachedSupplier.java index e5ab006bd3c14..f394d9d8367cb 100644 --- a/server/src/main/java/org/elasticsearch/common/util/CachedSupplier.java +++ b/server/src/main/java/org/elasticsearch/common/util/CachedSupplier.java @@ -19,7 +19,9 @@ public final class CachedSupplier implements Supplier { private volatile Supplier supplier; - private volatile T result; + // result does not need to be volatile as we only read it after reading that the supplier got nulled out. Since we null out the + // supplier after setting the result, total store order from an observed volatile write is sufficient to make a plain read safe. + private T result; public static CachedSupplier wrap(Supplier supplier) { if (supplier instanceof CachedSupplier c) { @@ -38,14 +40,18 @@ public T get() { if (supplier == null) { return result; } - initResult(); - return result; + return initResult(); } - private synchronized void initResult() { - if (supplier != null) { - result = supplier.get(); + private synchronized T initResult() { + var s = supplier; + if (s != null) { + T res = s.get(); + result = res; supplier = null; + return res; + } else { + return result; } }