This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5540-localized-text-provider-caching in repository https://gitbox.apache.org/repos/asf/struts.git
commit 775bf8b2174b037074dc75018534b09d3314d59a Author: Lukasz Lenart <[email protected]> AuthorDate: Thu Jul 23 13:12:23 2026 +0200 WW-5540 perf(core): cache class-hierarchy text resolution Cache the class/interface/superclass traversal in findText keyed on (classloader, class name, key, locale), storing the raw pattern or a NOT_FOUND marker. Formatting stays per call and falls through to the next tier when a cached pattern formats to null. Invalidated on reloadBundles/clearBundle/clearMissingBundlesCache; reload is hoisted to the top of findText so caches are cleared before they are read. Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../text/AbstractLocalizedTextProvider.java | 71 ++++++++++++++ .../struts2/text/StrutsLocalizedTextProvider.java | 27 ++++-- .../java/org/apache/struts2/text/CacheFixture.java | 37 +++++++ .../text/StrutsLocalizedTextProviderTest.java | 107 +++++++++++++++++++++ .../apache/struts2/text/CacheFixture.properties | 4 + 5 files changed, 238 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java b/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java index 222d234a6..dc647d6d2 100644 --- a/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java +++ b/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java @@ -56,6 +56,7 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { private static final String TOMCAT_WEBAPP_CLASSLOADER = "org.apache.catalina.loader.WebappClassLoader"; private static final String TOMCAT_WEBAPP_CLASSLOADER_BASE = "org.apache.catalina.loader.WebappClassLoaderBase"; private static final String RELOADED = "org.apache.struts2.util.LocalizedTextProvider.reloaded"; + private static final String NOT_FOUND = new String("__STRUTS_TEXT_NOT_FOUND__"); // unique identity sentinel; compared with == protected final ConcurrentMap<String, ResourceBundle> bundlesMap = new ConcurrentHashMap<>(); protected boolean devMode = false; @@ -66,6 +67,7 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { private final ConcurrentMap<Integer, List<String>> classLoaderMap = new ConcurrentHashMap<>(); private final Set<String> missingBundles = ConcurrentHashMap.newKeySet(); private final ConcurrentMap<Integer, ClassLoader> delegatedClassLoaderMap = new ConcurrentHashMap<>(); + private final ConcurrentMap<TextCacheKey, String> classHierarchyCache = new ConcurrentHashMap<>(); @Override public void addDefaultResourceBundle(String bundleName) { @@ -90,6 +92,15 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { return Thread.currentThread().getContextClassLoader(); } + private int currentLoaderHashCode() { + return getCurrentThreadContextClassLoader().hashCode(); + } + + /** Test-support accessor: current number of cached class-hierarchy resolutions. */ + protected int classHierarchyCacheSize() { + return classHierarchyCache.size(); + } + @Inject(value = StrutsConstants.STRUTS_CUSTOM_I18N_RESOURCES, required = false) public void setCustomI18NResources(String bundles) { if (bundles == null || bundles.isEmpty()) { @@ -187,6 +198,7 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { protected void clearBundle(final String bundleName, Locale locale) { final String key = createMissesKey(String.valueOf(getCurrentThreadContextClassLoader().hashCode()), bundleName, locale); final ResourceBundle removedBundle = bundlesMap.remove(key); + classHierarchyCache.clear(); LOG.debug("Clearing resource bundle [{}], locale [{}], result: [{}].", bundleName, locale, removedBundle != null); } @@ -204,6 +216,7 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { */ protected void clearMissingBundlesCache() { missingBundles.clear(); + classHierarchyCache.clear(); LOG.debug("Cleared the missing bundles cache."); } @@ -222,6 +235,7 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { } if (!reloaded) { bundlesMap.clear(); + classHierarchyCache.clear(); clearResourceBundleClassloaderCaches(); // now, for the true and utter hack, if we're running in tomcat, clear @@ -620,6 +634,29 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { return null; } + /** + * Cached resolution of the class/interface/superclass hierarchy for a key. Returns the raw pattern + * found, or {@link #NOT_FOUND} when the key is absent from the entire hierarchy. Keyed on the + * context classloader hash + class name + key + locale, so no {@link Class} reference is retained. + * Uses get + putIfAbsent (never computeIfAbsent) because the child-property path recurses into findText. + */ + protected String resolveClassHierarchyRaw(Class<?> clazz, String textKey, String indexedKey, Locale locale) { + TextCacheKey cacheKey = new TextCacheKey(currentLoaderHashCode(), clazz.getName(), textKey, locale); + String cached = classHierarchyCache.get(cacheKey); + if (cached != null) { + return cached; + } + String raw = findMessageRaw(clazz, textKey, indexedKey, locale, null); + String toStore = (raw != null) ? raw : NOT_FOUND; + classHierarchyCache.putIfAbsent(cacheKey, toStore); + return toStore; + } + + /** @return true when a cached raw-resolution result represents "not found". */ + protected boolean isNotFound(String cachedRawResult) { + return cachedRawResult == NOT_FOUND; + } + /** * Traverse up class hierarchy looking for message. Looks at class, then implemented interface, * before going up hierarchy. @@ -702,6 +739,40 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { } } + static class TextCacheKey { + private final int classLoaderHash; + private final String className; + private final String textKey; + private final Locale locale; + + TextCacheKey(int classLoaderHash, String className, String textKey, Locale locale) { + this.classLoaderHash = classLoaderHash; + this.className = className; + this.textKey = textKey; + this.locale = locale; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + TextCacheKey that = (TextCacheKey) o; + return classLoaderHash == that.classLoaderHash + && Objects.equals(className, that.className) + && Objects.equals(textKey, that.textKey) + && Objects.equals(locale, that.locale); + } + + @Override + public int hashCode() { + int result = classLoaderHash; + result = 31 * result + (className != null ? className.hashCode() : 0); + result = 31 * result + (textKey != null ? textKey.hashCode() : 0); + result = 31 * result + (locale != null ? locale.hashCode() : 0); + return result; + } + } + static class GetDefaultMessageReturnArg { String message; boolean foundInBundle; diff --git a/core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java b/core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java index bfdfe22fb..dcee2e579 100644 --- a/core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java +++ b/core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java @@ -65,6 +65,11 @@ public class StrutsLocalizedTextProvider extends AbstractLocalizedTextProvider { LOG.debug("Key is null, short-circuit to default message"); return defaultMessage; } + + // Trigger bundle reload (and cache invalidation) once, before any cached hierarchy lookup, + // so that in reload/devMode the hierarchy caches are cleared before they are read. + reloadBundles(valueStack != null ? valueStack.getContext() : null); + String indexedTextName = extractIndexedName(textKey); // Allow for and track an early lookup for the message in the default resource bundles first, before searching the class hierarchy. @@ -81,11 +86,14 @@ public class StrutsLocalizedTextProvider extends AbstractLocalizedTextProvider { } } - // search up class hierarchy - String msg = findMessage(startClazz, textKey, indexedTextName, locale, args, null, valueStack); - - if (msg != null) { - return msg; + // search up class hierarchy (cached raw resolution; format per call) + String classHierarchyRaw = resolveClassHierarchyRaw(startClazz, textKey, indexedTextName, locale); + String msg = null; + if (!isNotFound(classHierarchyRaw)) { + msg = formatMessage(classHierarchyRaw, locale, valueStack, args); + if (msg != null) { + return msg; + } } if (ModelDriven.class.isAssignableFrom(startClazz)) { @@ -99,9 +107,12 @@ public class StrutsLocalizedTextProvider extends AbstractLocalizedTextProvider { if (action instanceof ModelDriven) { Object model = ((ModelDriven<?>) action).getModel(); if (model != null) { - msg = findMessage(model.getClass(), textKey, indexedTextName, locale, args, null, valueStack); - if (msg != null) { - return msg; + String modelRaw = resolveClassHierarchyRaw(model.getClass(), textKey, indexedTextName, locale); + if (!isNotFound(modelRaw)) { + msg = formatMessage(modelRaw, locale, valueStack, args); + if (msg != null) { + return msg; + } } } } diff --git a/core/src/test/java/org/apache/struts2/text/CacheFixture.java b/core/src/test/java/org/apache/struts2/text/CacheFixture.java new file mode 100644 index 000000000..4e072a570 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/text/CacheFixture.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.text; + +/** + * Simple fixture whose class-associated bundle ({@code CacheFixture.properties}) backs the + * localized-text caching tests. The {@code name} property is exposed so OGNL expressions such as + * {@code ${name}} can be resolved against a value stack. + */ +public class CacheFixture { + + private final String name; + + public CacheFixture(String name) { + this.name = name; + } + + public String getName() { + return name; + } +} diff --git a/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java b/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java index 06c84f4e7..b29426e5e 100644 --- a/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java +++ b/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java @@ -547,6 +547,105 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase { assertEquals("Result of bean2.name lookup not as expected ?", "Okay! You found Me!", messageResult); } + public void testClassHierarchyCacheReusesFoundPattern() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + assertEquals("Cache not empty before first lookup ?", 0, provider.classHierarchyCacheSize()); + String first = provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack); + assertEquals("Static cached value", first); + assertEquals("Cache not populated after found lookup ?", 1, provider.classHierarchyCacheSize()); + + String second = provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack); + assertEquals("Second lookup differs from first ?", first, second); + assertEquals("Cache grew on repeated lookup ?", 1, provider.classHierarchyCacheSize()); + } + + public void testClassHierarchyCacheStoresMisses() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + String first = provider.findText(CacheFixture.class, "cache.missing", Locale.ENGLISH, "Fallback", null, valueStack); + assertEquals("Fallback", first); + assertEquals("Miss not cached ?", 1, provider.classHierarchyCacheSize()); + + String second = provider.findText(CacheFixture.class, "cache.missing", Locale.ENGLISH, "Fallback", null, valueStack); + assertEquals("Fallback", second); + assertEquals("Miss cache grew on repeat ?", 1, provider.classHierarchyCacheSize()); + } + + public void testFormattingIsPerCallNotCached() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + String x = provider.findText(CacheFixture.class, "cache.withparam", Locale.ENGLISH, null, new Object[]{"X"}, valueStack); + String y = provider.findText(CacheFixture.class, "cache.withparam", Locale.ENGLISH, null, new Object[]{"Y"}, valueStack); + assertEquals("Value with param X", x); + assertEquals("Value with param Y", y); + } + + public void testOgnlTranslationIsPerCall() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + valueStack.push(new CacheFixture("World")); + String world = provider.findText(CacheFixture.class, "cache.withognl", Locale.ENGLISH, null, null, valueStack); + valueStack.pop(); + valueStack.push(new CacheFixture("Mars")); + String mars = provider.findText(CacheFixture.class, "cache.withognl", Locale.ENGLISH, null, null, valueStack); + valueStack.pop(); + + assertEquals("Hello World", world); + assertEquals("Hello Mars", mars); + } + + public void testNullFormattingFallsThroughToDefault() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + // "{0}" with a null arg formats to the literal "null"; findText must fall through to the default. + String first = provider.findText(CacheFixture.class, "cache.nullformat", Locale.ENGLISH, "Fallback", new Object[]{null}, valueStack); + assertEquals("Fallback", first); + // Repeat after the pattern is cached — still falls through. + String second = provider.findText(CacheFixture.class, "cache.nullformat", Locale.ENGLISH, "Fallback", new Object[]{null}, valueStack); + assertEquals("Fallback", second); + } + + public void testReloadClearsClassHierarchyCache() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack); + assertEquals("Cache not populated ?", 1, provider.classHierarchyCacheSize()); + + provider.callReloadBundlesForceReload(); + assertEquals("Reload did not clear class hierarchy cache ?", 0, provider.classHierarchyCacheSize()); + } + + public void testClearBundleAndClearMissingCacheEmptyClassHierarchyCache() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack); + assertEquals("Cache not populated ?", 1, provider.classHierarchyCacheSize()); + provider.callClearBundleWithLocale("org/apache/struts2/text/CacheFixture", Locale.ENGLISH); + assertEquals("clearBundle did not empty class hierarchy cache ?", 0, provider.classHierarchyCacheSize()); + + provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack); + assertEquals("Cache not repopulated ?", 1, provider.classHierarchyCacheSize()); + provider.callClearMissingBundlesCache(); + assertEquals("clearMissingBundlesCache did not empty class hierarchy cache ?", 0, provider.classHierarchyCacheSize()); + } + + public void testDeprecatedFindMessageStillDelegates() { + // findMessage leaves findText's hot path in this task; this locks the deprecated delegator. + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + assertEquals("Static cached value", provider.callFindMessage(CacheFixture.class, "cache.static", Locale.ENGLISH, valueStack)); + assertNull(provider.callFindMessage(CacheFixture.class, "cache.missing", Locale.ENGLISH, valueStack)); + } + @Override protected void setUp() throws Exception { super.setUp(); @@ -616,5 +715,13 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase { final Object reloadedObject = ActionContext.getContext().get(RELOADED); return reloadedObject instanceof Boolean && (Boolean) reloadedObject; } + + public int classHierarchyCacheSize() { + return super.classHierarchyCacheSize(); + } + + public String callFindMessage(Class<?> clazz, String key, Locale locale, ValueStack valueStack) { + return super.findMessage(clazz, key, null, locale, null, null, valueStack); + } } } diff --git a/core/src/test/resources/org/apache/struts2/text/CacheFixture.properties b/core/src/test/resources/org/apache/struts2/text/CacheFixture.properties new file mode 100644 index 000000000..71e6ae182 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/text/CacheFixture.properties @@ -0,0 +1,4 @@ +cache.static=Static cached value +cache.withparam=Value with param {0} +cache.withognl=Hello ${name} +cache.nullformat={0}
