Repository: focus-shift/jollyday (java)

Issue:
Caching "getHolidays"
### More information

The [isHoliday](https://github.com/focus-shift/jollyday/blob/main/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManager.java#L188) method features a caching implementation, meaning that when a holiday is requested, it is cached for future reference. Leveraging this caching mechanism within the [getHoliday](https://github.com/focus-shift/jollyday/blob/main/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManager.java#L300) function can optimize performance, as it avoids repeating the entire code execution for every call within the classpath. Since the XML files remain static during runtime, this caching mechanism is particularly advantageous, effectively accelerating the library's operations.




Pull request:
Improve `getHoliday` caching and improve ops/s
closes #460 

With the cache mechanism moved from the `isHoliday(..)` method to the basic method `getHoliday(..)` that is also used by `isHoliday(..)`. We will get an nice performance boost from `44437.257 ops/s`

```
Benchmark                                                    Mode  Cnt      Score     Error  Units
HolidayManagerGetHolidayBenchmarkTest.benchmarkGetHolidays  thrpt   15  44437.257 ± 665.105  ops/s
```
see https://github.com/focus-shift/jollyday/pull/474/checks

up to `23832222.693 ops/s`


```
Benchmark                                                    Mode  Cnt         Score        Error  Units
HolidayManagerGetHolidayBenchmarkTest.benchmarkGetHolidays  thrpt   15  23832222.693 ± 870646.268  ops/s
```
see https://github.com/focus-shift/jollyday/actions/runs/8111314784/job/22170341131

That means an improvement of `(23832222 - 44437) / 44437 * 100 = 53531 %`

<!--

Thanks for contributing.
Please review the following notes before submitting you pull request.

Please look for other issues or pull requests which already work on this topic. Is somebody already on it? Do you need to synchronize?

# Security Vulnerabilities

🛑 STOP! 🛑 If your contribution fixes a security vulnerability, please do not submit it.
Instead, please write an E-Mail to to one of the maintainer with all the information
to recreate the security vulnerability.

# Describing Your Changes

If, having reviewed the notes above, you're ready to submit your pull request, please
provide a brief description of the proposed changes.

If they:
 🐞 fix a bug, please describe the broken behaviour and how the changes fix it.
    Please label with 'type: bug' and 'status: new'
    
 🎁 make an enhancement, please describe the new functionality and why you believe it's useful.
    Please label with 'type: enhancement' and 'status: new'
 
If your pull request relates to any existing issues,
please reference them by using the issue number prefixed with #.

-->


Patch:
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
index 397b3512..5a8b6bd2 100644
--- 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 abstract class HolidayManager {
   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 abstract class HolidayManager {
    * @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/jollyday-core/src/main/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandler.java b/jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManagerValueHandler.java
similarity index 75%
rename from jollyday-core/src/main/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandler.java
rename to jollyday-core/src/main/java/de/focus_shift/jollyday/core/HolidayManagerValueHandler.java
index 1397614f..f12237ab 100644
--- a/jollyday-core/src/main/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandler.java
+++ b/jollyday-core/src/main/java/de/focus_shift/jollyday/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
index 95b1e595..117f8fa3 100644
--- 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.CalendarHierarchy;
 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.Map;
 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 class DefaultHolidayManager extends HolidayManager {
    */
   @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 @@ public class DefaultHolidayManager extends HolidayManager {
     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);
   }
 
   /**


Test patch:
diff --git a/jollyday-core/src/test/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandlerTest.java b/jollyday-core/src/test/java/de/focus_shift/jollyday/core/HolidayManagerValueHandlerTest.java
similarity index 97%
rename from jollyday-core/src/test/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandlerTest.java
rename to jollyday-core/src/test/java/de/focus_shift/jollyday/core/HolidayManagerValueHandlerTest.java
index f79ef045..69cbf0ab 100644
--- a/jollyday-core/src/test/java/de/focus_shift/jollyday/core/caching/HolidayManagerValueHandlerTest.java
+++ b/jollyday-core/src/test/java/de/focus_shift/jollyday/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/jollyday-tests/src/test/java/de/focus_shift/jollyday/tests/HolidayManagerGetHolidayBenchmarkTest.java b/jollyday-tests/src/test/java/de/focus_shift/jollyday/tests/HolidayManagerGetHolidayBenchmarkTest.java
index 5c30ef0b..eb303b5b 100644
--- a/jollyday-tests/src/test/java/de/focus_shift/jollyday/tests/HolidayManagerGetHolidayBenchmarkTest.java
+++ b/jollyday-tests/src/test/java/de/focus_shift/jollyday/tests/HolidayManagerGetHolidayBenchmarkTest.java
@@ -31,7 +31,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 @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 {