perf(ENGKNOW-3939): remove global lock and per-call re-parse from yml template rendering

[gmagnu]

Problem

Rendering a yml (freemarker) template query (gor some.yml?a=1&b=2, gor some.yml(a=1), including template:// sources) goes through FreemarkerQueryUtilities.requestQuery, which was synchronized public static. That serialises every yml render in the JVM. Each call also:

• re-read and re-parsed the yml with SnakeYAML,
• built a new freemarker Configuration per dialog, so the query template was re-compiled on every call,
• re-rendered the whole query once in the dialog constructor and once more for every argument set (setArgumentValues fires a property change per argument, and each change re-renders). A template that calls gor(...) ran those queries every time.

Why the lock was there

The lock was hiding real shared mutable state. Removing only the synchronized keyword makes the new concurrency test fail at once with NumberFormatException: For input string: ".1E.01E0". The shared state was:

1. RangedNumberFormatter passed one static NumberFormat (not thread-safe) to every NumberArgument formatter.
2. Perspective kept a static freemarker Configuration with a static StringTemplateLoader (backed by a plain HashMap) and wrote to it from every dialog build. It also used one shared, stateful skip method model.
3. Each Dialog configuration held stateful objects that were reset for each render: the skip method model and the DialogTemplateExceptionHandler, which counts missing required arguments.

Change

• Parsed-definition cache (AbstractDialogFactory.buildDialogs(resource, cacheDir)): the yml is still read on every call, but it is only re-parsed when its content differs from the cached copy. The cache is keyed on (resource, cacheDir) and bounded by Caffeine. Detection does not rely on modification times, so an edited file is always picked up. Every call gets a deep copy of the parsed map, so its dialog and argument objects are its own. Embedded ${key} files are re-created if they have been removed from the cache dir.
• Compiled-template cache (TemplateCache, new, Caffeine-bounded): compiled templates are keyed on (configuration, name, full source). An edited template compiles to a new entry, and a hash collision cannot hand out another template's source.
• Shared, read-only freemarker configuration in Dialog. It is set up once in static init and never modified. All per-render state is now created for each render and set on that render's Environment: a new skip model, gor bound to the caller's QueryEvaluator, and a new exception handler. Data-model arguments named skip/gor still take precedence, as before. When dialog.macrodir is set, macros load through the dialog's file resolver, so each dialog keeps its own configuration, as before.
• Perspective compiles its filter/view templates through TemplateCache rather than writing them to the shared loader, and sets skip and its exception handler on each render's Environment.
• RangedNumberFormatter gives each formatter its own clone of the default NumberFormat.
• requestQuery sets setDeferUpdates(true) before setting arguments, so the query is rendered once, with all arguments set, when it is requested. It no longer renders once per argument.
• synchronized removed from requestQuery. The private getQuery helper is unchanged.

Thread-safety argument

After this change, requestQuery only shares these objects between threads:

• Freemarker Configurations (Dialog shared config, Perspective static config). They are fully configured in static initialisers and not modified afterwards. Freemarker documents that a Configuration is safe for concurrent use once it is no longer modified.
• Compiled Templates (via TemplateCache). Freemarker documents that a Template can be processed by many threads at once. We no longer call Template.setTemplateExceptionHandler. Every mutable piece of render state is on the per-render Environment.
• Caffeine caches. They are thread-safe. Cached parsed maps are never handed out, only deep copies.
• Read-only statics (ArgumentType map, the default NumberFormat, which is now only cloned). ArgumentBuilder.getPathMap was already synchronized.

Everything mutable is created per call: dialogs, arguments, the attribute maps (deep copies), SkipFirstMethodModel, exception handlers, QueryEvalMethodModel and writers.

Behaviour notes

• With deferred updates, if rendering the final query (all arguments set) throws, the error now reaches the caller. Before, the listener logged the exception, and getQuery() could silently return the output of an earlier render made with only some arguments set.
• Two concurrent first renders of the same new template may both parse it and both write the same embedded ${key} file. The content is identical, and the same class of race already exists for Cmd. It is not made worse, because on a cache hit the file is only rewritten when it is missing.

Measurements

I timed this with a throwaway harness (not committed). It rendered a liftover-style template through requestQuery: <#setting>, six <#assign>s, ?matches guards, nested <#if>, 9 arguments, a ~35-line query. Setup: local file, JDK 17, 1000 warm-up calls, then 2000 timed calls. For the concurrent case, 8 threads each did 2000 calls with different arguments, and every output was checked. Ranges are over repeated runs on a laptop.

Where the time went before (per call, single thread):

piece	cost
read + SnakeYAML parse	~0.4-0.7 ms
new Configuration + template compile	~0.7-0.9 ms
one render	~15-35 µs (× 1 + one per argument)

Before / after:

	before	after
1 thread p50	740-890 µs	88-96 µs
1 thread p95	1.9-6.6 ms	0.19-0.59 ms
8 threads p50	690-930 µs	135-151 µs
8 threads p95	23-59 ms	1.5-2.6 ms
8 threads p99	143-268 ms	11-13 ms
8 threads throughput	580-1060 renders/s	9 800-12 600 renders/s

Tests

New gorsat.UTestTemplateRenderingConcurrency:

• renderOutputReflectsArguments: checks the output, including skip() in the query and in a perspective filter and a number argument. An optional argument from one call does not leak into the next.
• concurrentRendersWithDifferentArgumentsProduceTheirOwnOutput: 8 threads × 500 renders over 200 argument sets, each compared with the output of a single-threaded render. This test fails on the old code with only synchronized removed.
• editedTemplateIsPickedUp: the file is rewritten with the same length and the same modification time, and the new content is used.
• unchangedTemplateIsNotReparsed: 48 more renders with an unchanged file cause no yml parse and no template compile. After an edit, the file is re-parsed exactly once.
• embeddedFileIsRestoredIfCacheDirIsCleaned

Also ran: the yml/template-related classes (UTestGorTemplate, UTestGorWrite, UTestTableFunction, UTestInputSourceParsing, UTestCommandParsing, UTestMacroUtilities, UTestGorPrePipe, UTestMacroParsing): 254 tests, 0 failures. Full ./gradlew :gortools:test: 2600 tests, 0 failures, 69 skipped.

🤖 Generated with Claude Code

[github-actions]

Junit Tests - Summary

4 862 tests  +11   4 691 ✅ +12   19m 6s ⏱️ - 2m 6s
  506 suites + 1     171 💤  -  1 
  506 files   + 1       0 ❌ ± 0 

Results for commit c2e0101. ± Comparison against base commit 9536c2f.

♻️ This comment has been updated with latest results.