Issue #3616341: Read a session once per hold instead of once per line, and register what the paths cost
Holding a party asked the same questions once per line. Both are one question about the session, so they are asked once per evaluation pass now, whatever the party.
What changed
The engine (BookingManager). The slot-wide consumption sum is memoized per session, and the memo is dropped at both edges of the locked section: when the slot locks are taken, and when the hold ends however it ends. Under the locks the read is taken once and then kept current in memory as each line is saved, which is what lets the group see its own effect without going back to the table.
A seam for constraints that read per line (GroupPrimedConstraintInterface). prime() above the locks and again under them, tally() as each line lands, forget() at both edges. A constraint keeps no notion of having been primed: its memos are dropped at the boundary either way, so a caller that is not the engine is as correct as one that is.
The locked-section read is the session's availability, not a batch of counts. PlaceBookable keeps nothing of its own and asks PlaceAvailability which places the session has taken. availableCandidates() now composes the hall against that same read instead of an anti-join, so the hall is read once per venue for the whole request rather than once per session, and the orphaned_places policy from #3616316 answers from a read PlaceBookable has already taken instead of adding one of its own. The constraints are primed before the group policies under the locks, which is what makes that sharing happen.
Measured
SQLite kernel, an identical warm-up hold first, before and after the same commit.
An unplaced session, HoldCostTest:
| party | before | after | of which the slot-wide read |
|---|---|---|---|
| 1 | 10 | 10 | 2 |
| 2 | 15 | 13 | 4 → 2 |
| 6 | 35 | 25 | 12 → 2 |
| 12 | 65 | 43 | 24 → 2 |
A 24-place single-row hall with one prior booking, PlaceHoldCostTest:
| party | before | after | of which read the places |
|---|---|---|---|
| 1 | 13 | 13 | 5 |
| 2 | 21 | 16 | 9 → 5 |
| 6 | 53 | 28 | 25 → 5 |
A party of one is unchanged in both, which is the point: there was never anything to share.
Tests
HoldCostTest's per-line ceiling is gone. A ceiling written from what the code already did is a description of the code: the path asked the same question once per line, the budget was set from that, and no party however large could exceed it. It is replaced by a comparison of a party of one against parties of two, six and twelve, asserting that the slot-wide read does not grow with the party. PlaceHoldCostTest does the same for the read naming the places. Both were seen to fail against the unfixed code, with exactly the count that grew.
Two refusals are asserted beside the measurements, because reading once for a whole group is only worth anything if the group is still refused where it must be: a group naming one place twice, and a party asking for a place another order holds.
AvailablePlaceReadCountTest now states the promise the new shape makes: one read of the hall per venue for a whole request, and one read of each session per evaluation pass, asserted apart because they scale differently.
Registering what the paths cost
The cost tests asserted a bound and threw the number away. A bound says whether something got worse than we allowed; a figure says what it costs, and only a figure can be compared with the same figure next month.
RecordsQueryCounts lets a cost test hand its figure out. Set YOYAKU_PERF_LOG to a path and every measurement lands in it; nothing is written when the variable is unset, which is every ordinary run and every CI job. Nine cost tests across the core module, placement, payment and the orchestra bridge use it, and the new docs/performance.md holds their figures with the command that refreshes them.
Keeping the memo honest outside a hold
Memoizing what a session has taken introduced a way to be wrong, and it is worth naming rather than burying.
That a released place is immediately free to the next reader was never anybody's decision. It was a property of the read running afresh every time it was asked for, so nothing named it, nothing tested it, and nothing warned that it was load bearing. Memoizing the read removed it silently, which is the whole hazard: a release is not a hold, so none of the boundaries the hold path maintains apply to it, and the next person to add a memo in this area needs to know that before they add it rather than after.
TakenPlaceSubscriber drops the session on every booking event the engine
already announces, so a seat freed from the map, the cart, the operator screens,
the API, a settlement or the cron sweep leaves the answer at once and no caller
has to remember anything. Subscribed rather than called at the release sites:
there are thirteen of them over nine files, and a memo each of them had to
remember to drop is a memo that goes stale the first time somebody adds a
fourteenth.
PlaceHoldCostTest::testThePlaceGivenBackIsFreeToTheNextReader pins it, and
fails with the subscriber removed.
Not done here
PinnedPlaceHold gained a tally() so what each allotment has taken stays current under the locks instead of being re-read per line, which only costs anything on a hall that pins places. hasAvailablePlace() is left as it is: it stops at the first row on purpose and is not on the hold path.
Why the availability shape, and not a batch of counts
The raw query counts above are the smaller argument. The better one is that another rule's cost goes to zero.
The orphaned_places rule from #3616316 judges the whole available house under
the slot lock, and when it was written that was a read of its own: what the
seating strategy had read moments earlier came from above the lock, and judging a
refusal against a stale picture is what the locked pass exists to avoid. Because
the constraint and the policy now ask one service about one session, and because
the constraints are primed before the group policies, whichever asks first pays
and the other is answered.
Measured, a party of three on the same 24-place hall, with and without the rule attached:
| before | after | |
|---|---|---|
| no rule attached | 29 | 19 |
| the rule attached | 30 | 19 |
Pinned by PlaceHoldCostTest::testTheOrphanRuleCostsNothingToEnforce rather than
claimed in a docblock, and seen to fail against the unfixed code with exactly the
one extra query #3616316 measured.