diff options
| author | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-08-24 22:02:39 +0200 |
|---|---|---|
| committer | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-08-24 22:02:39 +0200 |
| commit | b24702cc9b8d7f2e408bb92032e8f7fa2aaa836b (patch) | |
| tree | 52b6adb382445a0762f63fb862361d7fbd8637f2 /internal/calendar/calendar.go | |
| parent | ea6e59862dbdb607b9bcb221212a85ee3e4bd84a (diff) | |
| download | lectio-b24702cc9b8d7f2e408bb92032e8f7fa2aaa836b.tar.gz lectio-b24702cc9b8d7f2e408bb92032e8f7fa2aaa836b.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/calendar.go')
| -rw-r--r-- | internal/calendar/calendar.go | 79 |
1 files changed, 38 insertions, 41 deletions
diff --git a/internal/calendar/calendar.go b/internal/calendar/calendar.go index 00048b7..a033c81 100644 --- a/internal/calendar/calendar.go +++ b/internal/calendar/calendar.go @@ -36,45 +36,22 @@ func computeEF(date time.Time, sel Selection, layers []Layer) LiturgicalDay { td := temporalEF(date) merged := mergeLayers(layers) year := date.Year() - // occupiedByRank reports whether some OTHER fixed-date sanctoral - // celebration whose rank passes `allowed` resolves onto d this year. - // transferIfImpededEF uses this at two different thresholds: class 1 - // only, to decide whether a candidate is impeded in the first place (a - // class-2 occupant never impedes a class-1 feast -- class 1 always beats - // class 2 outright, no tie exists); and class 1 OR 2, for RG 96's "next - // day that is not I or II class" once a transfer is already under way - // (e.g. the Visitation, 2 July, blocking the Precious Blood's transfer - // off 1 July in 2011). - occupiedByRank := func(d time.Time, exceptSlug string, allowed func(Rank) bool) bool { - for slug2, rc2 := range merged { - if slug2 == exceptSlug { - continue - } - cel2 := buildCelebration(slug2, rc2) - if !allowed(cel2.Rank) { - continue - } - if when2, ok := celebrationDate(cel2, year, sel); ok && sameDay(when2, d) { - return true - } - } - return false - } - isClass1 := func(r Rank) bool { return r == RankClass1 } - isClass1Or2 := func(r Rank) bool { return r == RankClass1 || r == RankClass2 } - occ1 := func(d time.Time, except string) bool { return occupiedByRank(d, except, isClass1) } - occ1Or2 := func(d time.Time, except string) bool { return occupiedByRank(d, except, isClass1Or2) } // RG 97/98: the year's impeded I-class transfers are resolved as a set, // not one at a time, so two feasts impeded by the same early Easter cannot // both claim the same free day and lose one of themselves. // - // efTransferPlan is pure in (year, merged content, sel) and identical for - // every day of the year it is asked about -- computeEF runs once PER DAY, - // so a multi-day view (mobile.Days's week, a month view) was rebuilding - // it from scratch on every single one. efTransferPlanCached memoises it; - // see transfer_plan_cache.go for the cache key and why each of its three - // parts is load-bearing. - plan := efTransferPlanCached(year, merged, sel, occ1, occ1Or2) + // Both the plan and the occupancy index it is built from are pure in + // (year, merged content, sel) and identical for every day of the year + // they are asked about -- computeEF runs once PER DAY, so a multi-day + // view (mobile.Days's week, a month view) was rebuilding both from + // scratch on every single one. efTransferPlanCached memoises them + // together; see transfer_plan_cache.go for the cache key, why each of + // its three parts is load-bearing, and what the occupancy index is an + // index OF (every merged entry's own ORIGINAL, untransferred date -- + // never a transfer TARGET, which is decided during planning and tracked + // separately, only within one planning pass, by efTransferPlan's own + // `claimed`). + plan, occ := efTransferPlanCached(year, merged, sel) cands := []candidate{{Cel: td.Cel, Temporal: true, Season: td.Season, Sunday: td.Sunday}} for slug, rc := range merged { cel := buildCelebration(slug, rc) @@ -90,9 +67,22 @@ func computeEF(date time.Time, sel Selection, layers []Layer) LiturgicalDay { } effective, planned := plan[cel.Slug] if !planned { + // occ.occupied answers exactly what computeEF's own former + // occupiedByRank closure did -- "does some OTHER fixed-date + // sanctoral celebration whose rank passes `allowed` resolve onto + // d this year" -- at the same two thresholds transferIfImpededEF + // has always used: class 1 only, to decide whether a candidate is + // impeded in the first place (a class-2 occupant never impedes a + // class-1 feast -- class 1 always beats class 2 outright, no + // tie-break is even reached); and class 1 OR 2, for RG 96's "next + // day that is not I or II class" once a transfer is already under + // way (e.g. the Visitation, 2 July, blocking the Precious + // Blood's transfer off 1 July in 2011). It now reads a + // precomputed index instead of scanning merged afresh -- see + // transfer_plan_cache.go. effective = transferIfImpededEF(cel, when, - func(d time.Time) bool { return occ1(d, cel.Slug) }, - func(d time.Time) bool { return occ1Or2(d, cel.Slug) }) + func(d time.Time) bool { return occ.occupied(d, cel.Slug, isClass1Rank) }, + func(d time.Time) bool { return occ.occupied(d, cel.Slug, isClass1Or2Rank) }) } if sameDay(effective, date) { cands = append(cands, candidate{Cel: cel, Temporal: false, Season: td.Season}) @@ -248,6 +238,14 @@ func transferIfImpeded(cel Celebration, when time.Time, sel Selection) time.Time return day } +// isClass1Rank and isClass1Or2Rank are the two occupancy thresholds +// efTransferPlan and transferIfImpededEF (via computeEF's call site) test +// against efOccupancyIndex -- see transfer_plan_cache.go's doc comment on +// efOccupancyIndex for what "occupied" means and why it is safe to +// precompute once per (year, merged content, sel). +func isClass1Rank(r Rank) bool { return r == RankClass1 } +func isClass1Or2Rank(r Rank) bool { return r == RankClass1 || r == RankClass2 } + // efTransferPlan resolves ALL of a year's impeded I-class transfers together, // which RG 97/98 require and which resolving them one at a time cannot do. // @@ -268,8 +266,7 @@ func transferIfImpeded(cel Celebration, when time.Time, sel Selection) time.Time // takes its proper seat on 2 April; St Joseph, impeded on the 19th, walks past // Holy Week, the Easter octave and that claimed Monday to 3 April. Before this, // St Joseph was observed on no day of 2008, 2035 or 2046 at all. -func efTransferPlan(year int, merged map[string]RawCelebration, sel Selection, - occupiedByClass1, occupiedByClass1Or2 func(time.Time, string) bool) map[string]time.Time { +func efTransferPlan(year int, merged map[string]RawCelebration, sel Selection, occ efOccupancyIndex) map[string]time.Time { type pending struct { slug string @@ -297,7 +294,7 @@ func efTransferPlan(year int, merged map[string]RawCelebration, sel Selection, st := temporalEF(when) tCand := candidate{Cel: st.Cel, Temporal: true, Season: st.Season, Sunday: st.Sunday} sCand := candidate{Cel: cel, Temporal: false} - if precedenceEF(tCand) >= precedenceEF(sCand) && !occupiedByClass1(when, cel.Slug) { + if precedenceEF(tCand) >= precedenceEF(sCand) && !occ.occupied(when, cel.Slug, isClass1Rank) { continue // not impeded; stays put } // RG 96(a): a proper seat, claimed before anything queues. Guarded to @@ -328,7 +325,7 @@ func efTransferPlan(year int, merged map[string]RawCelebration, sel Selection, for i := 0; i < 60; i++ { b := temporalEF(day) isHighClass := b.Cel.Rank == RankClass1 || b.Cel.Rank == RankClass2 - if isHighClass || occupiedByClass1Or2(day, p.cel.Slug) || claimed[day.Format("2006-01-02")] { + if isHighClass || occ.occupied(day, p.cel.Slug, isClass1Or2Rank) || claimed[day.Format("2006-01-02")] { day = day.AddDate(0, 0, 1) continue } |
