From e537e94d39f7b9c192071f6e43f3a28d1cd4b0b3 Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Fri, 21 Dec 2018 19:16:55 +0100 Subject: [PATCH] HotEntryCache will update Instants only once per second Calling Instant.now() several hundred thousand times per second can be expensive. In my measurements >10% of the time spend when loading new data was spend calling Instant.now(). Fixed this by storing an Instant as static member and updating it periodically in a separate thread. --- .../pdb/datastore/internal/DataStore.java | 3 +- .../lucares/utils/cache/HotEntryCache.java | 113 ++++++++++++++---- .../utils/cache/HotEntryCacheTest.java | 19 ++- .../lucares/performance/db/TagsToFile.java | 2 +- 4 files changed, 107 insertions(+), 30 deletions(-) diff --git a/data-store/src/main/java/org/lucares/pdb/datastore/internal/DataStore.java b/data-store/src/main/java/org/lucares/pdb/datastore/internal/DataStore.java index 1e6660f..f29f97e 100644 --- a/data-store/src/main/java/org/lucares/pdb/datastore/internal/DataStore.java +++ b/data-store/src/main/java/org/lucares/pdb/datastore/internal/DataStore.java @@ -152,7 +152,8 @@ public class DataStore implements AutoCloseable { // A Doc will never be changed once it is created. Therefore we can cache them // easily. - private final HotEntryCache docIdToDocCache = new HotEntryCache<>(Duration.ofMinutes(10)); + private final HotEntryCache docIdToDocCache = new HotEntryCache<>(Duration.ofMinutes(10), + "docIdToDocCache"); private final DiskStorage diskStorage; private final Path diskStorageFilePath; diff --git a/pdb-utils/src/main/java/org/lucares/utils/cache/HotEntryCache.java b/pdb-utils/src/main/java/org/lucares/utils/cache/HotEntryCache.java index bfed7c5..9e0cdd3 100644 --- a/pdb-utils/src/main/java/org/lucares/utils/cache/HotEntryCache.java +++ b/pdb-utils/src/main/java/org/lucares/utils/cache/HotEntryCache.java @@ -4,8 +4,10 @@ import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.Arrays; +import java.util.ConcurrentModificationException; import java.util.EnumSet; import java.util.Set; +import java.util.UUID; import java.util.WeakHashMap; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; @@ -97,9 +99,9 @@ public class HotEntryCache { private V value; - public Entry(final V value, final Clock clock) { + public Entry(final V value, final Instant creationTime) { this.value = value; - lastAccessed = Instant.now(clock); + lastAccessed = creationTime; } public V getValue() { @@ -119,6 +121,39 @@ public class HotEntryCache { } } + private static final class TimeUpdaterThread extends Thread { + + private final WeakHashMap, Void> weakCaches = new WeakHashMap<>(); + + public TimeUpdaterThread() { + setDaemon(true); + setName("HotEntryCache-time"); + } + + public void addCache(final HotEntryCache cache) { + weakCaches.put(cache, null); + } + + @Override + public void run() { + while (true) { + try { + TimeUnit.SECONDS.sleep(1); + } catch (final InterruptedException e) { + // interrupted: update the 'now' instants of all caches + } + try { + for (final HotEntryCache cache : weakCaches.keySet()) { + cache.updateTime(); + } + } catch (final ConcurrentModificationException e) { + // ignore: might happen if an entry in weakCaches is garbage collected + // while we are iterating + } + } + } + } + private static final class EvictionThread extends Thread { private static final Duration MAX_SLEEP_PERIOD = Duration.ofDays(1); @@ -144,13 +179,18 @@ public class HotEntryCache { final CompletableFuture future = this.future.getAcquire(); - final Instant minNextEvictionTime = evictStaleEntries(); + try { + final Instant minNextEvictionTime = evictStaleEntries(); - timeToNextEviction = normalizeDurationToNextEviction(minNextEvictionTime); + timeToNextEviction = normalizeDurationToNextEviction(minNextEvictionTime); - if (future != null) { - future.complete(null); - this.future.set(null); + if (future != null) { + future.complete(null); + this.future.set(null); + } + } catch (final ConcurrentModificationException e) { + // ignore: might happen if an entry in weakCaches is garbage collected + // while we are iterating } } } @@ -208,10 +248,15 @@ public class HotEntryCache { private static final EvictionThread EVICTER = new EvictionThread(); + private static final TimeUpdaterThread TIME_UPDATER = new TimeUpdaterThread(); + static { EVICTER.start(); + TIME_UPDATER.start(); } + private static Instant now; + /** * Mapping of the key to the value. *

@@ -225,32 +270,44 @@ public class HotEntryCache { private Clock clock; - HotEntryCache(final Duration timeToLive, final Clock clock) { + private final String name; + + HotEntryCache(final Duration timeToLive, final Clock clock, final String name) { this.timeToLive = timeToLive; this.clock = clock; + this.name = name; + now = Instant.now(clock); + EVICTER.addCache(this); + TIME_UPDATER.addCache(this); + } + + HotEntryCache(final Duration timeToLive, final Clock clock) { + this(timeToLive, clock, UUID.randomUUID().toString()); + } + + public HotEntryCache(final Duration timeToLive, final String name) { + this(timeToLive, Clock.systemDefaultZone(), name); } public HotEntryCache(final Duration timeToLive) { - this(timeToLive, Clock.systemDefaultZone()); + this(timeToLive, Clock.systemDefaultZone(), UUID.randomUUID().toString()); } public int size() { return cache.size(); } + public String getName() { + return name; + } + public void addListener(final EventListener listener, final EventType... eventTypes) { listeners.add(new EventSubscribers<>(EnumSet.copyOf(Arrays.asList(eventTypes)), listener)); } public V get(final K key) { final Entry entry = cache.computeIfPresent(key, (k, e) -> { - final Instant now = Instant.now(clock); - if (isExpired(e, now)) { - handleEvent(EventType.EVICTED, k, e.getValue()); - return null; - } - touch(key, e); return e; }); @@ -270,7 +327,8 @@ public class HotEntryCache { oldEntry.setValue(value); entry = oldEntry; } else { - entry = new Entry<>(value, clock); + final Instant creationTime = now(); + entry = new Entry<>(value, creationTime); } touch(k, entry); return entry; @@ -302,7 +360,8 @@ public class HotEntryCache { final boolean wasEmptyBefore = cache.isEmpty(); final Entry entry = cache.computeIfAbsent(key, (k) -> { final V value = mappingFunction.apply(k); - final Entry e = new Entry<>(value, clock); + final Instant creationTime = now(); + final Entry e = new Entry<>(value, creationTime); touch(key, e); return e; }); @@ -341,10 +400,10 @@ public class HotEntryCache { } private Instant evict() { - final Instant now = Instant.now(clock); + final Instant now = now(); final Instant oldestValuesToKeep = now.minus(timeToLive); Instant lastAccessTime = Instant.MAX; - LOGGER.trace("cache size before eviction {}", cache.size()); + LOGGER.trace("{}: cache size before eviction {}", name, cache.size()); // for (final java.util.Map.Entry> mapEntry : // lastAccessMap.entrySet()) { @@ -368,7 +427,7 @@ public class HotEntryCache { }); } - LOGGER.trace("cache size after eviction {}", cache.size()); + LOGGER.trace("{}: cache size after eviction {}", name, cache.size()); final Instant nextEvictionTime = lastAccessTime.equals(Instant.MAX) ? Instant.MAX : lastAccessTime.plus(timeToLive); @@ -383,11 +442,18 @@ public class HotEntryCache { return a.compareTo(b) < 0 ? a : b; } + private Instant now() { + return now; + } + + // visible for test + void updateTime() { + now = Instant.now(clock); + } + private void touch(final K key, final Entry entry) { if (entry != null) { - - final Instant now = Instant.now(clock); - + final Instant now = now(); entry.touch(now); } @@ -408,6 +474,7 @@ public class HotEntryCache { // visible for test void triggerEvictionAndWait() { + updateTime(); final Future future = EVICTER.nextEvictionChangedWithFuture(); try { future.get(5, TimeUnit.MINUTES); diff --git a/pdb-utils/src/test/java/org/lucares/utils/cache/HotEntryCacheTest.java b/pdb-utils/src/test/java/org/lucares/utils/cache/HotEntryCacheTest.java index b211f74..d947316 100644 --- a/pdb-utils/src/test/java/org/lucares/utils/cache/HotEntryCacheTest.java +++ b/pdb-utils/src/test/java/org/lucares/utils/cache/HotEntryCacheTest.java @@ -45,6 +45,7 @@ public class HotEntryCacheTest { cache.put("key", "value1"); clock.plusSeconds(2); + cache.updateTime(); cache.put("key", "value2"); @@ -64,19 +65,23 @@ public class HotEntryCacheTest { Assert.assertEquals(cachedValue1_evicted, null); } - // TODO that does not make sense. Get should not evict. We should be happy that - // the element is still in the map when we need it. - public void testGetEvicts() throws Exception { + public void testGetTouches() throws Exception { final ModifiableFixedTimeClock clock = new ModifiableFixedTimeClock(); final Duration timeToLive = Duration.ofSeconds(10); final HotEntryCache cache = new HotEntryCache<>(timeToLive, clock); cache.put("key", "value1"); + // skip forward in time, but do not yet trigger eviction clock.plus(timeToLive.plusMillis(1)); + cache.updateTime(); - final String cachedValue1_evicted = cache.get("key"); - Assert.assertEquals(cachedValue1_evicted, null); + cache.get("key"); // will touch the entry + + cache.triggerEvictionAndWait(); // if get didn't touch, then this will evict the entry + + final String cachedValue1 = cache.get("key"); + Assert.assertEquals(cachedValue1, "value1"); } public void testEvictionByBackgroundThread() throws InterruptedException, ExecutionException, TimeoutException { @@ -92,6 +97,7 @@ public class HotEntryCacheTest { cache.put("key", "value1"); clock.plus(timeToLive.minusSeconds(1)); + cache.updateTime(); cache.put("key2", "value2"); clock.plus(Duration.ofSeconds(1).plusMillis(1)); @@ -149,6 +155,7 @@ public class HotEntryCacheTest { // seek, so that it is almost evicted clock.plus(timeToLive.minusMillis(1)); + cache.updateTime(); // the for each should touch the entries cache.forEach(s -> { @@ -156,12 +163,14 @@ public class HotEntryCacheTest { // seek again clock.plus(timeToLive.minusMillis(1)); + cache.triggerEvictionAndWait(); // if the touch didn't happen, then the value is now evicted Assert.assertEquals(evictionEventFuture.isDone(), false); // seek again, so that the entry will get evicted clock.plus(timeToLive.minusMillis(1)); + cache.triggerEvictionAndWait(); Assert.assertEquals(cache.get("key"), null); } diff --git a/performanceDb/src/main/java/org/lucares/performance/db/TagsToFile.java b/performanceDb/src/main/java/org/lucares/performance/db/TagsToFile.java index e67a3ec..82fdc54 100644 --- a/performanceDb/src/main/java/org/lucares/performance/db/TagsToFile.java +++ b/performanceDb/src/main/java/org/lucares/performance/db/TagsToFile.java @@ -71,7 +71,7 @@ public class TagsToFile implements AutoCloseable { public TagsToFile(final DataStore dataStore) { this.dataStore = dataStore; - writerCache = new HotEntryCache<>(Duration.ofSeconds(10)); + writerCache = new HotEntryCache<>(Duration.ofSeconds(10), "writerCache"); writerCache.addListener(new RemovalListener(), EventType.EVICTED, EventType.REMOVED); }