aboutsummaryrefslogtreecommitdiff
path: root/internal/calendar/transfer_plan_cache_test.go
diff options
context:
space:
mode:
authorLukasz Kasprzak <lukas@labunix.xyz>2026-08-24 22:02:39 +0200
committerLukasz Kasprzak <lukas@labunix.xyz>2026-08-24 22:02:39 +0200
commitb24702cc9b8d7f2e408bb92032e8f7fa2aaa836b (patch)
tree52b6adb382445a0762f63fb862361d7fbd8637f2 /internal/calendar/transfer_plan_cache_test.go
parentea6e59862dbdb607b9bcb221212a85ee3e4bd84a (diff)
downloadlectio-perf/hoist-date-independent-work.tar.gz
lectio-perf/hoist-date-independent-work.zip
perf(calendar): precompute the EF occupancy index alongside the transfer planperf/hoist-date-independent-work
The prior commit (ea6e598) memoised efTransferPlan but left the SAME occupiedByClass1/Or2 O(len(merged)) scan pattern in a second place: transferIfImpededEF's own fallback path, called from computeEF once per day for every class-1 candidate the (now cached) plan does not resolve -- confirmed by CPU profile, not assumed: computeEF.func1 (the old occupiedByRank closure) was ~62% of BenchmarkDays7EF's total time, almost all of it inside buildCelebration, called from BOTH efTransferPlan internally AND this second, uncached site. Before optimising, established precisely what "occupied" depends on, per the coordinator's warning that occupancy might genuinely mutate during planning (efTransferPlan's own `claimed` map suggested as much). It does not: occupiedByRank asks only "does some OTHER entry's ORIGINAL, untransferred date (buildCelebration + celebrationDate, which never considers a transfer) equal d" -- a pure function of (merged content, year, sel) alone, identical to what the transfer-plan cache already keys on. `claimed`, by contrast, genuinely mutates during one planning pass (it tracks which TARGET days a transfer walk has already assigned) and is NOT part of occupiedByRank's computation at all -- it remains computed fresh inside efTransferPlan every call, untouched by this change. These are independent, not the same thing wearing two names: efTransferPlan's own forward-walk loop already checks both, plus a third condition (the target day's own temporal class), as separate disjuncts. Given that, an occupancy INDEX -- not a second cache, and not a "we already know the answer" shortcut derived from plan's absence (which would have been correct for the transferIfImpededEF fallback's own control flow ONLY by coincidence: it ignores the unconditional All Souls Sunday-transfer special case that runs before the class-1 check on ANY rank, so a shortcut skipping straight past it would misfire the moment a user overlay retagged All Souls class-1, however unlikely on shipped data) -- is the safe fix: buildEFOccupancyIndex does the same merged-scan ONCE, into date -> []{slug, rank}, and .occupied does the exact O(1)-ish lookup + tiny-list filter occupiedByRank always computed, just precomputed. transferIfImpededEF's own signature, control flow and All Souls handling are completely unchanged; only what its two closure parameters read from changed. The index shares the transfer-plan cache's existing key (year, Selection, SHA-256 of merged) rather than adding a new one -- both are pure in exactly those three inputs, built in the same pass, so one key correctly covers both. Only the plan is copied per call (clonePlan); the index, which can hold one entry per merged slug (~330 on shipped data), is returned uncopied and documented immutable-after-construction -- safe under Go's concurrent-read guarantee since nothing anywhere writes to a returned index. New TestEFOccupancyIndexDetectsSameDateClass1Collision covers a path no existing test reached: two class-1 SANCTORAL entries sharing one ORIGINAL date (St Joseph/the Annunciation, covered by the prior commit's tests, collide via HOLY WEEK's temporal precedence on DIFFERENT dates, never with each other). A synthetic overlay (Compute's own public API, same style as the existing tests) puts two class-1 entries on the same otherwise-ordinary date: alone, either is simply observed; both together, RG 97/98 transfer both forward and the shared date reverts to its temporal office. Mutation-proved: dropping the index's exceptSlug self-exclusion (reverted after) made the test fail immediately -- every class-1 entry saw itself in the index and wrongly self-impeded, even the single-entry case. TestEFTransferPlanCacheConcurrentUse gained a third, disjoint key exercising this same collision path from goroutines alongside the two existing ones; `go test -race` on the whole package is clean. Benchmarked (interleaved before/after, same method throughout this branch): BenchmarkDays7EF drops from ~46-48ms (the prior commit's own plan-cache-only state) to ~15-18ms/op (allocs 208639 -> 98727, -53%; bytes 16.1MB -> 4.2MB, -74%) -- roughly a further 3x, ~4.6x cumulative against the original ~74ms. BenchmarkDaysWeek (OF, which never touches any of this) is unaffected: ~4.9-5.9ms/op both before and after, with byte-for-byte identical allocs/bytes in every run -- the ms-level wobble is machine noise, not a regression. A fresh CPU profile confirms the new remaining bottleneck precisely: writeSortedFields/hashMergedForPlan (the cache key's own SHA-256 of merged, ~330 entries) is now ~35% of total time, because it still runs on EVERY day (7x/week) to know whether a call is a cache hit, even though the work it gates now mostly isn't. Not fixed here: hoisting the key computation itself up to mobile.Days's batch level (mirroring the first commit on this branch, b6ee8f0) would need Compute's public signature to accept a precomputed key, a bigger surface change than this task's scope, reported rather than taken unilaterally. Output identity re-verified: the same 492-case sweep (both forms, both UI languages, all four corpora, the leap day/Triduum/Requiem/season- boundary dates, and the three Joseph/Annunciation years) is byte-identical (SHA-256-equal) before and after -- the same SHA-256 as the prior commit's own sweep, confirming zero output drift across the whole chain. go test ./... and make ci (both build tags, oracle/differential suite included) are green; go test -race on the whole internal/calendar package is clean.
Diffstat (limited to 'internal/calendar/transfer_plan_cache_test.go')
-rw-r--r--internal/calendar/transfer_plan_cache_test.go154
1 files changed, 138 insertions, 16 deletions
diff --git a/internal/calendar/transfer_plan_cache_test.go b/internal/calendar/transfer_plan_cache_test.go
index 6b3b48b..1592644 100644
--- a/internal/calendar/transfer_plan_cache_test.go
+++ b/internal/calendar/transfer_plan_cache_test.go
@@ -150,37 +150,69 @@ func TestEFTransferPlanCacheConcurrentUse(t *testing.T) {
}}
baseLayers := []calendar.Layer{base}
overlaidLayers := []calendar.Layer{base, overlay}
+ // A third, disjoint key exercising the occupancy-index collision path
+ // (TestEFOccupancyIndexDetectsSameDateClass1Collision) concurrently too
+ // -- the plan alone does not touch efOccupancyIndex.occupied unless a
+ // candidate's own precedence doesn't already decide it, which the
+ // Joseph/Annunciation scenario above never triggers (see that test's own
+ // doc comment).
+ collideLayers := []calendar.Layer{base, {ID: "user", Cels: map[string]calendar.RawCelebration{
+ "zz-test-alpha": {Fields: map[string]string{"rank": "class-1", "date": "07-06", "name.en": "ZZ Test Alpha"}, Variants: map[string]map[string]string{}},
+ "zz-test-beta": {Fields: map[string]string{"rank": "class-1", "date": "07-06", "name.en": "ZZ Test Beta"}, Variants: map[string]map[string]string{}},
+ }}}
years := []int{2008, 2011, 2035, 2046}
var wg sync.WaitGroup
- for g := 0; g < 12; g++ {
+ for g := 0; g < 15; 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
+ switch g % 3 {
+ case 0:
+ 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, baseLayers).Observed.Slug
+ }
+ }
+ case 1:
+ 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, overlaidLayers).Observed.Slug
+ }
+ }
+ default:
+ for i := 0; i < 6; i++ {
+ for _, ymd := range []string{"2026-07-06", "2026-07-07", "2026-07-08"} {
+ d, err := time.Parse("2006-01-02", ymd)
+ if err != nil {
+ t.Errorf("bad date: %v", err)
+ return
+ }
+ _ = calendar.Compute(d.UTC(), sel, collideLayers).Observed.Slug
}
- _ = 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.
+ // After the concurrent hammering, correctness must still hold for all
+ // three keys -- the concurrency test is not a substitute for the
+ // correctness tests above, so re-assert their 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)
@@ -188,4 +220,94 @@ func TestEFTransferPlanCacheConcurrentUse(t *testing.T) {
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)
}
+ d2, _ := time.Parse("2006-01-02", "2026-07-06")
+ if got := calendar.Compute(d2.UTC(), sel, collideLayers).Observed.Slug; got != "ef-time-after-pentecost-6-monday" {
+ t.Errorf("after concurrent use, colliding 2026-07-06 = %q, want ef-time-after-pentecost-6-monday", got)
+ }
+}
+
+// TestEFOccupancyIndexDetectsSameDateClass1Collision covers a path the tests
+// above do not: efOccupancyIndex.occupied's use from computeEF's
+// transferIfImpededEF call site (the per-candidate fallback for a class-1
+// entry efTransferPlan judged NOT impeded by temporal precedence alone), and
+// efOccupancyIndex's own use inside efTransferPlan's initial impeded check
+// (occupiedByClass1(when, cel.Slug) -- "does some OTHER fixed-date class-1
+// SANCTORAL feast already sit on `when`", the doc comment's own example, RG
+// 97/98). Neither TestEFTwoTransfersDoNotCollide nor the overlay/year tests
+// above exercise it: St Joseph and the Annunciation are impeded by HOLY
+// WEEK'S OWN temporal precedence, on DIFFERENT original dates -- never by
+// colliding with each other's original date -- so no existing test had ever
+// driven two class-1 SANCTORAL entries onto the exact same calendar date.
+//
+// A synthetic overlay is used because no two class-1 feasts share a fixed
+// date in the shipped 1962 calendar (an ordinary Time-after-Pentecost
+// Monday, 6 July 2026, was checked empirically before writing this test:
+// alone, a single synthetic class-1 entry there is simply observed, since
+// nothing outranks or occupies it; the real collision case exists only by
+// construction).
+//
+// What is varied: whether a SECOND class-1 entry shares the first one's
+// date -- both entries otherwise identical (rank class-1, real content
+// unrelated to any liturgical rule under test). If efOccupancyIndex failed
+// to detect the collision (or wrongly matched exceptSlug against ITSELF,
+// the most dangerous failure shape -- see the mutation proof below), 6 July
+// would keep reporting the lone entry regardless.
+func TestEFOccupancyIndexDetectsSameDateClass1Collision(t *testing.T) {
+ sel := calendar.DefaultSelection()
+ sel.Form = "old"
+ base := caldata.Tridentine()
+
+ 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
+ }
+
+ alpha := calendar.RawCelebration{
+ Fields: map[string]string{"rank": "class-1", "date": "07-06", "name.en": "ZZ Test Alpha"},
+ Variants: map[string]map[string]string{},
+ }
+ beta := calendar.RawCelebration{
+ Fields: map[string]string{"rank": "class-1", "date": "07-06", "name.en": "ZZ Test Beta"},
+ Variants: map[string]map[string]string{},
+ }
+
+ soloLayers := []calendar.Layer{base, {ID: "user", Cels: map[string]calendar.RawCelebration{
+ "zz-test-alpha": alpha,
+ }}}
+ collideLayers := []calendar.Layer{base, {ID: "user", Cels: map[string]calendar.RawCelebration{
+ "zz-test-alpha": alpha,
+ "zz-test-beta": beta,
+ }}}
+
+ // Alone, alpha is simply observed on its own date: nothing occupies 6
+ // July, so efTransferPlan judges it unimpeded and computeEF's fallback
+ // (occ.occupied via transferIfImpededEF) must agree and leave it there.
+ if got := compute(soloLayers, "2026-07-06"); got != "zz-test-alpha" {
+ t.Fatalf("alpha alone, 2026-07-06 = %q, want zz-test-alpha", got)
+ }
+
+ // Add beta on the SAME date. Both now occupy each other's date, so BOTH
+ // are impeded (RG 97/98) and transfer forward in table/impeded-first
+ // order (alpha to 7 July, beta to 8 -- empirically confirmed before
+ // writing this test); 6 July itself reverts to the ordinary temporal
+ // office, since NEITHER candidate keeps its place.
+ if got := compute(collideLayers, "2026-07-06"); got != "ef-time-after-pentecost-6-monday" {
+ t.Errorf("alpha+beta colliding, 2026-07-06 = %q, want ef-time-after-pentecost-6-monday (a stale/broken occupancy index would still show zz-test-alpha)", got)
+ }
+ if got := compute(collideLayers, "2026-07-07"); got != "zz-test-alpha" {
+ t.Errorf("alpha+beta colliding, 2026-07-07 = %q, want zz-test-alpha (transferred here)", got)
+ }
+ if got := compute(collideLayers, "2026-07-08"); got != "zz-test-beta" {
+ t.Errorf("alpha+beta colliding, 2026-07-08 = %q, want zz-test-beta (transferred here)", got)
+ }
+
+ // And alpha alone (no beta) must be unaffected by having since computed
+ // the colliding scenario -- the two overlays must not cross-contaminate
+ // the shared cache.
+ if got := compute(soloLayers, "2026-07-06"); got != "zz-test-alpha" {
+ t.Errorf("alpha alone after collision query, 2026-07-06 = %q, want zz-test-alpha (unaffected)", got)
+ }
}