perf(fonts): resolve the override directory once, not per glyph - #628
Merged
benletchford merged 1 commit intoAug 15, 2026
Merged
Conversation
get_font_face runs on the per-glyph drawing path and every call consulted SYSTEMLESS_ORIGINAL_FONTS_DIR, which walks the environment and allocates, then took a mutex. A CPU profile of EV Override attributed several percent of the process to that, across the handful of lookups each character makes. Resolve on first use and reuse; when no overrides exist -- the usual case, since the variable is an opt-in debugging hook -- lookups now take neither the environment nor the lock. -5.88% process CPU (t=-4.54, faster in 14 of 16 paired runs on a fixed 400M-instruction workload). This changes a documented behaviour: the cache previously followed the environment on every lookup. refresh_font_overrides() restores that for embedders that set the variable after other code has queried metrics. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3pHH4jDhVuFn8ZeWwkKmA
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.
get_font_facesits on the per-glyph drawing path, and every call resolved theSYSTEMLESS_ORIGINAL_FONTS_DIRoverride directory by callingenv::var_os— which walks the process environment and allocates — and then took the override-cache mutex, several times per character drawn.This PR resolves the directory on the first lookup and reuses the result. When no overrides exist — the usual case, since the variable is an opt-in hook for locally generated font blobs — subsequent lookups touch neither the environment nor the mutex (two relaxed-ordering atomic loads). A new
refresh_font_overrides()re-arms resolution for callers that change the variable at runtime.The contract change
This changes a documented behavior, and whether that is acceptable is your call. The cache previously followed the environment on every lookup, so a harness or embedder could set
SYSTEMLESS_ORIGINAL_FONTS_DIRafter other code had already queried font metrics and the next lookup would pick it up. With this PR, such callers must callrefresh_font_overrides()after setting or clearing the variable; setting it before the first font access (the README's launch-time framing) behaves as before. We searched for affected consumers: the only user of the public fonts API we can find is this repository's ownfont_specimenbinary, which does not use the override variable, and a GitHub code search finds no external consumers. Alternatives if you prefer them: keep the per-call environment read and only skip the mutex when no overrides are loaded — we measured that variant and it recovers approximately nothing (below) — or gate the caching behind a feature.What we benchmarked
EV Override 1.0.1 driven headless by a scripted input replay keyed to retired instruction count: every run performs exactly the same 400,000,000 guest instructions (boot, shareware dialog, menus, pilot creation, gameplay) from a pristine save directory. Metric: child-process CPU seconds on that fixed work; arms are distinct binaries alternated within each repetition, compared as within-pair ratios so machine-load drift cancels. 16 pairs:
Two narrower variants measured on the same instrument were flat, which is what isolates the environment walk as the cost: hoisting the per-glyph face resolution to once per
draw_charmeasured +0.0% (t = +0.01), and skipping only the mutex while keeping the per-callenv::var_osmeasured +0.1% (t = +0.03).Scope: one application, one scripted route, one host. The workload includes the menu and dialog phases where text rendering is dense; in pure gameplay scenes the per-glyph path is much cooler, so the benefit is concentrated in text-heavy screens. The claim is CPU draw on fixed work, not frame rate.
Verification
rustfmt clean,
clippy --all-targets -D warningsclean, full library test suite passing on this head. The override lookup semantics are exercised by the existing font tests; behavior with the variable set at launch is unchanged.