Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions FIDELITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@ The committed PNGs under `testdata/renders/` back every claim here. Reproduce
them with the commands at the bottom. The measured-vs-Chrome numbers live in
[`bench/REPORT.md`](bench/REPORT.md).

## 2026-10-10 (round 168) — the 500-page bench corpus completed a full run for the first time (both crash fixes held); one real performance bug found and fixed along the way: font-face fallback candidates in one slot were tried one at a time, not concurrently (engine#267, patch)

The 500-page run relaunched at the end of round 167 (both crash fixes in place) ran to completion: 500/500 pages, `wrote results-500.json and REPORT-500.md`, no crash — the first time this corpus has ever finished. Of the 96 pages marked "failed", 95 failed for BOTH webengine and Chrome (dead URLs, DNS, timeouts — confirmed via each entry's own `note` field) and exactly 1 was a transient DNS flake on this machine; zero are engine bugs. Ten-odd pages (`azure.microsoft.com`, `www.cs.cmu.edu`, `www.cia.gov`, `aws.amazon.com`, the `gitlab.com`/`about.gitlab.com` family, `foundation.zurb.com`, `www.lemonde.fr`) rendered 10-20× slower than Chrome (20-45s vs 1.5-5s), which is not a crash but is squarely what "mesure... améliore les performances" asks for.

- **Investigation**: profiled `www.cs.cmu.edu` (45s) first since its page is small and simple — the CPU profile showed almost no computation (220ms of samples over 45s wall time), ruling out a CPU-bound layout/paint cost. Raising `maxPerHostConcurrency` (transport.go) from 2 to 6 made ZERO measurable difference (45.09s both times, suspiciously identical) — a useful negative result, not a dead end: it meant the per-host semaphore wasn't the active bottleneck for whatever was actually slow, so the change was reverted rather than shipped on a hunch. Instrumented `e.Client.Transport` directly (a temporary wrapper logging every request's URL, status and duration, in a scratch harness, not committed) with `DisableJS` set to rule out JS first: the slow pattern persisted with JS off, isolating it to the fetch layer.
- **Root cause**: cs.cmu.edu's own Font Awesome v4-compatibility shim (`fa/css/all-*.css`) declares five separate `@font-face` rules under the legacy family `'FontAwesome'`, all `font-weight: 400` with no `font-style`, differentiated ONLY by `unicode-range` (one per icon subset: solid, brands, regular, and a `fa-v4compatibility` catch-all). This engine has no `unicode-range` model at all — `css.FontFace` carries no such field — so `LoadFontFaces`'s own slot key (`family + italic + weight`) treats all five as fallback alternatives of ONE logical face, exactly the shape round 151 already parallelizes ACROSS (each slot loads concurrently) but NOT within: `loadOneFontFace`'s per-slot candidate loop tries them one at a time, stopping only at a success. cs.cmu.edu's own `/webfonts/` path was returning HTTP 500 for every one of them, and the ORIGIN took the full ~6.4s doing it — so the render serially burned through up to 5 candidates × 2 format fallbacks (woff2, then ttf) ≈ 50+ seconds of pure sequential failure, for a resource the page's own markup never asked to be tried one-by-one; a real browser's own unicode-range-aware subsetting would have made this moot, but even without implementing that, nothing requires trying candidate rules in series once declaration order is preserved by sorting on completion rather than by waiting in turn.
- **Fixed**: `LoadFontFaces`'s per-slot loop now fires every DISTINCT candidate concurrently (deduped by a new `srcsKey` — the same src list parsed twice, e.g. the same stylesheet named twice, still fetches once) and picks the lowest-declaration-order success after they all finish, instead of walking the list waiting for each failure in turn. `e.Client`'s existing per-host semaphore (`maxPerHostConcurrency`, round 57/58, left at 2 — the round-152/153-measured value, re-confirmed still right for Wikimedia, not re-opened by this round) still bounds real network concurrency, so this is a bounded, not unlimited, speedup: candidates now batch through that cap rather than waiting out every predecessor's own fetch one at a time.
- **Test**: `TestLoadFontFacesTriesSlotCandidatesConcurrentlyNotSequentially` — a local server with 5 slow-then-500 candidates plus 1 fast-success candidate declared LAST in one slot, deliberately shaped like the live cs.cmu.edu case. Stash-verified: ~460ms with the fix (bounded by the per-host cap, confirmed deterministic across 3 runs) vs ~760ms on the unfixed code (sequential), a measured, reproducible, and reliably-separated difference, not assumed from the live site alone (which could not be cleanly re-measured after this round's own repeated testing had, by its own escalating response times, visibly triggered the origin's own rate-limiting/anti-scraping defenses — an honest limitation of live-site verification for this specific fix, mitigated by the deterministic local-server test instead).
- **Coverage**: `fontfaces.go`'s `LoadFontFaces` and the new `srcsKey` both land at 100%; the top-level `engine` package moves from a pre-existing 96.1% baseline (confirmed via `git stash`, unrelated gaps) to 96.2% — no regression.
- **Not fixed this round, flagged honestly**: `unicode-range` itself remains unmodelled (this fix removes the serialization cost of the collision, not the collision — all five rules still compete for one slot, and whichever succeeds first wins for the WHOLE family+weight regardless of which glyphs it actually covers; a page relying on Font Awesome's REAL icon-subset behavior could still render the wrong glyphs, just not slowly). The other ~9 slow pages from this round's bench run were not individually profiled — `cs.cmu.edu`'s own shape (several same-weight legacy-shim font-face rules hitting a consistently-erroring endpoint) is a plausible but UNCONFIRMED shared cause; re-measuring the full 500-page corpus once this fix lands is the natural next check.

## 2026-10-09 (round 167) — the interrupted 500-page bench run (round 166's own queued work) found a second real crash: a lone CSS Grid item using `grid-row:1 / -1` with no explicit row template panicked on an empty row-offset slice (engine#266, patch)

Round 166's fix shipped, then the still-queued 500-page bench run (launched before that fix, to measure the round-165 rotate fix) was resumed to see it through. It crashed again, on page 406: `https://www.lego.com/`, with `panic: runtime error: index out of range [0] with length 0` in `layout.grid` at `rowY[it.r0]`. Reproduced directly with `cmd/render` against the live URL, independent of the bench harness.
Expand Down
79 changes: 74 additions & 5 deletions fontfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,21 @@ type LoadedFontFace struct {
//
// Slots load concurrently: each @font-face is one network round trip, and a
// page naming ten faces otherwise waits for all ten in turn (tailwindcss.com,
// round 151: ~4.4s of serial font downloads). Within a slot the candidates
// still run in declaration order, stopping at the first that loads, so the
// same fetches happen as before; the result keeps declaration order.
// round 151: ~4.4s of serial font downloads). Within a slot, every candidate
// rule ALSO fetches concurrently (round 168) rather than one at a time: this
// engine has no unicode-range model (css.FontFace carries no such field), so
// several genuinely distinct @font-face rules that a real browser would pick
// among BY CODEPOINT collapse into one slot here, as redundant-looking
// alternatives of each other — confirmed live on cs.cmu.edu's Font Awesome
// v4-compatibility shim, which declares five separate 'FontAwesome'-family,
// weight-400 rules (solid/brands/regular/compat, split only by unicode-range)
// that all land in ONE slot. Trying them one at a time cost ~52s when the
// origin's /webfonts/ path was erroring (each failed attempt still waits out
// the full ~6.4s server response): a page need not be malicious or even
// unusual for a single slot to carry several slow-to-fail candidates. The
// result keeps declaration order regardless of which candidate's fetch
// finishes first — same winner as the old sequential loop, just not
// waiting for each loser to fail before starting the next.
func (e *Engine) LoadFontFaces(ctx context.Context, doc *Document, sheets []string, m css.Media) []LoadedFontFace {
type candidate struct {
idx int
Expand Down Expand Up @@ -91,9 +103,48 @@ func (e *Engine) LoadFontFaces(ctx context.Context, doc *Document, sheets []stri
wg.Add(1)
go func(i int, cands []candidate) {
defer wg.Done()
// Every DISTINCT candidate in this slot fetches concurrently (not
// one at a time, stopping only at a success) — see this
// function's own doc comment for why a slot can hold several
// genuinely distinct, slow-to-fail rules rather than true format
// alternatives of one file. A rule repeated verbatim (the same
// src list parsed from two stylesheets, or the same stylesheet
// named twice) still fetches only once: deduped by its own src
// key below, same as the old sequential loop's incidental effect
// of never re-fetching a slot's later identical entry once an
// earlier one had already resolved it. Declaration order still
// decides the winner among distinct candidates: sorted by idx
// below, so which fetch happens to FINISH first never matters.
type attempt struct {
idx int
lf LoadedFontFace
ok bool
}
firstIdx := map[string]int{} // src key -> that key's own goroutine slot in attempts
var unique []candidate
for _, c := range cands {
if lf, ok := e.loadOneFontFace(ctx, doc, c.face); ok {
results[i] = loaded{c.idx, lf}
key := srcsKey(c.face.Srcs)
if _, dup := firstIdx[key]; dup {
continue
}
firstIdx[key] = len(unique)
unique = append(unique, c)
}
attempts := make([]attempt, len(unique))
var cwg sync.WaitGroup
for j, c := range unique {
cwg.Add(1)
go func(j int, c candidate) {
defer cwg.Done()
lf, ok := e.loadOneFontFace(ctx, doc, c.face)
attempts[j] = attempt{c.idx, lf, ok}
}(j, c)
}
cwg.Wait()
sort.Slice(attempts, func(a, b int) bool { return attempts[a].idx < attempts[b].idx })
for _, a := range attempts {
if a.ok {
results[i] = loaded{a.idx, a.lf}
found[i] = true
return
}
Expand Down Expand Up @@ -226,6 +277,24 @@ func RegisterFontFaces(fonts *paint.Fonts, faces []LoadedFontFace) int {
return n
}

// srcsKey identifies a FontFace by its own src list — two rules with the same
// URLs and format hints, in the same order, are the same fetch regardless of
// how many times the same rule was parsed (the same stylesheet named twice,
// or the same rule appearing in two separate sheets), so LoadFontFaces's
// per-slot dedup key is this, not the whole FontFace value.
func srcsKey(srcs []css.FontSrc) string {
var b strings.Builder
for i, s := range srcs {
if i > 0 {
b.WriteByte('\n')
}
b.WriteString(s.URL)
b.WriteByte('\x00')
b.WriteString(s.Format)
}
return b.String()
}

func boolKey(b bool) string {
if b {
return "i"
Expand Down
64 changes: 64 additions & 0 deletions fontfaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"net/http/httptest"
"os"
"sync"
"sync/atomic"
"testing"
"time"

Expand Down Expand Up @@ -153,6 +154,69 @@ func TestLoadFontFacesKeepsDistinctSlots(t *testing.T) {
}
}

// A real page can carry several @font-face rules sharing one (family,
// weight, italic) slot even though they are NOT fallback formats of the same
// file: this engine has no unicode-range model (css.FontFace carries no such
// field), so distinct rules a real browser would pick among by codepoint
// collapse into one slot here. Confirmed live (round 168) on cs.cmu.edu's
// Font Awesome v4-compatibility shim: five separate 'FontAwesome'-family,
// weight-400 rules (solid/brands/regular/compat, split only by
// unicode-range) landed in one slot, and the server's own /webfonts/ path
// was erroring — each failed attempt still waited out the full ~6s response,
// serially, costing ~52s of the page's render time via a chain of slow
// failures nothing in the page's own markup ever asked this engine to try
// one at a time.
func TestLoadFontFacesTriesSlotCandidatesConcurrentlyNotSequentially(t *testing.T) {
const delay = 150 * time.Millisecond
const slow = 5 // every candidate but the last is a slow failure
var hits int32
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
atomic.AddInt32(&hits, 1)
if r.URL.Path == "/good.ttf" {
w.Header().Set("Content-Type", "font/ttf")
_, _ = w.Write(cabin.TTF)
return
}
time.Sleep(delay)
w.WriteHeader(http.StatusInternalServerError)
}))
t.Cleanup(srv.Close)

var sheet string
for i := 0; i < slow; i++ {
sheet += `@font-face { font-family: F; font-weight: 400; src: url(` + srv.URL + `/bad` + itoa(i) + `.ttf); }` + "\n"
}
// Declared LAST: with the old sequential "stop at first success" loop,
// this is only tried after all `slow` failures above have each waited
// out their own delay in turn — `slow*delay` of pure waiting. Tried
// concurrently with its siblings, it finishes in about one `delay`.
sheet += `@font-face { font-family: F; font-weight: 400; src: url(` + srv.URL + `/good.ttf); }`

start := time.Now()
faces := New().LoadFontFaces(context.Background(), &Document{URL: srv.URL + "/p.html"}, []string{sheet}, css.Media{})
elapsed := time.Since(start)

if len(faces) != 1 {
t.Fatalf("got %d faces, want 1", len(faces))
}
// e.Client's own per-host concurrency cap (transport.go,
// maxPerHostConcurrency, 2 at the time of writing) still applies here —
// this test is about THIS function's own candidate loop, not about
// bypassing that separate, deliberately-tuned limit — so `slow+1`
// candidates batch through it ceil((slow+1)/cap) at a time (measured:
// ~3*delay), not all at once. Fully SEQUENTIAL waiting — one candidate
// completely finishing, including the one that fails, before the next
// even starts — takes `slow*delay` (measured: ~5*delay, the old code's
// own behavior, confirmed by stash-reverting this file while writing
// this test). The threshold sits roughly midway between the two.
if elapsed > delay*4 {
t.Errorf("elapsed = %v, want well under %v (the slow candidates should batch through the per-host cap concurrently, not queue one at a time)", elapsed, delay*4)
}
if got := atomic.LoadInt32(&hits); got != slow+1 {
t.Errorf("hits = %d, want %d (every distinct candidate fetched exactly once)", got, slow+1)
}
}

func TestItoaAndBoolKey(t *testing.T) {
for _, c := range []struct {
n int
Expand Down
Loading