From 491dfbc15ecd89e9b88caa906a5baad69da58c83 Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Thu, 4 Nov 2021 08:35:03 +0000 Subject: [PATCH] Cache access tweaks (#750) * don't create new cache key when there are no filters, just use the path * fix non volatile double checked locking cache initialisation, replace with variant of holder pattern --- .../jayway/jsonpath/internal/JsonContext.java | 6 +-- .../jsonpath/spi/cache/CacheProvider.java | 46 +++++++++++++------ 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/json-path/src/main/java/com/jayway/jsonpath/internal/JsonContext.java b/json-path/src/main/java/com/jayway/jsonpath/internal/JsonContext.java index 9d8d8ee4..76985f78 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/internal/JsonContext.java +++ b/json-path/src/main/java/com/jayway/jsonpath/internal/JsonContext.java @@ -29,13 +29,12 @@ import com.jayway.jsonpath.spi.cache.CacheProvider; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.LinkedList; +import java.util.Arrays; import java.util.List; import static com.jayway.jsonpath.JsonPath.compile; import static com.jayway.jsonpath.internal.Utils.notEmpty; import static com.jayway.jsonpath.internal.Utils.notNull; -import static java.util.Arrays.asList; public class JsonContext implements DocumentContext { @@ -216,7 +215,8 @@ public class JsonContext implements DocumentContext { private JsonPath pathFromCache(String path, Predicate[] filters) { Cache cache = CacheProvider.getCache(); - String cacheKey = Utils.concat(path, new LinkedList(asList(filters)).toString()); + String cacheKey = filters == null || filters.length == 0 + ? path : Utils.concat(path, Arrays.toString(filters)); JsonPath jsonPath = cache.get(cacheKey); if (jsonPath == null) { jsonPath = compile(path, filters); diff --git a/json-path/src/main/java/com/jayway/jsonpath/spi/cache/CacheProvider.java b/json-path/src/main/java/com/jayway/jsonpath/spi/cache/CacheProvider.java index fe5ced53..f90cb94a 100644 --- a/json-path/src/main/java/com/jayway/jsonpath/spi/cache/CacheProvider.java +++ b/json-path/src/main/java/com/jayway/jsonpath/spi/cache/CacheProvider.java @@ -2,31 +2,47 @@ package com.jayway.jsonpath.spi.cache; import com.jayway.jsonpath.JsonPathException; +import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; + import static com.jayway.jsonpath.internal.Utils.notNull; public class CacheProvider { - private static Cache cache; + + private static final AtomicReferenceFieldUpdater UPDATER = + AtomicReferenceFieldUpdater.newUpdater(CacheProvider.class, Cache.class, "cache"); + private static final CacheProvider instance = new CacheProvider(); + + private volatile Cache cache; + + private static class CacheHolder { + static final Cache CACHE; + static { + Cache cache = CacheProvider.instance.cache; + // the application is trying to use the cache + // and if no external implementation has been registered, + // we need to initialise it to the default LRUCache + if (cache == null) { + cache = getDefaultCache(); + // on the off chance that the cache implementation was registered during + // initialisation of the holder, this should be respected, so if the + // default cache can't be written back, just read the user supplied value again + if (!UPDATER.compareAndSet(instance, null, cache)) { + cache = CacheProvider.instance.cache; + } + } + CACHE = cache; + } + } public static void setCache(Cache cache){ notNull(cache, "Cache may not be null"); - synchronized (CacheProvider.class){ - if(CacheProvider.cache != null){ - throw new JsonPathException("Cache provider must be configured before cache is accessed."); - } else { - CacheProvider.cache = cache; - } + if (!UPDATER.compareAndSet(instance, null, cache)) { + throw new JsonPathException("Cache provider must be configured before cache is accessed and must not be registered twice."); } } public static Cache getCache() { - if(CacheProvider.cache == null){ - synchronized (CacheProvider.class){ - if(CacheProvider.cache == null){ - CacheProvider.cache = getDefaultCache(); - } - } - } - return CacheProvider.cache; + return CacheHolder.CACHE; }