bound the picture memo, evicting the least recently used

The memo held one entry per picture ever rendered, for the life of the process.
Correct for one author's laptop, wrong for what this engine is meant to be: a
server that runs for months and serves a whole site to many readers.

Least-recently-used rather than clearing when full, because a site has pages
nobody opens for months and a front page opened every minute — discarding
wholesale throws away exactly the entries about to be asked for again. A map
into a recency-ordered list: reads promote, evictions take the back, both
constant time.

1024 entries is a few hundred kilobytes. Generous enough that a normal site
never evicts, bounded enough that no site can grow the process without limit.
The number is a constant and not a setting, because a knob with one user is a
knob nobody asked for.

Eviction, replacement and the bound are tested, including under -race, since
requests are concurrent and the store is shared.

Latent item cleared. ext 1975/2000.
This commit is contained in:
Claude Opus 5
2026-08-01 23:50:53 +06:00
committed by bdeshi
parent 751ab9c06f
commit 6447a995d1
6 changed files with 185 additions and 30 deletions
+18
View File
@@ -1206,3 +1206,21 @@ largest test in the repo, and anything else wanting the shipped wiring must live
never claimed to be the shipped list, so there is nothing for it to drift from.
Revisit if: something outside `cmd` genuinely needs the composed renderer — which is the registry question,
and this decision deliberately does not answer it.
## ADR-0073 — The picture memo is bounded, least-recently-used
Date: 2026-08-01 · Status: accepted (bounds the cache ADR-0044 measured into existence)
Decision: `inspected` becomes a `memo` — a map into a recency-ordered list, holding at most 1024 pictures and
evicting the least recently used. Reads promote; both reads and evictions are constant time.
Why: the memo was unbounded, one entry per picture ever rendered, for the life of the process. Correct for
one author's laptop and wrong for the thing this engine is: a server that runs for months and serves a whole
site to many readers. Least-recently-used rather than clearing when full, because a site has pages nobody
opens for months and a front page opened every minute — discarding wholesale throws away exactly the entries
about to be asked for again. 1024 entries is a few hundred kilobytes, generous enough that a normal site
never evicts and bounded enough that no site can grow the process without limit.
Consequence: cheap — the benchmark that justified the cache (ADR-0044) is unaffected below the bound, and
above it the cost of a miss is the same ~102µs inspection the memo was built to avoid. Eviction and recency
are tested, and the store is exercised under `-race`, since requests are concurrent. Expensive — a list and a
map where there was one map, and the bound is a constant rather than a setting, so a site large enough to
thrash it has no knob. That is deliberate: a knob with one user is a knob nobody has asked for.
Revisit if: a real site evicts often enough to matter, which is a measurement and not a guess — and then the
question is the number, not the policy.
+1 -2
View File
@@ -25,7 +25,7 @@ table owns.
| `internal/render/view.go` | the theme contract in Go, and now actually all of it: `Page`, `List`, `Sequence`, `Extras`, `Item`, `Partial`, `Fragment` (with `Body`, `Headings` and `Lang` — ADR-0064, ADR-0065, ADR-0067), `Heading`, `Picture`, `Origin` |
| `internal/render/chrome.go` | the engine's own words: phrase table, month names, digits, and the `t`/`num`/`day` template funcs (ADR-0034), including the words a shortcode fragment supplies when the author gives none (ADR-0067) |
| `internal/render/templates/` | reference theme, complete (six icon names map to Unicode, no assets — ADR-0063): `base.html` (shell, navigation, language links, feed and OpenGraph), `page.html` (bundle, sequence, tags, extras), `list.html`, `extras.html`, `shortcodes/` — seven fragment files rather than one, and a site may use either form (ADR-0071) — with the `sizes` its own layout implies (ADR-0068), `theme.css` (ADR-0026, ADR-0049) |
| `internal/ext/shortcodes/` | first feature: `::name{key=value}` block parser and node renderer, rendering through a theme fragment (ADR-0036). `figure`, `gallery`, `include`, plus the derivative pass and remembered picture inspection (ADR-0042, ADR-0044). `FootnotePrefix` namespaces an included file's footnote ids (ADR-0058). Directive syntax since ADR-0059, plus `icons.go`: `:name:` inline, rendered by the theme's one `icon` fragment (ADR-0063), `containers.go`: `:::name{…}``:::` wrapping a rendered body (ADR-0064), and `toc.go`: the document's headings for a `::toc` call, to a depth the call may set (ADR-0065, ADR-0066). `Merge` splices includes before the parse for a bundle that asks for it (ADR-0066) |
| `internal/ext/shortcodes/` | first feature: `::name{key=value}` block parser and node renderer, rendering through a theme fragment (ADR-0036). `figure`, `gallery`, `include`, plus the derivative pass and remembered picture inspection, bounded and least-recently-used in `memo.go` (ADR-0042, ADR-0044, ADR-0073). `FootnotePrefix` namespaces an included file's footnote ids (ADR-0058). Directive syntax since ADR-0059, plus `icons.go`: `:name:` inline, rendered by the theme's one `icon` fragment (ADR-0063), `containers.go`: `:::name{…}``:::` wrapping a rendered body (ADR-0064), and `toc.go`: the document's headings for a `::toc` call, to a depth the call may set (ADR-0065, ADR-0066). `Merge` splices includes before the parse for a bundle that asks for it (ADR-0066) |
| `internal/ext/notation/` | the inline marks CommonMark lacks: `~sub~`, `^sup^`, `==mark==`, and `~~strike~~`, which it owns so a single tilde can mean subscript (ADR-0061). `abbr.go` adds `*[TERM]:` definitions and the pass that expands them (ADR-0062) |
| `internal/ext/scaffold/` | writes one draft directory bundle into a site root through `os.Root`: never an overwrite |
| `internal/ext/watch/` | polls `content/` and `templates/` on an interval it is given, ignores editor droppings, and reports a settled change (ADR-0022, ADR-0048, ADR-0056). `site.yaml` is deliberately not fingerprinted (ADR-0055) |
@@ -124,7 +124,6 @@ with a stated reason. A list nothing drains is a graveyard of known defects.
| The root listing's `<title>` repeats itself — "A Khosra Demo · A Khosra Demo" | Spotted 2026-08-01 by looking at the served page, not by any test: `base.html` joins page title and site title unconditionally, and at the root those are the same string. Cosmetic, and the fix is one `if` in a template — theme layer, not engine | The first time the reference theme is worked on (Phase G4 touches it), or sooner if a feed or OpenGraph title inherits the same doubling |
| The theme and the index are two separate `atomic.Pointer` stores, so a request landing between them sees a new theme with the previous index | Accepted 2026-08-01 with ADR-0056: both halves are internally coherent and the gap is microseconds, so no page is ever internally inconsistent — it is simply not a snapshot of the disk. Closing it means one pointer holding both, which changes `web.Handler`'s signature and 20 test construction sites | Anything that makes the gap observable — a request rate high enough to land in it, or a feature where content and theme must agree exactly (an export, where every page is generated in one pass) |
| Under the default include model, a fragment's footnotes render where the include sits, so a long one puts an `<hr>` and a numbered list mid-article | Spotted 2026-08-01 by looking at the served page, not by any test. It is ADR-0038's documented consequence, and the ids are correctly namespaced (ADR-0058); only the placement reads badly. **An author who minds now says `include: merge`** (ADR-0066), which makes the page one document and puts every note at its end, so this is a default rather than a limit | The default itself proving wrong — a site where every composed page sets the flag, at which point the flag is the wrong way round |
| The picture memo is never evicted — one entry per picture on the site, for the life of the process | Correct for one author's site, and the alternative is an eviction policy nothing needs. It is keyed on size and modification time, so it cannot go stale, only grow | A site root large enough that memory matters, or a long-running process where pictures churn |
## Open questions
+21 -14
View File
@@ -175,9 +175,9 @@ doc.go 8 · scaffold.go 94
- scaffold.go:76 func titleFrom(key string) string
- scaffold.go:85 func mkdirAll(root *os.Root, dir string) error
## internal/ext/shortcodes — 1034 lines + 691 test
## internal/ext/shortcodes — 1098 lines + 758 test
containers.go 173 · doc.go 7 · icons.go 129 · images.go 250 · shortcodes.go 380 · toc.go 95
containers.go 173 · doc.go 7 · icons.go 129 · images.go 241 · memo.go 73 · shortcodes.go 380 · toc.go 95
- containers.go:22 var containerKind = ast.NewNodeKind("ShortcodeContainer")
- containers.go:24 type container struct
@@ -207,18 +207,25 @@ containers.go 173 · doc.go 7 · icons.go 129 · images.go 250 · shortcodes.go
- icons.go:87 func iconName(line []byte) (string, int, bool)
- icons.go:117 func isNameByte(c byte, first bool) bool
- icons.go:129 func isWordRune(r rune) bool { return unicode.IsLetter(r) || unicode.IsDigit(r) }
- images.go:31 var widths = []int{480, 960, 1440}
- images.go:38 func Derive(siteFS fs.FS, cacheDir string) (int, error)
- images.go:76 func derive(data []byte, name, cacheDir string) (int, error)
- images.go:103 func scale(src image.Image, width int) image.Image
- images.go:115 func write(file string, img image.Image, source string) error
- images.go:133 func encode(w *os.File, img image.Image, source string) error
- images.go:149 var
- images.go:159 func picture(origin render.Origin, file string) (render.Picture, bool)
- images.go:214 func derivedName(data []byte, source string, width int) string
- images.go:224 func lossless(source string) bool
- images.go:234 func showable(name string) bool
- images.go:244 func derivable(name string) bool
- images.go:30 var widths = []int{480, 960, 1440}
- images.go:37 func Derive(siteFS fs.FS, cacheDir string) (int, error)
- images.go:75 func derive(data []byte, name, cacheDir string) (int, error)
- images.go:102 func scale(src image.Image, width int) image.Image
- images.go:114 func write(file string, img image.Image, source string) error
- images.go:132 func encode(w *os.File, img image.Image, source string) error
- images.go:148 var inspected = newMemo(remembered)
- images.go:155 func picture(origin render.Origin, file string) (render.Picture, bool)
- images.go:205 func derivedName(data []byte, source string, width int) string
- images.go:215 func lossless(source string) bool
- images.go:225 func showable(name string) bool
- images.go:235 func derivable(name string) bool
- memo.go:13 const remembered = 1024
- memo.go:21 type memo struct
- memo.go:30 type entry struct
- memo.go:35 func newMemo(max int) *memo
- memo.go:40 func (m *memo) get(key string) (render.Picture, bool)
- memo.go:52 func (m *memo) put(key string, picture render.Picture)
- memo.go:69 func (m *memo) len() int
- shortcodes.go:28 const
- shortcodes.go:37 func New(partial render.Partial) goldmark.Extender
- shortcodes.go:41 type extension struct
+5 -14
View File
@@ -16,7 +16,6 @@ import (
"path/filepath"
"strconv"
"strings"
"sync"
"golang.org/x/image/draw"
_ "golang.org/x/image/webp"
@@ -145,11 +144,8 @@ func encode(w *os.File, img image.Image, source string) error {
// asks for one, and this is the smallest thing the benchmark asks for.
//
// Keyed by name, size and modification time, so an edited picture is inspected again rather than remembered
// wrongly. A map behind a mutex because requests are concurrent and there is one entry per picture on the site.
var (
inspectedMu sync.Mutex
inspected = map[string]render.Picture{}
)
// wrongly, and bounded, so a long-running process serving a large site cannot grow without limit (ADR-0073).
var inspected = newMemo(remembered)
// Picture describes one image for a fragment: where to fetch it, what a browser may choose instead, and the
// intrinsic size, so a page reserves the right box before any bytes arrive.
@@ -174,11 +170,8 @@ func picture(origin render.Origin, file string) (render.Picture, bool) {
return p, true
}
key := fmt.Sprintf("%s\x00%d\x00%d", name, info.Size(), info.ModTime().UnixNano())
inspectedMu.Lock()
remembered, known := inspected[key]
inspectedMu.Unlock()
if known {
return remembered, true
if known, is := inspected.get(key); is {
return known, true
}
data, err := fs.ReadFile(origin.Files, name)
if err != nil {
@@ -203,9 +196,7 @@ func picture(origin render.Origin, file string) (render.Picture, bool) {
sources = append(sources, file+" "+strconv.Itoa(cfg.Width)+"w")
p.Srcset = strings.Join(sources, ", ")
}
inspectedMu.Lock()
inspected[key] = p
inspectedMu.Unlock()
inspected.put(key, p)
return p, true
}
+73
View File
@@ -0,0 +1,73 @@
package shortcodes
import (
"container/list"
"sync"
"khosra/internal/render"
)
// remembered is how many pictures are kept. An entry is two strings and two ints, so a thousand of them is a
// few hundred kilobytes — small enough to be generous, bounded enough that a long-running process serving a
// large site cannot grow without limit (ADR-0073).
const remembered = 1024
// memo is a bounded least-recently-used store of what each picture is.
//
// Least-recently-used rather than "drop everything when full", because a site has pages nobody reads for
// months and a front page read every minute: discarding wholesale would throw away exactly the entries that
// are about to be asked for again. The list holds entries in recency order, most recent at the front, and the
// map finds one without walking it — the two together are what make both reads and evictions constant time.
type memo struct {
mu sync.Mutex
max int
order *list.List
at map[string]*list.Element
}
// entry is what one list element holds. It carries its own key so an eviction can find the map entry to
// delete, which is the only reason the key appears twice.
type entry struct {
key string
picture render.Picture
}
func newMemo(max int) *memo {
return &memo{max: max, order: list.New(), at: make(map[string]*list.Element, max)}
}
// get returns what was remembered and marks it as most recently used.
func (m *memo) get(key string) (render.Picture, bool) {
m.mu.Lock()
defer m.mu.Unlock()
el, known := m.at[key]
if !known {
return render.Picture{}, false
}
m.order.MoveToFront(el)
return el.Value.(*entry).picture, true
}
// put remembers a picture, evicting the least recently used until the store is within bounds.
func (m *memo) put(key string, picture render.Picture) {
m.mu.Lock()
defer m.mu.Unlock()
if el, known := m.at[key]; known {
el.Value.(*entry).picture = picture
m.order.MoveToFront(el)
return
}
m.at[key] = m.order.PushFront(&entry{key: key, picture: picture})
for m.order.Len() > m.max {
oldest := m.order.Back()
m.order.Remove(oldest)
delete(m.at, oldest.Value.(*entry).key)
}
}
// len reports how many pictures are held. Only a test asks.
func (m *memo) len() int {
m.mu.Lock()
defer m.mu.Unlock()
return m.order.Len()
}
+67
View File
@@ -0,0 +1,67 @@
package shortcodes
import (
"strconv"
"sync"
"testing"
"khosra/internal/render"
)
func TestTheMemoEvictsTheLeastRecentlyUsed(t *testing.T) {
m := newMemo(3)
for _, k := range []string{"a", "b", "c"} {
m.put(k, render.Picture{Src: k})
}
// Touching "a" makes "b" the oldest, which is the whole difference between this and dropping the first
// thing that arrived.
if _, known := m.get("a"); !known {
t.Fatal("a should still be held")
}
m.put("d", render.Picture{Src: "d"})
if m.len() != 3 {
t.Errorf("the store should stay at its bound, got %d", m.len())
}
if _, known := m.get("b"); known {
t.Error("b was least recently used and should have gone")
}
for _, k := range []string{"a", "c", "d"} {
if _, known := m.get(k); !known {
t.Errorf("%s should still be held", k)
}
}
}
func TestPuttingTheSameKeyTwiceReplacesRatherThanGrows(t *testing.T) {
m := newMemo(2)
m.put("a", render.Picture{Src: "first"})
m.put("a", render.Picture{Src: "second"})
if m.len() != 1 {
t.Errorf("one key is one entry, got %d", m.len())
}
if got, _ := m.get("a"); got.Src != "second" {
t.Errorf("the later value should win, got %q", got.Src)
}
}
// Requests are concurrent, so the store is too. Run with -race, which the Makefile's test target does.
func TestTheMemoIsSafeUnderConcurrentUse(t *testing.T) {
m := newMemo(64)
var wg sync.WaitGroup
for i := 0; i < 8; i++ {
wg.Add(1)
go func(n int) {
defer wg.Done()
for j := 0; j < 200; j++ {
k := strconv.Itoa((n*j)%128) + "-k"
m.put(k, render.Picture{Src: k})
m.get(k)
}
}(i)
}
wg.Wait()
if m.len() > 64 {
t.Errorf("the bound must hold under concurrency, got %d", m.len())
}
}