Thread: We should lazy-initialize the deadlock checker state memory

We should lazy-initialize the deadlock checker state memory

From
Andres Freund
Date:
Hi,

While looking at [1] I noticed that a decent amount of CPU usage was spent
allocating memory in InitDeadLockChecking(), which we do very early during
backend startup.

With values of max_connections (et al) that are common today, each backend
allocates a surprisingly large amount of memory for the deadlock checker -
despite it being very common that a backend will never run a deadlock check.

E.g. with max_connections = 5000 my configuration has
┌──────────────────┬────────┬───────┬──────┬─────────────┬───────────────┬────────────┬─────────────┐
│       name       │ ident  │ level │ path │ total_bytes │ total_nblocks │ free_bytes │ free_chunks │
├──────────────────┼────────┼───────┼──────┼─────────────┼───────────────┼────────────┼─────────────┤
│ TopMemoryContext │ (null) │     1 │ {1}  │     1173920 │            11 │      10024 │           8 │
└──────────────────┴────────┴───────┴──────┴─────────────┴───────────────┴────────────┴─────────────┘
but if I remove the call to InitDeadLockChecking() it is:
┌──────────────────┬────────┬───────┬──────┬─────────────┬───────────────┬────────────┬─────────────┐
│       name       │ ident  │ level │ path │ total_bytes │ total_nblocks │ free_bytes │ free_chunks │
├──────────────────┼────────┼───────┼──────┼─────────────┼───────────────┼────────────┼─────────────┤
│ TopMemoryContext │ (null) │     1 │ {1}  │       65616 │             3 │      10024 │           8 │
└──────────────────┴────────┴───────┴──────┴─────────────┴───────────────┴────────────┴─────────────┘

I.e. we spend ~1.1MB allocating memory for the deadlock checker. In every
backend.


The reason for eagerly allocating it is explained above
InitDeadLockChecking():
 * [...] We want to allocate the
 * space at startup because (a) the deadlock checker might be invoked when
 * there's no free memory left, and (b) the checker is normally run inside a
 * signal handler, which is a very dangerous place to invoke palloc from.


But somebody (look over there, in that other building), just forgot to update
that comment, in 6753333f55e1, in 2015. These days deadlock checking happens
"in the foreground" in ProcSleep().


ISTM that we could move the call to InitDeadLockChecking() to the start of
CheckDeadLock(), before it acquires all the locks. That'd require making it
safe to call InitDeadLockChecking() multiple times, but that's obviously
trivial.


Greetings,

Andres Freund

[1] https://postgr.es/m/hgs2vs74tzxigf2xqosez7rpf3ia5e7izalg5gz3lv3nqfptxx%40thanmprbpl4e



Andres Freund <andres@anarazel.de> writes:
> ISTM that we could move the call to InitDeadLockChecking() to the start of
> CheckDeadLock(), before it acquires all the locks. That'd require making it
> safe to call InitDeadLockChecking() multiple times, but that's obviously
> trivial.

Hmph.  Do we even need that to be persistent storage at all, rather
than just allocating it for the duration of CheckDeadLock?

            regards, tom lane