perf(velocity): remove avoidable per-render allocations in template engine hot path
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 970
- Forks
- 486
- Avg merge
- 3d 33m
- Merged PRs (30d)
- 170
Description
Problem
Two spots in the Velocity rendering hot path do avoidable work on every template render. Both are pure CPU/allocation overhead with no behavioral effect.
1. VelocityResourceKey recompiles a regex on every cache lookup
VelocityResourceKey is constructed on every Velocity resource cache get() — i.e. once per page/template/container/contentlet/field lookup, dozens to hundreds of times per page render. Its constructor calls:
final String[] pathArry = path.split("[/\\.]", 0);
String.split only fast-paths single literal characters; a character class ([/\.]) falls through to Pattern.compile(...) on every call. That's regex compilation on the hottest path in the rendering engine.
2. ASTMethod allocates new Object[0] for every zero-arg method call
org.apache.velocity.runtime.parser.node.ASTMethod.execute() does:
Object [] params = new Object[paramCount];
For zero-arg calls ($foo.size(), $x.toUpperCase(), $date.getTime() — extremely common), this allocates a throwaway new Object[0] every render. The sibling paramClasses array already uses the shared ArrayUtils.EMPTY_CLASS_ARRAY for the zero-arg case; params was simply left inconsistent.
Fix
- Hoist the split regex to a precompiled
static final Pattern(compile once at class load). - Reuse
ArrayUtils.EMPTY_OBJECT_ARRAYfor the zero-arg case inASTMethod, mirroring the existingparamClassesoptimization.
Both changes are behavior-identical.
Explicitly out of scope (and why)
- Arg-array pooling (
new Object[n]/new Class[n]for n>0): unsafe — method calls are re-entrant ($a.foo($b.bar())), so a ThreadLocal buffer corrupts. The arrays are tiny, eden-allocated, and die young. StringWriterper interpolated string literal: speculative; the buffer is unavoidable and the synchronizedStringBufferis usually lock-elided by the JIT. Touching the fragile interpolation kludge isn't worth an unmeasured gain.- MethodHandle caching for invocation: redundant on Java 25 — core reflection has been MethodHandle-backed since JDK 18 (JEP 416), and the resolved executor is already cached per node/class. No reflection-lookup churn to remove.
Verification
- Existing
VelocityResourceKeyTestcovers parsing behavior; a variant-with-underscores case is added. - New
ASTMethodTestrenders zero-arg, single/multi-arg, repeated, and mixed/nested method calls through Velocity to lock in behavior.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with VelocityResourceKey and ASTMethod, then read the existing VelocityResourceKeyTest and the requested ASTMethodTest coverage. Verify the cache-key parsing behavior, zero-argument method calls, argument counts, repeated calls, and nested calls; done means both hot-path allocations are removed without changing rendering behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, performance
- Issue type
- Refactor
- Difficulty
- 2/5
- Estimated time
- Half a day
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 82/100