measure the render path, then remember pictures instead of caching pages
The entry said to measure first and put the number in the commit, so: a plain page renders in 14µs, a twelve-picture gallery in 1.23ms. Of that, ~102µs per picture was reading, hashing and decoding bytes the previous request had already read. Remembering that one fact — keyed by path, size and modification time — brings the same gallery to 63µs. 19.5× faster, 21× fewer bytes allocated, twenty-odd lines. After which nothing is slow enough to justify caching whole pages, so ADR-0044 declines the page cache and leaves the parked validity model parked, now with a measurement rather than an intuition behind its trigger. That parked model has five axes and was written before any code existed. The problem it would have been built for turned out to be one repeated file read. Benchmarks live in internal/web so they measure through the real handler, which is also what conventions.md wants before any cache goes in the render path. The invalidation risk has its own test: an edited picture is a different key, so the memo cannot serve yesterday's dimensions. Everything runs clean under -race, since the map is read by concurrent requests.
This commit is contained in:
@@ -16,6 +16,7 @@ import (
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"golang.org/x/image/draw"
|
||||
_ "golang.org/x/image/webp"
|
||||
@@ -131,6 +132,20 @@ func encode(w *os.File, img image.Image, source string) error {
|
||||
return jpeg.Encode(w, img, &jpeg.Options{Quality: 82})
|
||||
}
|
||||
|
||||
// inspected remembers what each picture is.
|
||||
//
|
||||
// Measured, not assumed: without this the same bytes were read, hashed and decoded on every request, costing
|
||||
// ~102µs per picture — a twelve-picture gallery spent 1.2ms of its 1.24ms doing work it had already done
|
||||
// (BenchmarkGalleryPage in internal/web). conventions.md allows a cache in the render path once a benchmark
|
||||
// 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{}
|
||||
)
|
||||
|
||||
// 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.
|
||||
//
|
||||
@@ -148,6 +163,18 @@ func picture(origin render.Origin, file string) (render.Picture, bool) {
|
||||
return p, true
|
||||
}
|
||||
name := path.Join(origin.Dir, file)
|
||||
info, err := fs.Stat(origin.Files, name)
|
||||
if err != nil {
|
||||
slog.Error("picture unreadable", "path", name, "err", err)
|
||||
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
|
||||
}
|
||||
data, err := fs.ReadFile(origin.Files, name)
|
||||
if err != nil {
|
||||
slog.Error("picture unreadable", "path", name, "err", err)
|
||||
@@ -171,6 +198,9 @@ 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()
|
||||
return p, true
|
||||
}
|
||||
|
||||
|
||||
@@ -12,6 +12,9 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
"testing/fstest"
|
||||
"time"
|
||||
|
||||
"khosra/internal/render"
|
||||
)
|
||||
|
||||
// wide builds a real encoded picture of a given width, so the tests exercise decoding rather than a stub.
|
||||
@@ -118,3 +121,32 @@ func TestOriginalsAreNeverTouched(t *testing.T) {
|
||||
t.Error("the author's own file was modified")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAnEditedPictureIsInspectedAgain(t *testing.T) {
|
||||
// The whole risk of remembering: serving yesterday's size or srcset. The key carries size and modification
|
||||
// time, so an edit is a different key rather than a stale hit.
|
||||
fsys := fstest.MapFS{
|
||||
"content/art/set/index.md": {Data: []byte("---\ntitle: Set\n---\n")},
|
||||
"content/art/set/one.jpg": {Data: wide(t, 1200, false), ModTime: time.Unix(1000, 0)},
|
||||
}
|
||||
origin := render.Origin{Dir: "content/art/set", Files: fsys}
|
||||
first, ok := picture(origin, "one.jpg")
|
||||
if !ok || first.Width != 1200 {
|
||||
t.Fatalf("first inspection = %+v", first)
|
||||
}
|
||||
if again, _ := picture(origin, "one.jpg"); again != first {
|
||||
t.Errorf("a second look at the same file should be identical, got %+v", again)
|
||||
}
|
||||
|
||||
fsys["content/art/set/one.jpg"] = &fstest.MapFile{Data: wide(t, 800, false), ModTime: time.Unix(2000, 0)}
|
||||
edited, ok := picture(origin, "one.jpg")
|
||||
if !ok {
|
||||
t.Fatal("still a picture")
|
||||
}
|
||||
if edited.Width != 800 {
|
||||
t.Errorf("width = %d, want 800 — the edit was not noticed", edited.Width)
|
||||
}
|
||||
if edited.Srcset == first.Srcset {
|
||||
t.Error("the srcset should name different derivatives after an edit")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user