perf(registry): cache linked Handlebars templates per component version - #1567
Open
ricardo-devis-agullo wants to merge 2 commits into
Open
perf(registry): cache linked Handlebars templates per component version#1567ricardo-devis-agullo wants to merge 2 commits into
ricardo-devis-agullo wants to merge 2 commits into
Conversation
oc-template-handlebars render re-runs utils.validator() + handlebars.template() on every render because its internal cache never hits (cache.set omits the value argument, so cache.get misses forever). Wrap the template-link step registry-side with an LRU-bounded cache keyed by template key (WeakMap identity fallback when keyless) and serve oc-client the wrapped module, so linking happens once per component version regardless of the upstream bug. Fail-open: misses delegate to the upstream render, preserving its validation/error semantics.
…ache Custom/test-double template modules may expose only getCompiledTemplate (no getInfo/render). Passing such a module into oc-client breaks its uniqTemplates (reads getInfo().type), so pass them through unwrapped and skip Client injection unless the module satisfies oc-client's contract.
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.
Problem
Every Handlebars render on the registry hot path re-runs
utils.validator()+handlebars.template()(the template-link step). Upstreamoc-template-handlebars@6.0.26lib/render.jsintends to cache the linked template per key but never hits:cache.gettherefore misses forever (andlinkedis never assigned from the cached entry either, so a hit could not even be served). The registry artifact cache (file-contentsinget-component.ts) only caches the precompiled spec (result ofgetCompiledTemplate), so the link step still runs per render insideocTemplate.render(reached via oc-clientTemplateRenderer->client.renderTemplate).Note: the referenced
packages/oc/registry-performance-improvements.mddoes not exist in this worktree, so this implements item 1 from the task description against the code as found.Fix (defensive, registry-side; node_modules untouched)
packages/oc/src/registry/domain/linked-template-cache.ts:withLinkedTemplateCache(template)decorator. Misses delegate to the wrapped module (preserving its validation/error semantics exactly); after a successful render the spec is linked once and cached. Hits skip validator + link and run the cached linked fn against the current model. Fail-open throughout (link/load failures just delegate again).options.key(component template hashKey on the registry path) in an LRU cache (reusesBoundedCache, default 500 entries). Keyless renders fall back to aWeakMapkeyed by spec identity instead of a content hash (specs contain functions invisible to JSON, so hashing would be costly and collision-prone; identity is exact and GC-bounded).get-component.ts: the repository's handlebars module is wrapped once per registry and appended to theClient({ templates })list after user templates (custom templates keep precedence; oc-client's own base copy loses via first-seen uniq), so the hot-pathclient.renderTemplatedispatches to the cached-link render.Verify
test/unit/registry-domain-linked-template-cache.js(10 tests): link-once-per-key, per-key independence, error propagation without caching, fail-open on link throw, linked-run throw without re-delegating, LRU eviction at bound, keyless identity caching, non-object passthrough, api preservation + wrap idempotency, default real-handlebars link path.test/unit/registry-routes-helpers-get-component-linked-cache.js(2 tests): two sequential rendered requests for one Handlebars component through the real oc-client + real template module return identical HTML while the upstreamrender(link step) runs exactly once.registry-routes-helpers-get-component*,registry-domain-register-templates,utils-bounded-cache(39 passing alongside new tests; 25 passing in final re-run).tscclean;biome checkclean (incl. pre-commit hook).Upstream note
Proper upstream fix is one line in
oc-template-handlebars/lib/render.js:cache.set(cacheType, options.key, linked)(plus assigninglinked = cachedon hit). This PR deliberately does not touchnode_modules; the registry-side cache makes renders immune to that bug regardless.Benchmark note
bench:quickwas attempted (baseline on master, comparison on this branch) but the comparison run was invalidated, so no RPS/p95 numbers are reported: thelocal-memoryafter-run served 100% 4xx (thewelcomefixture_packagewas missing while the worktreedistwas still being repaired, so its high RPS is the 404 fast-path, not rendered output), and the remaining scenarios ran while builds and test suites shared the box, making their deltas unreliable in the other direction. Evidence for this change therefore stands on the unit tests (upstream link step runs exactly once per key with byte-identical HTML) plus green CI. A clean-roombench:quickon an idle machine remains the right before/after gate if a reviewer wants numbers.