dotCMS / dotCMS/core

perf(velocity): remove avoidable per-render allocations in template engine hot path

Open Beginner friendly
#36,201 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

stale
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

  1. Hoist the split regex to a precompiled static final Pattern (compile once at class load).
  2. Reuse ArrayUtils.EMPTY_OBJECT_ARRAY for the zero-arg case in ASTMethod, mirroring the existing paramClasses optimization.

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.
  • StringWriter per interpolated string literal: speculative; the buffer is unavoidable and the synchronized StringBuffer is 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 VelocityResourceKeyTest covers parsing behavior; a variant-with-underscores case is added.
  • New ASTMethodTest renders zero-arg, single/multi-arg, repeated, and mixed/nested method calls through Velocity to lock in behavior.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.