diff --git a/src/main/java/org/rutebanken/util/LocalTimeISO8601XmlAdapter.java b/src/main/java/org/rutebanken/util/LocalTimeISO8601XmlAdapter.java index bdde2eb..372859f 100644 --- a/src/main/java/org/rutebanken/util/LocalTimeISO8601XmlAdapter.java +++ b/src/main/java/org/rutebanken/util/LocalTimeISO8601XmlAdapter.java @@ -22,7 +22,7 @@ import java.time.format.DateTimeFormatterBuilder; import java.time.temporal.ChronoField; import java.util.HashMap; -import java.util.concurrent.ConcurrentHashMap; +import java.util.Map; public class LocalTimeISO8601XmlAdapter extends XmlAdapter { @@ -34,21 +34,41 @@ public class LocalTimeISO8601XmlAdapter extends XmlAdapter { // .parseDefaulting(ChronoField.OFFSET_SECONDS,OffsetDateTime.now().getLong(ChronoField.OFFSET_SECONDS) ).toFormatter(); + private static final int SECONDS_PER_DAY = 24 * 60 * 60; /** - * We store a cache of parsed LocalTime instances to avoid wasting memory in immutable value - * objects that strictly identical and interchangeable. - * - * We only cache times that are full seconds to avoid increasing the size of the cache unduly, - * since there is a limited number of seconds in a single day. + * Parsing a LocalTime with a DateTimeFormatter is expensive and shows up in real-world + * profiles, since NeTEx documents contain a huge number of time values. Since there are only + * 86400 distinct whole-second times in a day, we precompute all of them once: as canonical + * LocalTime instances indexed by second-of-day (for reuse/dedup), and by their formatted + * string representation (for a direct lookup that bypasses the parser entirely for the common + * "HH:mm:ss" case). */ - private final ConcurrentHashMap cache = new ConcurrentHashMap<>(); + private static final LocalTime[] TIMES_BY_SECOND_OF_DAY = new LocalTime[SECONDS_PER_DAY]; + private static final Map TIME_BY_STRING; + + static { + var tmp = new HashMap(SECONDS_PER_DAY * 2); + for (int secondOfDay = 0; secondOfDay < SECONDS_PER_DAY; secondOfDay++) { + LocalTime time = LocalTime.ofSecondOfDay(secondOfDay); + TIMES_BY_SECOND_OF_DAY[secondOfDay] = time; + tmp.put(formatter.format(time), time); + } + TIME_BY_STRING = Map.copyOf(tmp); + } @Override public LocalTime unmarshal(String input) { + // fast path: avoid the DateTimeFormatter parser entirely for plain whole-second times + LocalTime cached = TIME_BY_STRING.get(input); + if (cached != null) { + return cached; + } + var key = LocalTime.parse(input, formatter); - // only cache if nano is zero - if(key.getNano() == 0){ - return cache.computeIfAbsent(key, time -> time); + // only reuse the cached instance if nano is zero, so as not to increase the size of the + // cache unduly, since there is a limited number of seconds in a single day + if (key.getNano() == 0) { + return TIMES_BY_SECOND_OF_DAY[key.toSecondOfDay()]; } // sub-second times are not cached to not increase the size of the cache unduly else { diff --git a/src/test/java/org/rutebanken/util/LocalTimeISO8601XmlAdapterTest.java b/src/test/java/org/rutebanken/util/LocalTimeISO8601XmlAdapterTest.java index f32e1af..b875436 100644 --- a/src/test/java/org/rutebanken/util/LocalTimeISO8601XmlAdapterTest.java +++ b/src/test/java/org/rutebanken/util/LocalTimeISO8601XmlAdapterTest.java @@ -146,4 +146,43 @@ public void testCachingIdenticalValue() { parsed.forEach(time -> assertSame(first, time, "Same time value should return same instance")); } + + @Test + public void testFastPathReturnsSameInstanceAsParsedEquivalent() { + // "10:20:30" is a plain "HH:mm:ss" string that hits the precomputed second-of-day cache + // directly, without ever going through the DateTimeFormatter parser. Values that must go + // through the parser (offset, fractional zero) should still resolve to that same cached + // instance rather than a freshly parsed, distinct object. + LocalTime fastPath = adapter.unmarshal("10:20:30"); + LocalTime viaOffset = adapter.unmarshal("10:20:30+02:00"); + LocalTime viaFraction = adapter.unmarshal("10:20:30.000"); + LocalTime viaFractionAndOffset = adapter.unmarshal("10:20:30.000-05:00"); + + assertSame(fastPath, viaOffset); + assertSame(fastPath, viaFraction); + assertSame(fastPath, viaFractionAndOffset); + } + + @Test + public void testEverySecondOfTheDayIsCachedAndConsistent() { + for (int secondOfDay = 0; secondOfDay < 24 * 60 * 60; secondOfDay += 37) { + LocalTime expected = LocalTime.ofSecondOfDay(secondOfDay); + String input = String.format("%02d:%02d:%02d", expected.getHour(), expected.getMinute(), expected.getSecond()); + + LocalTime first = adapter.unmarshal(input); + LocalTime second = adapter.unmarshal(input); + + assertEquals(expected, first); + assertSame(first, second, "Repeated unmarshal of '" + input + "' should return the cached instance"); + } + } + + @Test + public void testSubSecondTimesAreNotCached() { + LocalTime first = adapter.unmarshal("18:00:00.001"); + LocalTime second = adapter.unmarshal("18:00:00.001"); + + assertEquals(first, second); + assertNotSame(first, second, "Sub-second times should not be served from the whole-second cache"); + } }