Problem/Motivation
PlaceHoldCostTest measures what the hold path costs, and measures it against the size of the party rather than per booking, which is the reasoning its own docblock gives for catching #3616341: One hold asks the same slot-wide question once per line, so a party of twelve costs 102 queries. Nothing does the same for any other path.
That gap is not theoretical. While fixing #3620326: Read the house once per page, open an add form in one place, and name only what exists an extra call to getRunningBookingIds() was added to a page. The lookup behind it runs a paged query and resolves each instance's transaction one at a time: with its page size of fifty it costs 102 queries a call, so calling it twice cost more than the fifty entity loads the batching had just saved. The page ended up about 52 queries worse while the commit claimed to remove an N+1.
It passed everything: phpstan, phpcs with DrupalPractice, the kernel suite, and eight consecutive green pipelines. No gate in the toolchain counts queries, so nothing could have caught it. It was found by reading the diff.
The same blind spot is where every performance finding in that audit lived: a section read per area on the seat map's cache fill, an entity load per option in the venue-scoped widget, a grade load per row in the configuration area table. All of them admin screens or cache fills, none of them measured.
Proposed resolution
Extend the Database::startLog() pattern already used by PlaceHoldCostTest to the paths the audit touched, asserted the way that class asserts them: as a budget that does not grow with the size of the input.
- The venue map's settled-state fill, which must not grow with the number of areas.
- The venue-scoped options widget, which must not grow with the number of options.
- The configuration area table, which must not grow with the number of rows.
Each wants two fixtures of different sizes, so that what is asserted is that the count does not follow the input rather than that it equals a particular number.
One trap to write down while it is known: a kernel test cannot see the cache backend's own queries, because the backend there is cache.backend.memory. The assertion has to be on the reads the fill performs, never on the cache get or set.
Remaining tasks
Whether the repeated-costly-call detector written during that audit belongs in CI as well as in a test is worth deciding separately. A test that pins counts catches the effect; a detector catches the shape.
User interface changes
None.
API changes
None.
Data model changes
None.
AI-Generated: Yes (Claude Code was used to help draft this issue summary. I reviewed it before posting; there is no code on this issue yet.)
Issue fork yoyaku-3620377
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
mably commented