Merge pull request #474 from focus-shift/improve-caching

Improve `getHoliday` caching and improve ops/s
focus-shift · Mar 4, 2024 · a563e03 · a563e03
2 parents eb2c4e6 + f6215a9
commit a563e03
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 59 deletions.
diff --git a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManager.java b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManager.java
@@ -1,7 +1,6 @@
 package de.focus_shift.jollyday.core;
 
 import de.focus_shift.jollyday.core.caching.Cache;
-import de.focus_shift.jollyday.core.caching.HolidayManagerValueHandler;
 import de.focus_shift.jollyday.core.configuration.ConfigurationProviderManager;
 import de.focus_shift.jollyday.core.datasource.ConfigurationServiceManager;
 import de.focus_shift.jollyday.core.parser.functions.CalendarToLocalDate;
@@ -48,17 +47,6 @@ public abstract class HolidayManager {
   private static final ConfigurationServiceManager configurationServiceManager =
     new ConfigurationServiceManager(new LazyServiceLoaderCache<>(ConfigurationService.class));
 
-  /**
-   * Caches the instance based country specific holidays so that e.g.
-   * the created HolidayManager via
-   * {@code HolidayManager.getInstance(create("de"))} only contains the german holidays.
-   * <p>
-   * This is also the reason this cache is not static.
-   * If it was static all holidays over all holiday manager instances would be cached,
-   * but only the german holidays are important for the german holiday manager, so only save them.
-   */
-  private final Cache<Set<Holiday>> holidayCache = new Cache<>();
-
   /**
    * The datasource to get the holiday data from.
    */
@@ -123,6 +111,7 @@ public static HolidayManager getInstance(final ManagerParameter parameter) {
   private static HolidayManager createManager(final ManagerParameter parameter) {
     LOG.debug("Creating HolidayManager for calendar '{}'. Caching enabled: {}", parameter, isManagerCachingEnabled());
     CONFIGURATION_MANAGER_PROVIDER.mergeConfigurationProperties(parameter);
+
     final String managerImplClassName = readManagerImplClassName(parameter);
     final HolidayManagerValueHandler holidayManagerValueHandler = new HolidayManagerValueHandler(parameter, managerImplClassName, configurationServiceManager);
     if (isManagerCachingEnabled()) {
@@ -223,27 +212,7 @@ public boolean isHoliday(final LocalDate localDate, final String... args) {
    * @return true if the given date is a holiday in the state/region and below, otherwise false
    */
   public boolean isHoliday(final LocalDate localDate, final HolidayType holidayType, final String... args) {
-
-    final StringBuilder keyBuilder = new StringBuilder();
-    keyBuilder.append(localDate.getYear());
-    for (String arg : args) {
-      keyBuilder.append("_");
-      keyBuilder.append(arg);
-    }
-
-    final Cache.ValueHandler<Set<Holiday>> valueHandler = new Cache.ValueHandler<>() {
-      @Override
-      public String getKey() {
-        return keyBuilder.toString();
-      }
-
-      @Override
-      public Set<Holiday> createValue() {
-        return getHolidays(localDate.getYear(), args);
-      }
-    };
-
-    final Set<Holiday> holidays = holidayCache.get(valueHandler);
+    final Set<Holiday> holidays = getHolidays(localDate.getYear(), args);
     return CalendarUtil.contains(holidays, localDate, holidayType);
   }
 

diff --git a/...e/caching/HolidayManagerValueHandler.java → ...yday/core/HolidayManagerValueHandler.java b/...e/caching/HolidayManagerValueHandler.java → ...yday/core/HolidayManagerValueHandler.java
@@ -1,21 +1,20 @@
-package de.focus_shift.jollyday.core.caching;
+package de.focus_shift.jollyday.core;
 
-import de.focus_shift.jollyday.core.HolidayManager;
-import de.focus_shift.jollyday.core.ManagerParameter;
+import de.focus_shift.jollyday.core.caching.Cache;
 import de.focus_shift.jollyday.core.datasource.ConfigurationServiceManager;
 import de.focus_shift.jollyday.core.spi.ConfigurationService;
 import de.focus_shift.jollyday.core.util.ClassLoadingUtil;
 
 /**
- * Creates the {@link Cache.ValueHandler} which constructs a {@link HolidayManager}.
+ * Creates the {@link Cache.ValueHandler} which constructs and caches a {@link HolidayManager}.
  */
-public class HolidayManagerValueHandler implements Cache.ValueHandler<HolidayManager> {
+class HolidayManagerValueHandler implements Cache.ValueHandler<HolidayManager> {
 
   private final ManagerParameter parameter;
   private final String managerImplClassName;
   private final ConfigurationServiceManager configurationServiceManager;
 
-  public HolidayManagerValueHandler(final ManagerParameter parameter, final String managerImplClassName, final ConfigurationServiceManager configurationServiceManager) {
+  HolidayManagerValueHandler(final ManagerParameter parameter, final String managerImplClassName, final ConfigurationServiceManager configurationServiceManager) {
     this.parameter = parameter;
     this.managerImplClassName = managerImplClassName;
     this.configurationServiceManager = configurationServiceManager;

diff --git a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/impl/DefaultHolidayManager.java b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/impl/DefaultHolidayManager.java
@@ -4,14 +4,13 @@
 import de.focus_shift.jollyday.core.Holiday;
 import de.focus_shift.jollyday.core.HolidayManager;
 import de.focus_shift.jollyday.core.HolidayType;
+import de.focus_shift.jollyday.core.caching.Cache;
 import de.focus_shift.jollyday.core.parser.HolidayParser;
 import de.focus_shift.jollyday.core.spi.Configuration;
 import de.focus_shift.jollyday.core.spi.Holidays;
-import de.focus_shift.jollyday.core.util.ClassLoadingUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
@@ -25,6 +24,7 @@
 import java.util.Objects;
 import java.util.Set;
 
+import static de.focus_shift.jollyday.core.util.ClassLoadingUtil.loadClass;
 import static java.util.Arrays.copyOfRange;
 import static java.util.stream.Collectors.toCollection;
 import static java.util.stream.Collectors.toSet;
@@ -45,9 +45,20 @@ public class DefaultHolidayManager extends HolidayManager {
   private static final String PARSER_IMPL_PREFIX = "parser.impl.";
 
   /**
-   * Parser cache by XML class name.
+   * Caches all {@link HolidayParser} instances actually used by the HolidayManager
    */
-  private final Map<String, HolidayParser> parserCache = new HashMap<>();
+  private final Cache<HolidayParser> parserCache = new Cache<>();
+
+  /**
+   * Caches the instance based country specific holidays so that e.g.
+   * the created HolidayManager via
+   * {@code HolidayManager.getInstance(create("de"))} only contains the german holidays.
+   * <p>
+   * This is also the reason this cache is not static.
+   * If it was static all holidays over all holiday manager instances would be cached,
+   * but only the german holidays are important for the german holiday manager, so only save them.
+   */
+  private final Cache<Set<Holiday>> holidayCache = new Cache<>();
 
   /**
    * Configuration parsed on initialization.
@@ -77,9 +88,29 @@ public void doInit() {
    */
   @Override
   public Set<Holiday> getHolidays(final int year, final String... args) {
-    Set<Holiday> holidaySet = Collections.synchronizedSet(new HashSet<>());
-    getHolidays(year, configuration, holidaySet, args);
-    return holidaySet;
+
+    final StringBuilder keyBuilder = new StringBuilder();
+    keyBuilder.append(year);
+    for (String arg : args) {
+      keyBuilder.append("_");
+      keyBuilder.append(arg);
+    }
+
+    final Cache.ValueHandler<Set<Holiday>> holidayValueHandler = new Cache.ValueHandler<>() {
+      @Override
+      public String getKey() {
+        return keyBuilder.toString();
+      }
+
+      @Override
+      public Set<Holiday> createValue() {
+        final Set<Holiday> holidaySet = Collections.synchronizedSet(new HashSet<>());
+        getHolidays(year, configuration, holidaySet, args);
+        return holidaySet;
+      }
+    };
+
+    return holidayCache.get(holidayValueHandler);
   }
 
   /**
@@ -204,18 +235,31 @@ private Collection<HolidayParser> getParsers(final Holidays config) {
     return parsers;
   }
 
-  private HolidayParser instantiateParser(final String className) throws ClassNotFoundException, InstantiationException, IllegalAccessException, InvocationTargetException, NoSuchMethodException {
-    HolidayParser holidayParser = parserCache.get(className);
-    if (holidayParser == null) {
-      final String propName = PARSER_IMPL_PREFIX + className;
-      final String parserClassName = getManagerParameter().getProperty(propName);
-      if (parserClassName != null) {
-        final Class<?> parserClass = ClassLoadingUtil.loadClass(parserClassName);
-        holidayParser = (HolidayParser) parserClass.getConstructor().newInstance();
-        parserCache.put(className, holidayParser);
+  private HolidayParser instantiateParser(final String className) {
+
+    final Cache.ValueHandler<HolidayParser> parserValueHandler = new Cache.ValueHandler<>() {
+      @Override
+      public String getKey() {
+        return className;
       }
-    }
-    return holidayParser;
+
+      @Override
+      public HolidayParser createValue() {
+        final String parserClassName = getManagerParameter().getProperty(PARSER_IMPL_PREFIX + className);
+        if (parserClassName != null) {
+
+          try {
+            return (HolidayParser) loadClass(parserClassName).getConstructor().newInstance();
+          } catch (ReflectiveOperationException | SecurityException e) {
+            throw new IllegalStateException("Cannot create parsers.", e);
+          }
+        }
+
+        return null;
+      }
+    };
+
+    return parserCache.get(parserValueHandler);
   }
 
   /**

diff --git a/...ching/HolidayManagerValueHandlerTest.java → .../core/HolidayManagerValueHandlerTest.java b/...ching/HolidayManagerValueHandlerTest.java → .../core/HolidayManagerValueHandlerTest.java
@@ -1,6 +1,7 @@
-package de.focus_shift.jollyday.core.caching;
+package de.focus_shift.jollyday.core;
 
 import de.focus_shift.jollyday.core.HolidayManager;
+import de.focus_shift.jollyday.core.HolidayManagerValueHandler;
 import de.focus_shift.jollyday.core.ManagerParameter;
 import de.focus_shift.jollyday.core.datasource.ConfigurationServiceManager;
 import de.focus_shift.jollyday.core.impl.DefaultHolidayManager;

diff --git a/...ts/src/test/java/de/focus_shift/jollyday/tests/HolidayManagerGetHolidayBenchmarkTest.java b/...ts/src/test/java/de/focus_shift/jollyday/tests/HolidayManagerGetHolidayBenchmarkTest.java
@@ -31,7 +31,7 @@
 @Measurement(iterations = 5, time = 2000, timeUnit = TimeUnit.MILLISECONDS)
 public class HolidayManagerGetHolidayBenchmarkTest extends Benchmarks {
 
-  private static final double REFERENCE_SCORE = 45_000.00;
+  private static final double REFERENCE_SCORE = 22_000_000.00;
 
   @State(Scope.Thread)
   public static class HolidayManagerState {