otp icon indicating copy to clipboard operation
otp copied to clipboard

Optimise accesses to the Table name -> Table id map

Open RobinMorisset opened this issue 1 year ago • 23 comments

Currently these accesses are guarded by (striped) read-write locks.

With this patch, there are still (regular) locks for protecting writing threads from each other, but threads that only need to read that map (so the overwhelming majority) no longer need to use any lock. Instead we now rely on lightweight atomic synchronisation, and on the thread_progress functionality for memory reclaimation.

One ugly hack is that if we add 1 table, then remove it, the corresponding bucket may go

  • inline, empty
  • inline, with a value
  • outlined vector without a value

Once we have a filled the inline vector, we can only transition to an outlined vector, and never go back to the inline state. This saves us from an ABA hazard:

  • Reader sees inline vector of size 1
  • Reader reads the name NameA that corresponds to table IdA, sees it matches what it looked for
  • Writer deletes (or rename the table), moving us to an empty vector
  • Writer inserts another table in that bucket, with a different name: NameB -> IdB
  • Reader reads IdB, concluding that NameA -> IdB. As long as we stick to outline vectors and always allocate a fresh one when deleting tables, we don't have that risk, as the vectors only get freed (and thus the adress may get reused by another vector) once all readers have made thread progress (and thus are no longer in the middle of an operation).

One consequence of this hack, is that we must lie to the memory counting system, pretending to have an outlined vector at all times, even when we only have the initial inlined vector. Otherwise, the tests detect the memory increase in inline (1 element) -> outlined (0 element) -> outlined (1 element), and report a memory leak.

This patch was motivated by noticing that heavyweight services can spend several % of CPU time in db_get_table_aux. A trial in production confirmed that this patch roughly halves the time spent in that function.

RobinMorisset avatar Apr 13 '23 14:04 RobinMorisset