Performance optimizations for jinjava 3.0 expression resolution#1300
Merged
jasmith-hs merged 4 commits intomasterfrom Mar 20, 2026
Merged
Performance optimizations for jinjava 3.0 expression resolution#1300jasmith-hs merged 4 commits intomasterfrom
jasmith-hs merged 4 commits intomasterfrom
Conversation
ed7643e to
d746aca
Compare
The cycle detection allocated a HashSet and walked the full parent chain on every new Context (i.e. every enterScope in for loops). This was a defensive check added long ago but is not needed in normal operation and causes measurable overhead in hot template rendering paths. Benchmark: +37% throughput on complexTemplateBenchmark (3,316 -> 4,542 ops/s) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
d746aca to
470417f
Compare
String/Number/Boolean are always allowed return types, so skip the ConcurrentHashMap lookup entirely. Also changed the cache key from String (canonical class name) to Class<?> to avoid the cost of getCanonicalName() on cache hits. Benchmark: +64% throughput on complexTemplateBenchmark (3,316 -> 5,428 ops/s) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
wrap() now short-circuits for String/Number/Boolean before the instanceof chain. getCurrent() was calling CURRENT_INTERPRETER.get() twice per invocation; now stores the result in a local variable. Benchmark: +62% throughput on complexTemplateBenchmark (3,316 -> 5,380 ops/s) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
In 3.0, LegacyOverrides.THREE_POINT_0 defaults usePyishObjectMapper=true, causing every expression output to go through full Jackson ObjectWriter serialization. For String/Number/Boolean (the vast majority of expression results), this is unnecessary -- val.toString() produces the same output. This was the single largest source of the 3.0 vs 2.8.x regression. Benchmark: +104% throughput on complexTemplateBenchmark (3,316 -> 6,772 ops/s) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
470417f to
17dcd0d
Compare
jasmith-hs
commented
Mar 20, 2026
Comment on lines
-29
to
-46
| Set<ScopeMap<K, V>> parents = new HashSet<>(); | ||
| if (parent != null) { | ||
| ScopeMap<K, V> p = parent.getParent(); | ||
| while (p != null) { | ||
| parents.add(p); | ||
| if (parents.contains(parent)) { | ||
| ENGINE_LOG.error( | ||
| "Parent loop detected:\n{}", | ||
| Arrays | ||
| .stream(Thread.currentThread().getStackTrace()) | ||
| .map(StackTraceElement::toString) | ||
| .collect(Collectors.joining("\n")) | ||
| ); | ||
| break; | ||
| } | ||
| p = p.getParent(); | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This is some debug logging that was added a few years ago and isn't functionally necessary
rewaddell
approved these changes
Mar 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Jinjava 3.0 introduced several new features (return type validation, method validation, pyish object mapping by default) that added per-expression overhead in the hot rendering path. JMH benchmarks showed 3.0 was ~34% slower than 2.8.x on
complexTemplateBenchmark(a template with ~274 expression evaluations across nested for loops).These optimizations bring 3.0 performance above 2.8.x levels. Each optimization was benchmarked individually to isolate its contribution:
For reference, jinjava 2.8.x scores ~7,719 ops/s on the same benchmark, so combined these changes make 3.0 slightly faster than 2.8.x.
The biggest single win is the PyishObjectMapper fast-path: 3.0 defaults
usePyishObjectMapper=trueviaLegacyOverrides.THREE_POINT_0, which was routing every expression result through full JacksonObjectWriterserialization even for simple String/Number/Boolean values.BRAVE
Backwards Compatibility
All changes are internal optimizations that preserve existing behavior. The fast-paths return the same results as the full code paths for the types they short-circuit (String, Number, Boolean).
Rollout and Rollback Plan
These ship with jinjava 3.0 which has its own rollout plan via LegacyOverrides. Can be reverted independently if needed.
Automated Testing
Existing test suite covers all modified code paths. JMH benchmarks added in prior commits validate throughput.
Verification
JMH benchmark results shown above. Each optimization was benchmarked in isolation and combined.
Expect Dependencies to Fail
No expected failures — these are internal optimizations with no API changes.
REVIEWERS: Please review both the code changes and the answers above, and validate that they match the expectations for BRAVE