From b01def59337e46e442dee58c1b2dd1ab4465404c Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Mon, 14 Feb 2022 22:14:55 +1100 Subject: [PATCH 1/2] This introduces a special exception that can be used to short circuit the value cache lookups --- .../java/org/dataloader/DataLoaderHelper.java | 40 ++++++++++++++----- src/main/java/org/dataloader/ValueCache.java | 24 +++++++++-- .../org/dataloader/impl/NoOpValueCache.java | 30 +++++++++++--- 3 files changed, 76 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/dataloader/DataLoaderHelper.java b/src/main/java/org/dataloader/DataLoaderHelper.java index 41f6801..ae22e04 100644 --- a/src/main/java/org/dataloader/DataLoaderHelper.java +++ b/src/main/java/org/dataloader/DataLoaderHelper.java @@ -331,22 +331,33 @@ CompletableFuture> invokeLoader(List keys, List keyContexts, CompletableFuture>> cacheCallCF = getFromValueCache(keys); return cacheCallCF.thenCompose(cachedValues -> { - assertState(keys.size() == cachedValues.size(), () -> "The size of the cached values MUST be the same size as the key list"); - // the following is NOT a Map because keys in data loader can repeat (by design) // and hence "a","b","c","b" is a valid set of keys List> valuesInKeyOrder = new ArrayList<>(); List missedKeyIndexes = new ArrayList<>(); List missedKeys = new ArrayList<>(); List missedKeyContexts = new ArrayList<>(); - for (int i = 0; i < keys.size(); i++) { - Try cacheGet = cachedValues.get(i); - valuesInKeyOrder.add(cacheGet); - if (cacheGet.isFailure()) { + + // if they return a ValueCachingNotSupported exception then we insert this special marker value, and it + // means it's a total miss, we need to get all these keys via the batch loader + if (cachedValues == NOT_SUPPORTED_LIST) { + for (int i = 0; i < keys.size(); i++) { + valuesInKeyOrder.add(ALWAYS_FAILED); missedKeyIndexes.add(i); missedKeys.add(keys.get(i)); missedKeyContexts.add(keyContexts.get(i)); } + } else { + assertState(keys.size() == cachedValues.size(), () -> "The size of the cached values MUST be the same size as the key list"); + for (int i = 0; i < keys.size(); i++) { + Try cacheGet = cachedValues.get(i); + valuesInKeyOrder.add(cacheGet); + if (cacheGet.isFailure()) { + missedKeyIndexes.add(i); + missedKeys.add(keys.get(i)); + missedKeyContexts.add(keyContexts.get(i)); + } + } } if (missedKeys.isEmpty()) { // @@ -442,9 +453,16 @@ int dispatchDepth() { } } + private final List> NOT_SUPPORTED_LIST = emptyList(); + private final CompletableFuture>> NOT_SUPPORTED = CompletableFuture.completedFuture(NOT_SUPPORTED_LIST); + private final Try ALWAYS_FAILED = Try.alwaysFailed(); + private CompletableFuture>> getFromValueCache(List keys) { try { return nonNull(valueCache.getValues(keys), () -> "Your ValueCache.getValues function MUST return a non null CompletableFuture"); + } catch (ValueCache.ValueCachingNotSupported ignored) { + // use of a final field prevents CF object allocation for this special purpose + return NOT_SUPPORTED; } catch (RuntimeException e) { return CompletableFutureKit.failedFuture(e); } @@ -456,16 +474,18 @@ private CompletableFuture> setToValueCache(List assembledValues, List if (completeValueAfterCacheSet) { return nonNull(valueCache .setValues(missedKeys, missedValues), () -> "Your ValueCache.setValues function MUST return a non null CompletableFuture") - // we dont trust the set cache to give us the values back - we have them - lets use them - // if the cache set fails - then they wont be in cache and maybe next time they will + // we don't trust the set cache to give us the values back - we have them - lets use them + // if the cache set fails - then they won't be in cache and maybe next time they will .handle((ignored, setExIgnored) -> assembledValues); } else { // no one is waiting for the set to happen here so if its truly async - // it will happen eventually but no result will be dependant on it + // it will happen eventually but no result will be dependent on it valueCache.setValues(missedKeys, missedValues); } + } catch (ValueCache.ValueCachingNotSupported ignored) { + // ok no set caching is fine if they say so } catch (RuntimeException ignored) { - // if we cant set values back into the cache - so be it - this must be a faulty + // if we can't set values back into the cache - so be it - this must be a faulty // ValueCache implementation } return CompletableFuture.completedFuture(assembledValues); diff --git a/src/main/java/org/dataloader/ValueCache.java b/src/main/java/org/dataloader/ValueCache.java index 44071bc..1d01b77 100644 --- a/src/main/java/org/dataloader/ValueCache.java +++ b/src/main/java/org/dataloader/ValueCache.java @@ -74,13 +74,18 @@ static ValueCache defaultValueCache() { * a successful Try contain the cached value is returned. *

* You MUST return a List that is the same size as the keys passed in. The code will assert if you do not. + *

+ * If your cache does not have anything in it at all, and you want to quickly short-circuit this method and avoid any object allocation + * then throw {@link ValueCachingNotSupported} and the code will know there is nothing in cache at this time. * * @param keys the list of keys to get cached values for. * * @return a future containing a list of {@link Try} cached values for each key passed in. + * + * @throws ValueCachingNotSupported if this cache wants to short-circuit this method completely */ - default CompletableFuture>> getValues(List keys) { - List>> cacheLookups = new ArrayList<>(); + default CompletableFuture>> getValues(List keys) throws ValueCachingNotSupported { + List>> cacheLookups = new ArrayList<>(keys.size()); for (K key : keys) { CompletableFuture> cacheTry = Try.tryFuture(get(key)); cacheLookups.add(cacheTry); @@ -106,8 +111,10 @@ default CompletableFuture>> getValues(List keys) { * @param values the values to store * * @return a future containing the stored values for fluent composition + * + * @throws ValueCachingNotSupported if this cache wants to short-circuit this method completely */ - default CompletableFuture> setValues(List keys, List values) { + default CompletableFuture> setValues(List keys, List values) throws ValueCachingNotSupported { List> cacheSets = new ArrayList<>(); for (int i = 0; i < keys.size(); i++) { K k = keys.get(i); @@ -140,4 +147,15 @@ default CompletableFuture> setValues(List keys, List values) { * @return a void future for error handling and fluent composition */ CompletableFuture clear(); + + + /** + * This special exception can be used to short-circuit a caching method + */ + class ValueCachingNotSupported extends UnsupportedOperationException { + @Override + public synchronized Throwable fillInStackTrace() { + return this; + } + } } \ No newline at end of file diff --git a/src/main/java/org/dataloader/impl/NoOpValueCache.java b/src/main/java/org/dataloader/impl/NoOpValueCache.java index bd82f03..f5146d1 100644 --- a/src/main/java/org/dataloader/impl/NoOpValueCache.java +++ b/src/main/java/org/dataloader/impl/NoOpValueCache.java @@ -1,9 +1,11 @@ package org.dataloader.impl; +import org.dataloader.Try; import org.dataloader.ValueCache; import org.dataloader.annotations.Internal; +import java.util.List; import java.util.concurrent.CompletableFuture; /** @@ -20,14 +22,27 @@ @Internal public class NoOpValueCache implements ValueCache { - public static NoOpValueCache NOOP = new NoOpValueCache<>(); + /** + * a no op value cache instance + */ + public static final NoOpValueCache NOOP = new NoOpValueCache<>(); + + // avoid object allocation by using a final field + private final ValueCachingNotSupported NOT_SUPPORTED = new ValueCachingNotSupported(); + private final CompletableFuture NOT_SUPPORTED_CF = CompletableFutureKit.failedFuture(NOT_SUPPORTED); + private final CompletableFuture NOT_SUPPORTED_VOID_CF = CompletableFuture.completedFuture(null); /** * {@inheritDoc} */ @Override public CompletableFuture get(K key) { - return CompletableFutureKit.failedFuture(new UnsupportedOperationException()); + return NOT_SUPPORTED_CF; + } + + @Override + public CompletableFuture>> getValues(List keys) throws ValueCachingNotSupported { + throw NOT_SUPPORTED; } /** @@ -35,7 +50,12 @@ public CompletableFuture get(K key) { */ @Override public CompletableFuture set(K key, V value) { - return CompletableFuture.completedFuture(value); + return NOT_SUPPORTED_CF; + } + + @Override + public CompletableFuture> setValues(List keys, List values) throws ValueCachingNotSupported { + throw NOT_SUPPORTED; } /** @@ -43,7 +63,7 @@ public CompletableFuture set(K key, V value) { */ @Override public CompletableFuture delete(K key) { - return CompletableFuture.completedFuture(null); + return NOT_SUPPORTED_VOID_CF; } /** @@ -51,6 +71,6 @@ public CompletableFuture delete(K key) { */ @Override public CompletableFuture clear() { - return CompletableFuture.completedFuture(null); + return NOT_SUPPORTED_VOID_CF; } } \ No newline at end of file From 66f1dbe44f30c577835f2c610555ab94df11da11 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Tue, 15 Feb 2022 08:10:55 +1100 Subject: [PATCH 2/2] Removed synchronised --- src/main/java/org/dataloader/ValueCache.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/dataloader/ValueCache.java b/src/main/java/org/dataloader/ValueCache.java index 1d01b77..551dc5d 100644 --- a/src/main/java/org/dataloader/ValueCache.java +++ b/src/main/java/org/dataloader/ValueCache.java @@ -154,7 +154,7 @@ default CompletableFuture> setValues(List keys, List values) throw */ class ValueCachingNotSupported extends UnsupportedOperationException { @Override - public synchronized Throwable fillInStackTrace() { + public Throwable fillInStackTrace() { return this; } }