Repository navigation
perf: dedupe image fetches within a single render - #66
Merged
Merged
Conversation
The settle loop reloads a page's images after any script-driven relayout
(dynamic.go's `if relaidOut && !overBudget() { e.loadImages(...) }`), and
e.ImageCache is opt-in / nil by default — its doc comment is explicit that
this means "fetch every time" across renders, a deliberate choice. But
within ONE render, reloading is unconditional too: an image whose src never
changed still pays a second full network round trip.
Found while profiling caniuse.com (chasing its ~4.2s render time): three
images were fetched twice, ~500ms apart in the trace, both times for
identical bytes. (The page's remaining ~3.6s is a separate, non-actionable
finding: the request timeline shows the classic shape of server-side
connection pacing on caniuse.com's own origin — confirmed by fetching the
same 4 stylesheets directly with the same client outside the engine, which
showed the identical clustering, and by the bench corpus's own Chrome
comparison already timing this page at ~4s. Not a client bug.)
Fix: imgByteCache, an ephemeral in-memory cache keyed by absolute URL,
created fresh per render in renderCoreStaged and carried through
context.Context (which already threads unchanged into every image-fetching
call — the initial pass, the settle loop, the meta-fallback path — so no
signature changes were needed at any of those call sites). fetchImageBytes
checks it before e.ImageCache and before the network, and populates it
after every successful fetch regardless of source. Cross-render behaviour
is unchanged: a fresh render gets a fresh cache, so nil e.ImageCache still
means "fetch every render" exactly as documented — only the redundant
fetch WITHIN one render is eliminated.
caniuse.com: 19 requests -> 16, ~4.16s -> ~3.64s.
tannevaled
added a commit
that referenced
this pull request
Aug 31, 2026
Regenerates results.json/REPORT.md's table and montages against the merged grid track-sizing fix (#63), the esbuild sandbox-escape fix (#65), and the two perf dedupe wins (#64, #66). Records all four in FIDELITY.md's dated log, closes the CSS Grid "Known gaps" bullet (now fixed), and adds an honest note on tailwindcss.com's SSIM number after visually checking the montage.
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.
Summary
e.ImageCacheis opt-in (nil by default, deliberately — see its doc comment, "fetch every time" across renders), but this waste happens within a single render too.imgByteCache, an ephemeral per-render cache keyed by absolute URL, created fresh inrenderCoreStagedand threaded viacontext.Context(which already flows unchanged into every image-fetching call site, so no signature changes needed elsewhere). Checked beforee.ImageCacheand before the network.nile.ImageCachestill means "fetch every render" exactly as documented.Impact
Test plan
go test -short ./...— full suite green.TestImgByteCacheDedupesWithinRender: 3 fetches of the same URL under one render context hit the server exactly once.TestImgByteCacheScopedPerContextNotGlobal: two independent render contexts each fetch once (2 total) — proves the cache does NOT leak across renders.go vet ./...clean;dom/css/layout/paintcoverage floors all still met.