Re: Adding locks statistics

Hi,

On Sun, Mar 22, 2026 at 08:37:47PM +0100, Álvaro Herrera wrote:
> On 2026-Mar-21, Álvaro Herrera wrote:
>
> > I checked this, and found a couple of headers that can benefit from a
> > removal, as shown in the attached patches.
>
> I looked again and found some more; the first 14 patches attached here
> do so. A couple of them have no fallout to speak of, which I find
> really surprising; for the majority we just need a couple of extra
> includes somewhere or a typedef or two. I unleashed CI on it to see
> what would happen,
> https://cirrus-ci.com/build/5522804717649920

Thanks for looking at it!

I looked at the C files changes that now need to include lock.h (or other ones)
directly:

verify_heapam.c: lock.h was indirectly included through procarray.h
heapam_handler.c: lock.h was indirectly included through heapam.h -> tableam.h
-> vacuum.h
relation.c: lock.h was indirectly included through namespace.h
reloptions.c: lock.h was indirectly included through reloptions.h
indexam.c: lock.h was indirectly included through reloptions.h
relcache.c: lock.h was indirectly included through reloptions.h
syscache.c: lock.h was indirectly included through lmgr.h
discard.c: lock.h was indirectly included through namespace.h
pg_subscription.c: lock.h was indirectly included through heapam.h -> tableam.h
-> vacuum.h
nbtree.c: lock.h was indirectly included through vacuum.h
nbtutils.c: lock.h was indirectly included through reloptions.h
pg_inherits.c: lock.h was indirectly included through pg_inherits.h
inherit.c: lock.h was indirectly included through pg_inherits.h
conversioncmds.c: lock.h was indirectly included parse_func.h -> namespace.h
tablespace.c: lock.h was indirectly included through heapam.h -> tableam.h
-> vacuum.h
parse_oper.c: lock.h was indirectly included through parse_func.h -> namespace.h
sequencesync.c: lock.h was indirectly included through worker_internal.h
ts_cache.c: lock.h was indirectly included through namespace.h

So the changes done in your patches make sense to me.

I have 2 comments:

1/ wait_event.c

-#include "storage/lmgr.h" /* for GetLockNameFromTagType */
-#include "storage/lwlock.h" /* for GetLWLockIdentifier */
+#include "storage/lmgr.h"
+#include "storage/lwlock.h"
+#include "storage/shmem.h"
#include "storage/spin.h"
+#include "utils/hsearch.h"

hsearch.h is already included into shmem.h so its direct include is not needed.
That said wait_event.c needs it so including it directly might make sense just from
a coding "style" point of view (given that it is harmless as it is protected by
ifndef HSEARCH_H).

2/ Not directly linked to your patches

It looks like that aio_funcs.c does not need lock.h (reported by include-what-you-use).
If we remove its direct include, it's still indirectly included through proc.h
though. But I think that removing its direct include makes sense as it's not
needed at all.

If we still need to discuss about those lock.h related changes, maybe it's
worth creating a dedicated thread?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com