The basket pointer is re-read from the tempstore on every question asked of the cart, so one place click costs four identical key/value reads

Reads the session's basket pointer at most once per request.

Core does not cache the private tempstore, so every PrivateTempStore::get() is a SELECT. current(), activeOrder() and heldLines() all resolve the same pointer, so a surface asking the cart several questions read the same key/value row once per question. Taking one place on the graphical map read it four times.

adopt() and forget() are the only writers of the key and both keep the held value in step. Holding it for the request is also the more correct reading: the tempstore is shared across requests, so re-reading can answer one question from one basket and a later one from another.

Measured

before after
taking a place, basket already open 25 queries 22
drawing a 60 place hall 13 queries 13

Tests

BookingCartTest::testTheBasketPointerIsReadOncePerRequest counts the tempstore reads through Database::startLog(), and was seen to fail against unfixed code at 5 reads before passing at 1. It seeds the pointer directly rather than through adopt(), because adopt() is itself one of the writers that keeps the held value in step and would leave nothing to read. testAdoptAndForgetMoveThePointer guards the invalidation.

Two performance test classes come with it, the instrument that found this and the guard against it returning: the per-slot booking page and the place map, following core's Gander pattern.

What those tests deliberately do not measure

Query counts are asserted exactly, because that is what these tests exist to hold: a ceiling would let a slide back pass in silence and would record no improvement when one is earned. Everything that proved to answer to the environment rather than to this module is not asserted at all, rather than asserted loosely, so the file does not claim to measure what it does not:

  • No cold-cache scenario. Warming a whole site is mostly the cost of whatever else is installed: 58 queries in a development docroot against 50 on CI.
  • No CacheGetCount. It counts every read of every bin, so it moves with core's caching and with neighbouring modules. It differed by one between environments with none of this code changed.
  • The opening click of a visit is taken but not measured. It came to 49, 47 and 46 queries in three settings differing in none of yoyaku's code, because it turns on when the tempstore's table comes into being. The click that lands in an already-open basket is the one a booker choosing six places pays five times over, and it measured 22 everywhere.

Follow-up, filed and postponed: [#3615297] records the remaining 22, the four reductions that need no change of guarantee, and the two that are decisions rather than cleanups.

Drive-by, in the test class already being edited: an unused property and a @return that disagreed with the native return type.

Edited by Frank Mably

Merge request reports

Loading
Loading