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.

Edited by Frank Mably

Merge request reports

Loading
Loading