Public: anyone can read this; the crew can change it.
A repository's README is rendered once per blob: the files page's readme and render under 15 ms warm #6165
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6148-a-repository-s-readme-is-rendered-once-p → main 3 commits fork 2 threads · 2 unresolved
3 files
changed +171
−1
src/web/markdown.ts +5
| @@ -110,6 +110,10 @@ const MEMO_MAX = 512; | ||
| 110 | 110 | const MEMO_MAX_SOURCE = 20_000; |
| 111 | 111 | const memo = new Map<string, Html>(); |
| 112 | 112 | const counts = { hits: 0, misses: 0 }; |
| 113 | let worked = 0; | |
| 114 | ||
| 115 | /** How many times the markdown was actually parsed, memo or no memo. */ | |
| 116 | export const markdownWorkDone = (): number => worked; | |
| 113 | 117 | |
| 114 | 118 | /** How often a render was remembered or made; for tests to assert the memo works. */ |
| 115 | 119 | export function markdownMemoStats(): { hits: number; misses: number } { |
| @@ -137,6 +141,7 @@ export function renderMarkdown(source: string): Html { | ||
| 137 | 141 | } |
| 138 | 142 | |
| 139 | 143 | function renderUncached(source: string): Html { |
| 144 | worked++; | |
| 140 | 145 | const text = source.replaceAll("\r\n", "\n"); |
| 141 | 146 | // Escaping does not move a newline, so the two arrays stay in step: `lines` |
| 142 | 147 | // is what prose is built from, `raws` is what the tokenizer needs. |
src/web/pages/repo.ts +56 −1
| @@ -425,6 +425,52 @@ export function directoryTotal(entries: readonly TreeEntry[]): Html { | ||
| 425 | 425 | </span>`; |
| 426 | 426 | } |
| 427 | 427 | |
| 428 | /** | |
| 429 | * Rendered README cards by blob oid. An oid names its bytes for good, so an | |
| 430 | * entry never goes stale; the cache is bounded by the bytes of source behind | |
| 431 | * the entries it holds, oldest out first. | |
| 432 | */ | |
| 433 | const README_CACHE_BYTES = 8_000_000; | |
| 434 | const readmeCache = new Map<string, { card: Html; bytes: number }>(); | |
| 435 | let readmeCacheBytes = 0; | |
| 436 | ||
| 437 | function remember(key: string, card: Html, bytes: number): void { | |
| 438 | if (bytes > README_CACHE_BYTES) return; | |
| 439 | const old = readmeCache.get(key); | |
| 440 | if (old) { | |
| 441 | readmeCacheBytes -= old.bytes; | |
| 442 | readmeCache.delete(key); | |
| 443 | } | |
| 444 | readmeCache.set(key, { card, bytes }); | |
| 445 | readmeCacheBytes += bytes; | |
| 446 | for (const [k, v] of readmeCache) { | |
| 447 | if (readmeCacheBytes <= README_CACHE_BYTES) break; | |
| 448 | readmeCache.delete(k); | |
| 449 | readmeCacheBytes -= v.bytes; | |
| 450 | } | |
| 451 | } | |
| 452 | ||
| 453 | /** | |
| 454 | * Whose bytes an entry holds: the repository being read, then the oid and | |
| 455 | * name. An origin's oid is whatever the remote host sent, so it is never | |
| 456 | * trusted alone, and one that is not a real object id is not cached at all. | |
| 457 | */ | |
| 458 | function readmeKey(source: RepoSource, entry: TreeEntry): string | null { | |
| 459 | if (!/^[0-9a-f]{40}([0-9a-f]{24})?$/.test(entry.oid)) return null; | |
| 460 | const w = source.where; | |
| 461 | const repo = | |
| 462 | w.kind === "local" | |
| 463 | ? `local:${w.repo.id}` | |
| 464 | : `origin:${w.origin.id}:${w.fullName}`; | |
| 465 | return `${repo}:${entry.oid}:${entry.name}`; | |
| 466 | } | |
| 467 | ||
| 468 | /** Empties the README cache; for tests. */ | |
| 469 | export function clearReadmeCache(): void { | |
| 470 | readmeCache.clear(); | |
| 471 | readmeCacheBytes = 0; | |
| 472 | } | |
| 473 | ||
| 428 | 474 | /** |
| 429 | 475 | * The README under a directory listing, read through whichever source is |
| 430 | 476 | * answering. It is found in the entries we already have rather than by asking |
| @@ -442,6 +488,13 @@ export async function readmeFor( | ||
| 442 | 488 | e.type === "blob" && /^readme(\.md|\.markdown|\.txt)?$/i.test(e.name), |
| 443 | 489 | ); |
| 444 | 490 | if (!readme) return html``; |
| 491 | const key = readmeKey(source, readme); | |
A hit skips source.blob. That is safe because the caller already read the tree through the same source, so access was checked and the key is scoped to the repo. Keep that order if the call site moves. |
||
| 492 | const cached = key === null ? undefined : readmeCache.get(key); | |
| 493 | if (key !== null && cached) { | |
| 494 | readmeCache.delete(key); | |
| 495 | readmeCache.set(key, cached); | |
| 496 | return cached.card; | |
| 497 | } | |
| 445 | 498 | const at = prefix === "" ? readme.name : `${prefix}/${readme.name}`; |
| 446 | 499 | const blob = await source.blob(ref, at); |
| 447 | 500 | if (!blob.ok || blob.value.kind !== "text") return html``; |
| @@ -450,11 +503,13 @@ export async function readmeFor( | ||
| 450 | 503 | const rendered = /\.(md|markdown)$/i.test(readme.name) |
| 451 | 504 | ? renderMarkdown(text) |
| 452 | 505 | : html`<pre>${text}</pre>`; |
| 453 | return card(html`<div class="prose">${rendered}</div>`, { | |
| 506 | const made = card(html`<div class="prose">${rendered}</div>`, { | |
| 454 | 507 | pad: false, |
| 455 | 508 | head: html`${icon("file")} |
| 456 | 509 | <h2>${readme.name}</h2>`, |
| 457 | 510 | }); |
| 511 | if (key !== null) remember(key, made, blob.value.size + key.length); | |
| 512 | return made; | |
| 458 | 513 | } |
| 459 | 514 | |
| 460 | 515 | /** |
test/readme-cache.test.ts added +110
| @@ -0,0 +1,110 @@ | ||
| 1 | /** A README is rendered once per blob oid: the second look does no markdown work. */ | |
| 2 | import { beforeEach, expect, test } from "bun:test"; | |
| 3 | import type { RepoSource, TreeEntry } from "../src/view/source.ts"; | |
| 4 | import { markdownWorkDone, renderMarkdown } from "../src/web/markdown.ts"; | |
| 5 | import { clearReadmeCache, readmeFor } from "../src/web/pages/repo.ts"; | |
| 6 | ||
| 7 | const OID_A = "a".repeat(40); | |
| 8 | const OID_B = "b".repeat(40); | |
| 9 | const encode = (s: string) => new TextEncoder().encode(s); | |
| 10 | // Over the markdown memo's own limit, so only the oid cache can save the work. | |
| 11 | const BIG = `# Title <script>alert(1)</script>\n\n${"para with *em* and `code`\n\n".repeat(1200)}`; | |
| 12 | ||
| 13 | let reads = 0; | |
| 14 | function sourceOf(text: string, where?: unknown): RepoSource { | |
| 15 | return { | |
| 16 | where: where ?? { kind: "local", repo: { id: "r1" } }, | |
| 17 | blob: async () => { | |
| 18 | reads++; | |
| 19 | return { | |
| 20 | ok: true, | |
| 21 | value: { kind: "text", body: encode(text), size: text.length }, | |
| 22 | }; | |
| 23 | }, | |
| 24 | } as unknown as RepoSource; | |
| 25 | } | |
| 26 | const entry = (oid: string): TreeEntry => ({ | |
| 27 | mode: "100644", | |
| 28 | type: "blob", | |
| 29 | oid, | |
| 30 | size: 1, | |
| 31 | name: "README.md", | |
| 32 | }); | |
| 33 | ||
| 34 | beforeEach(() => { | |
| 35 | clearReadmeCache(); | |
| 36 | reads = 0; | |
| 37 | }); | |
| 38 | ||
| 39 | test("the second render of one blob does no markdown work", async () => { | |
| 40 | expect(BIG.length).toBeGreaterThan(20_000); | |
| 41 | const src = sourceOf(BIG); | |
| 42 | const first = await readmeFor(src, "main", [entry(OID_A)], ""); | |
| 43 | const before = markdownWorkDone(); | |
| 44 | const second = await readmeFor(src, "main", [entry(OID_A)], ""); | |
| 45 | expect(markdownWorkDone()).toBe(before); | |
| 46 | expect(reads).toBe(1); | |
| 47 | expect(String(second)).toBe(String(first)); | |
| 48 | }); | |
| 49 | ||
| 50 | test("a changed README renders anew", async () => { | |
| 51 | await readmeFor(sourceOf(BIG), "main", [entry(OID_A)], ""); | |
| 52 | const before = markdownWorkDone(); | |
| 53 | const changed = await readmeFor( | |
| 54 | sourceOf(`${BIG}\nmore`), | |
| 55 | "main", | |
| 56 | [entry(OID_B)], | |
| 57 | "", | |
| 58 | ); | |
| 59 | expect(markdownWorkDone()).toBe(before + 1); | |
| 60 | expect(String(changed)).toContain("more"); | |
| 61 | }); | |
| 62 | ||
| 63 | test("cached output is byte-identical to the uncached render and stays escaped", async () => { | |
| 64 | const first = String( | |
| 65 | await readmeFor(sourceOf(BIG), "main", [entry(OID_A)], ""), | |
| 66 | ); | |
| 67 | const cached = String( | |
| 68 | await readmeFor(sourceOf(BIG), "main", [entry(OID_A)], ""), | |
| 69 | ); | |
| 70 | clearReadmeCache(); | |
| 71 | const fresh = String( | |
| 72 | await readmeFor(sourceOf(BIG), "main", [entry(OID_A)], ""), | |
| 73 | ); | |
| 74 | expect(cached).toBe(fresh); | |
| 75 | expect(first).toBe(fresh); | |
| 76 | expect(fresh).not.toContain("<script>"); | |
| 77 | expect(fresh).toContain("<script>"); | |
| 78 | expect(fresh).toContain(String(renderMarkdown(BIG)).slice(0, 200)); | |
| 79 | }); | |
| 80 | ||
| 81 | const origin = (id: string, fullName: string) => ({ | |
| 82 | kind: "origin", | |
| 83 | origin: { id }, | |
| 84 | fullName, | |
| 85 | }); | |
| 86 | ||
| 87 | test("two repositories reporting one oid each get their own card", async () => { | |
| 88 | const real = await readmeFor(sourceOf("# Real"), "main", [entry(OID_A)], ""); | |
| 89 | const hostile = await readmeFor( | |
| 90 | sourceOf("# Hostile", origin("o1", "evil/x")), | |
| 91 | "main", | |
| 92 | [entry(OID_A)], | |
| 93 | "", | |
| 94 | ); | |
| 95 | expect(String(hostile)).toContain("Hostile"); | |
| 96 | expect(String(hostile)).not.toContain("Real"); | |
| 97 | const again = await readmeFor(sourceOf("# Real"), "main", [entry(OID_A)], ""); | |
| 98 | expect(String(again)).toBe(String(real)); | |
| 99 | }); | |
| 100 | ||
| 101 | test("an oid that is not a real object id is never cached", async () => { | |
| 102 | for (const oid of ["", "main", "A".repeat(40)]) { | |
| 103 | clearReadmeCache(); | |
| 104 | reads = 0; | |
| 105 | await readmeFor(sourceOf("# One"), "main", [entry(oid)], ""); | |
| 106 | const two = await readmeFor(sourceOf("# Two"), "main", [entry(oid)], ""); | |
| 107 | expect(String(two)).toContain("Two"); | |
| 108 | expect(reads).toBe(2); | |
| 109 | } | |
| 110 | }); |
new above
offline
High: this key has no repository in it, and for origin-backed sources
readme.oidis whatever the remote host sent, possibly "". Two repos can then share an entry: one can poison another's README, or a private one can leak into a public page. Key on repo id + oid + name, and skip caching unless the oid matches /^[0-9a-f]{40}([0-9a-f]{24})?$/.