diff options
| author | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-08-24 21:42:48 +0200 |
|---|---|---|
| committer | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-08-24 21:42:48 +0200 |
| commit | ea6e59862dbdb607b9bcb221212a85ee3e4bd84a (patch) | |
| tree | 6274d5bfd46663e3e80b6ef04564d57f5e71f646 /internal/calendar/transfer_plan_cache_test.go | |
| parent | b6ee8f09ad2c0bc9e83394e2427de861eb1e2a72 (diff) | |
| download | lectio-ea6e59862dbdb607b9bcb221212a85ee3e4bd84a.tar.gz lectio-ea6e59862dbdb607b9bcb221212a85ee3e4bd84a.zip | |
perf(calendar): memoise the EF transfer plan across a shared calendar
computeEF calls efTransferPlan once per day, but the plan is pure in
(year, the merged sanctoral content, Selection) and identical for every
day sharing those three -- e.g. every date in one mobile.Days week.
Rebuilding it per day was a real cost: efTransferPlan walks every
merged entry looking for class-1 candidates, and for each one it
considers, occupiedByClass1/Or2 (an occurrence check it also uses)
walks merged AGAIN -- confirmed by CPU profile, not just by reading the
code (github.com/lukaszkasprzak/lectio/internal/calendar.computeEF.func1,
the occupiedByRank closure, at ~62% of BenchmarkDays7EF's total time
before this fix, almost all of it inside buildCelebration).
efTransferPlanCached (transfer_plan_cache.go) wraps efTransferPlan with
a small, bounded, thread-safe LRU (container/list + sync.Mutex, capped
at 64 entries -- gomobile may call in from multiple goroutines, and an
unbounded map keyed by year would grow as a user scrolls through
decades). The cache key is (year, Selection, a SHA-256 of merged's full
content): merged is a map, so it cannot be a map key field itself, and
Go's randomised map iteration order means two calls with identical
content can visit it differently, so the hash sorts slugs and, within
each entry, its Fields/Variant keys before hashing, and covers every
field of every entry -- not just Rank/Date, the ones efTransferPlan's
own read path happens to touch today, because which entries even
qualify as class-1 is itself computed from that data, and
occupiedByClass1/Or2 scan ALL of merged, not just the class-1 subset.
Selection is included even though EF date resolution ignores it today
(resolveDate never reads its sel parameter) -- keying on it costs
nothing (four small strings) and protects a future change from
silently poisoning a cache that never accounted for it. The returned
map is always a fresh copy (clonePlan), never the cached instance, so
sharing it across goroutines needs no further synchronisation.
Verified the key is complete rather than trusted: with the content hash
temporarily dropped from the key (mutation test, not committed),
TestEFTransferPlanCacheInvalidatesOnOverlay failed immediately -- a
plan warmed for the shipped 2008 calendar was wrongly served back for
the same year with a user overlay applied (the Annunciation suppressed,
which changes where the RG 96(a)/97/98 collision sends St Joseph: 31
March instead of 1 April, empirically confirmed against the pre-cache
code before the test was written). TestEFTransferPlanCacheInvalidatesOnYear
is a lighter companion covering the year field. TestEFTransferPlanCacheConcurrentUse
hammers the cache from 12 goroutines across two different keys and
reasserts correctness afterward; clean under `go test -race`.
Benchmarked (interleaved before/after, same method as the readings.Prepare
commit, to control for machine thermal drift): BenchmarkDays7EF drops
from ~48-51ms to ~32-35ms/op (allocs 253810 -> 208639, -18%; bytes
28.1MB -> 16.1MB, -43%), roughly a third faster. BenchmarkDaysWeek (OF,
which never calls efTransferPlan at all) is unaffected, ~3.5-4.1ms/op
both before and after -- within noise, confirming this change is
EF-only as intended.
EF remains well outside OF's range (~32ms vs ~4ms), and a fresh CPU
profile after this fix places the dominant remaining cost precisely: it
is the SAME occupiedByClass1/Or2 pattern, but living OUTSIDE
efTransferPlan -- transferIfImpededEF's own fallback path, called once
per day for every class-1 candidate NOT already resolved by the (now
cached) plan, i.e. the ordinarily-unimpeded ones (~15-20 of them),
each triggering another O(len(merged)) scan. That call site was not
part of what this task named, and memoising it is a materially
different change (it is keyed per-candidate, not once per day), so it
is reported here rather than folded into this commit.
Output identity re-verified: a 492-case sweep of mobile.Day/mobile.Days
(both forms, both UI languages, all four corpora, a leap day, the
Sacred Triduum, a Requiem day, Christmas/Pentecost/Assumption/All Souls
windows, and the three Joseph/Annunciation transfer-collision years
this fix specifically touches -- 2008, 2035, 2046) produced
byte-identical (SHA-256-equal) JSON before and after. go test ./... and
make ci (both build tags, oracle/differential suite included) are
green.
Diffstat (limited to 'internal/calendar/transfer_plan_cache_test.go')
| -rw-r--r-- | internal/calendar/transfer_plan_cache_test.go | 191 |
1 files changed, 191 insertions, 0 deletions
diff --git a/internal/calendar/transfer_plan_cache_test.go b/internal/calendar/transfer_plan_cache_test.go new file mode 100644 index 0000000..6b3b48b --- /dev/null +++ b/internal/calendar/transfer_plan_cache_test.go @@ -0,0 +1,191 @@ +package calendar_test + +// Tests for the memoised EF transfer plan (transfer_plan_cache.go). These +// live in the external test package (calendar_test), exercising the real +// tridentine sanctoral data through the public Compute entry point, the same +// style precedence_ef_repro_test.go already uses -- the risk this cache +// introduces is entirely about whether the CACHE KEY captures every real +// input, which can only be demonstrated by varying an input through Compute +// and checking the observed office actually changes, not by testing the +// cache's internals in isolation. + +import ( + "sync" + "testing" + "time" + + "github.com/lukaszkasprzak/lectio/internal/caldata" + "github.com/lukaszkasprzak/lectio/internal/calendar" +) + +// TestEFTransferPlanCacheInvalidatesOnOverlay is THE bad-key test: it warms +// the memoised plan for (2008, EF, the shipped calendar) across a whole +// week -- mobile.Days's own access pattern -- then asks the SAME year +// through a layer stack with ONE user overlay added (suppressing the +// Annunciation) and asserts the observed office on 31 March and 1 April +// CHANGES accordingly. +// +// What is varied: the layer stack / merged sanctoral content (a user +// overlay), holding year and Selection fixed. This is deliberately not a +// synthetic scenario: with the Annunciation present, its RG 96(a) proper +// seat (the Monday after Low Sunday, RG 97/98) forces St Joseph -- also +// impeded that year -- to walk one day further, to 1 April +// (TestEFTwoTransfersDoNotCollide already pins this). Suppress the +// Annunciation and it no longer claims that Monday, so Joseph lands ON it, +// 31 March, and 1 April reverts to an ordinary feria. Verified empirically +// against the pre-cache code before this test was written (both outcomes +// reproduced exactly as asserted below). +// +// A cache keyed on year alone -- or on year+sel without the merged content +// -- would serve 2008's BASE-CALENDAR plan back for the overlaid query too, +// since both share every other key component; this test fails loudly if +// that happens (it would see 1 April still reporting Joseph, and 31 March +// still reporting the Annunciation, from the first, unrelated warm-up). +func TestEFTransferPlanCacheInvalidatesOnOverlay(t *testing.T) { + sel := calendar.DefaultSelection() + sel.Form = "old" + base := caldata.Tridentine() + baseLayers := []calendar.Layer{base} + + compute := func(layers []calendar.Layer, date string) string { + d, err := time.Parse("2006-01-02", date) + if err != nil { + t.Fatalf("bad test date %q: %v", date, err) + } + return calendar.Compute(d.UTC(), sel, layers).Observed.Slug + } + + // Warm the cache for (2008, sel, base-only-hash) across a whole week, + // exactly like mobile.Days resolving seven consecutive dates against the + // same Prepared layer stack. + for _, date := range []string{ + "2008-03-26", "2008-03-27", "2008-03-28", "2008-03-29", + "2008-03-30", "2008-03-31", "2008-04-01", "2008-04-02", + } { + compute(baseLayers, date) + } + if got := compute(baseLayers, "2008-03-31"); got != "annunciation-of-the-blessed-virgin-mary" { + t.Fatalf("baseline 2008-03-31 = %q, want annunciation-of-the-blessed-virgin-mary", got) + } + if got := compute(baseLayers, "2008-04-01"); got != "joseph-spouse-of-the-bl-virgin-mary" { + t.Fatalf("baseline 2008-04-01 = %q, want joseph-spouse-of-the-bl-virgin-mary", got) + } + + // Same year, same Selection, ONE overlay layer added: suppress the + // Annunciation. If the cache key omitted the sanctoral content, these + // two calls would silently return the baseline plan warmed above. + overlay := calendar.Layer{ID: "user", Cels: map[string]calendar.RawCelebration{ + "annunciation-of-the-blessed-virgin-mary": { + Fields: map[string]string{"suppress": "true"}, + Variants: map[string]map[string]string{}, + }, + }} + overlaid := []calendar.Layer{base, overlay} + + if got := compute(overlaid, "2008-03-31"); got != "joseph-spouse-of-the-bl-virgin-mary" { + t.Errorf("overlaid 2008-03-31 = %q, want joseph-spouse-of-the-bl-virgin-mary (Joseph now lands here, the Annunciation no longer claims it)", got) + } + if got := compute(overlaid, "2008-04-01"); got == "joseph-spouse-of-the-bl-virgin-mary" { + t.Errorf("overlaid 2008-04-01 = %q, want NOT joseph (a stale cache hit from the base-calendar warm-up)", got) + } + + // And the base calendar's own answer must be unaffected by having since + // computed the overlaid one -- the two keys must not collide either way. + if got := compute(baseLayers, "2008-03-31"); got != "annunciation-of-the-blessed-virgin-mary" { + t.Errorf("base calendar 2008-03-31 after overlaid query = %q, want annunciation-of-the-blessed-virgin-mary (unaffected)", got) + } + if got := compute(baseLayers, "2008-04-01"); got != "joseph-spouse-of-the-bl-virgin-mary" { + t.Errorf("base calendar 2008-04-01 after overlaid query = %q, want joseph-spouse-of-the-bl-virgin-mary (unaffected)", got) + } +} + +// TestEFTransferPlanCacheInvalidatesOnYear is a lighter companion: the same +// week-then-query pattern, varying the YEAR instead of the overlay (2008 vs +// 2035, both real Joseph/Annunciation collision years -- see +// TestEFTwoTransfersDoNotCollide -- but with different transfer targets). +// year is an explicit field of the cache key already, so this mainly guards +// against a key struct refactor accidentally dropping it; the overlay test +// above is the one guarding the field that is easy to omit by mistake. +func TestEFTransferPlanCacheInvalidatesOnYear(t *testing.T) { + sel := calendar.DefaultSelection() + sel.Form = "old" + layers := []calendar.Layer{caldata.Tridentine()} + + compute := func(date string) string { + d, err := time.Parse("2006-01-02", date) + if err != nil { + t.Fatalf("bad test date %q: %v", date, err) + } + return calendar.Compute(d.UTC(), sel, layers).Observed.Slug + } + + if got := compute("2008-04-01"); got != "joseph-spouse-of-the-bl-virgin-mary" { + t.Fatalf("2008-04-01 = %q, want joseph-spouse-of-the-bl-virgin-mary", got) + } + // 2035's Joseph lands on 2035-04-03, not 04-01 (a later Easter shifts the + // whole window). If the cache ignored the year, this would wrongly + // return 2008's plan. + if got := compute("2035-04-01"); got == "joseph-spouse-of-the-bl-virgin-mary" { + t.Errorf("2035-04-01 = %q, want NOT joseph (that is 2008's landing day, not 2035's)", got) + } + if got := compute("2035-04-03"); got != "joseph-spouse-of-the-bl-virgin-mary" { + t.Errorf("2035-04-03 = %q, want joseph-spouse-of-the-bl-virgin-mary", got) + } +} + +// TestEFTransferPlanCacheConcurrentUse exercises the memoised plan from many +// goroutines at once -- gomobile may call Day/Days in from multiple threads, +// and none of the tests above (all sequential) can catch a data race on the +// shared cache map/list. Run with -race; it is the actual proof of the +// "thread-safe" claim, not merely built with a mutex and assumed correct. +func TestEFTransferPlanCacheConcurrentUse(t *testing.T) { + sel := calendar.DefaultSelection() + sel.Form = "old" + base := caldata.Tridentine() + overlay := calendar.Layer{ID: "user", Cels: map[string]calendar.RawCelebration{ + "annunciation-of-the-blessed-virgin-mary": { + Fields: map[string]string{"suppress": "true"}, + Variants: map[string]map[string]string{}, + }, + }} + baseLayers := []calendar.Layer{base} + overlaidLayers := []calendar.Layer{base, overlay} + + years := []int{2008, 2011, 2035, 2046} + + var wg sync.WaitGroup + for g := 0; g < 12; g++ { + g := g + wg.Add(1) + go func() { + defer wg.Done() + layers := baseLayers + if g%2 == 0 { + layers = overlaidLayers // different goroutines hammer different cache keys + } + for i := 0; i < 6; i++ { + y := years[(g+i)%len(years)] + for _, md := range []string{"03-19", "03-31", "04-01"} { + d, err := time.Parse("2006-01-02", time.Date(y, 1, 1, 0, 0, 0, 0, time.UTC).Format("2006")+"-"+md) + if err != nil { + t.Errorf("bad date: %v", err) + return + } + _ = calendar.Compute(d.UTC(), sel, layers).Observed.Slug + } + } + }() + } + wg.Wait() + + // After the concurrent hammering, correctness must still hold for both + // keys -- the concurrency test is not a substitute for the correctness + // test above, so re-assert both outcomes here too. + d, _ := time.Parse("2006-01-02", "2008-04-01") + if got := calendar.Compute(d.UTC(), sel, baseLayers).Observed.Slug; got != "joseph-spouse-of-the-bl-virgin-mary" { + t.Errorf("after concurrent use, base 2008-04-01 = %q, want joseph-spouse-of-the-bl-virgin-mary", got) + } + if got := calendar.Compute(d.UTC(), sel, overlaidLayers).Observed.Slug; got == "joseph-spouse-of-the-bl-virgin-mary" { + t.Errorf("after concurrent use, overlaid 2008-04-01 = %q, want NOT joseph", got) + } +} |
