This is an automated email from the ASF dual-hosted git repository. papegaaij pushed a commit to branch resource-name-iterator-alloc in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 00c2ef56f1d5b79fa39e6c78cc0e1e93b4a166ec Author: Emond Papegaaij <[email protected]> AuthorDate: Sun Sep 6 07:09:37 2026 +0000 Stop re-deriving the locale part of every candidate resource name ResourceNameIterator.toString() built each candidate name from four prepend() calls, each concatenating a char with an Object, so four throwaway strings were allocated on top of the result. The locale part was the expensive one: it went through getLocale(), which calls Locale.of() - a LocaleObjectCache lookup rather than a field read - even though LocaleResourceNameIterator.next() had just built the identical suffix and discarded it. next() now keeps that suffix and toString() reuses it; getLocale() memoises the derived Locale per state for the locator's own read-back; the style and variation parts are rebuilt only when the style iterator advances; and each branch of toString() is a single concatenation, so only the returned string is allocated. ExtensionResourceNameIterator was also building its backing iterator twice. ResourceNameIteratorBenchmark, nl_NL without a style, 3 forks: walkAllCandidates (miss) 923.8 -> 167.8 ns/op 1224 -> 736 B/op firstCandidate (hit) 775.2 -> 421.6 ns/op 600 -> 432 B/op Verified equivalent by enumerating every candidate name and its locale, style, variation and extension across 6 paths x 2 styles x 2 variations x 7 locales x 5 extension lists x strict/non-strict - 9918 names - before and after the change: byte-identical. One behavioural note for reviewers: toString() now takes the locale segment from next() instead of from getLocale(). For the shipped implementation those are provably the same, which is what the enumeration above establishes. A subclass supplied through the protected newLocaleResourceNameIterator hook that overrode getLocale() without also overriding next() would see a difference; nothing in the tree does that. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../locator/ExtensionResourceNameIterator.java | 5 ++-- .../locator/LocaleResourceNameIterator.java | 34 ++++++++++++++++++++++ .../resource/locator/ResourceNameIterator.java | 30 ++++++++++++------- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ExtensionResourceNameIterator.java b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ExtensionResourceNameIterator.java index b1ffe2762a..540f8efda3 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ExtensionResourceNameIterator.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ExtensionResourceNameIterator.java @@ -42,13 +42,14 @@ public class ExtensionResourceNameIterator implements Iterator<String> public ExtensionResourceNameIterator(final Iterable<String> extensions) { // Fail safe: hasNext() needs to return at least once with true - if (extensions == null || !extensions.iterator().hasNext()) + Iterator<String> extensionIterator = extensions == null ? null : extensions.iterator(); + if (extensionIterator == null || !extensionIterator.hasNext()) { this.iterator = NULL_ITERABLE.iterator(); } else { - this.iterator = extensions.iterator(); + this.iterator = extensionIterator; } } diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/LocaleResourceNameIterator.java b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/LocaleResourceNameIterator.java index 3faedccda6..8edf3436df 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/LocaleResourceNameIterator.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/LocaleResourceNameIterator.java @@ -39,6 +39,13 @@ public class LocaleResourceNameIterator implements Iterator<String> private final boolean strict; + /** The locale part of the resource name that {@link #next()} produced for this state */ + private String suffix = ""; + + private int localeState = -1; + + private Locale currentLocale; + /** * Construct. * @@ -55,6 +62,18 @@ public class LocaleResourceNameIterator implements Iterator<String> * @return Locale */ public Locale getLocale() + { + // Locale.of() is a cache lookup, not a field read, and this is called once per candidate + // name; the state only changes in next(). + if (localeState != state) + { + localeState = state; + currentLocale = localeForState(); + } + return currentLocale; + } + + private Locale localeForState() { if (state == 1) { @@ -72,6 +91,15 @@ public class LocaleResourceNameIterator implements Iterator<String> return null; } + /** + * @return the locale part of the resource name for the current state, already prefixed with + * {@code '_'}, or the empty string if this state carries no locale + */ + public String getSuffix() + { + return suffix; + } + /** * * @see java.util.Iterator#hasNext() @@ -93,6 +121,12 @@ public class LocaleResourceNameIterator implements Iterator<String> */ @Override public String next() + { + suffix = nextSuffix(); + return suffix; + } + + private String nextSuffix() { if (locale == null) { diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceNameIterator.java b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceNameIterator.java index f89c959a3a..93265c880d 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceNameIterator.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceNameIterator.java @@ -69,6 +69,11 @@ public class ResourceNameIterator implements IResourceNameIterator // The various iterators used to locate the resource file private final StyleAndVariationResourceNameIterator styleIterator; private LocaleResourceNameIterator localeIterator; + + // The variation and style parts of the name, rebuilt only when the style iterator advances. + private String variationPart = ""; + + private String stylePart = ""; private ExtensionResourceNameIterator extensionsIterator; /** @@ -199,6 +204,8 @@ public class ResourceNameIterator implements IResourceNameIterator while (styleIterator.hasNext()) { styleIterator.next(); + variationPart = part(styleIterator.getVariation(), '_'); + stylePart = part(styleIterator.getStyle(), '_'); localeIterator = newLocaleResourceNameIterator(locale, strict); while (localeIterator.hasNext()) @@ -244,19 +251,22 @@ public class ResourceNameIterator implements IResourceNameIterator @Override public String toString() { - return path + prepend(getVariation(), '_') + prepend(getStyle(), '_') + - prepend(getLocale(), '_') + prepend(getExtension(), '.'); + // The locale part was already built by LocaleResourceNameIterator.next(); deriving it + // again through getLocale() would cost a Locale.of() lookup and its string form for every + // candidate name. Each branch is a single concatenation, so only the result is allocated. + String localePart = localeIterator != null ? localeIterator.getSuffix() : ""; + String extension = getExtension(); + + if (extension == null) + { + return path + variationPart + stylePart + localePart; + } + return path + variationPart + stylePart + localePart + '.' + extension; } - /** - * - * @param string - * @param prepend - * @return The string prepended with the char - */ - private String prepend(Object string, char prepend) + private static String part(String value, char prepend) { - return (string != null) ? prepend + string.toString() : ""; + return value == null ? "" : prepend + value; } /**
