From: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: allow benign typedef redefinitions (C11) |
Date: | 2025-10-02 13:18:16 |
Message-ID: | 202510021240.ptc2zl5cvwen@alvherre.pgsql |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2025-Sep-29, Álvaro Herrera wrote:
> I've been looking at removing some includes from a few more headers, but
> I'm not happy with the overall achievement, or at least I don't have a
> good metric to present as a win. So I'll leave that for another day and
> maybe present a different way to look at the problem.
Okay, I couldn't put this down. I wrote a script that greps for all the
"#include" lines in headers and in backend .c files, and put them in a
database. With that I can figure out the complete list of headers used
by each source file (recursively), which allows to me know how many
source files are including each header file. The script is attached, in
case anyone is curious. I didn't bother to optimize it, because I
wasn't going to run it many times anyway.
Using the numbers so obtained and the patches I've been looking at, I
can quantify the reduction in total header usage for each commit, so I
know whether any particular change is quantitatively worthwhile or not,
beyond qualitative modularity considerations.
So, for patch 0001, we can see a massive decrease of .c files that use
each of a bunch of .h files (the first column is the number of .c files
that include that particular .h file before patch; the second column is
the same number, after patch; third column is the difference):
include │ orig # included │ final # included │ difference
────────────────────────┼─────────────────┼──────────────────┼────────────
nodes/nodes.h │ 5501 │ 5130 │ 371
nodes/pg_list.h │ 4463 │ 4092 │ 371
storage/off.h │ 3943 │ 3424 │ 519
common/relpath.h │ 3917 │ 3600 │ 317
nodes/bitmapset.h │ 3888 │ 3562 │ 326
storage/itemptr.h │ 3638 │ 3119 │ 519
catalog/pg_attribute.h │ 3575 │ 3207 │ 368
access/tupdesc.h │ 3573 │ 3205 │ 368
access/htup.h │ 3297 │ 2778 │ 519
storage/dsm.h │ 3273 │ 1972 │ 1301
utils/relcache.h │ 2767 │ 2389 │ 378
port/atomics.h │ 2734 │ 1751 │ 983
lib/pairingheap.h │ 2211 │ 1629 │ 582
utils/dsa.h │ 2163 │ 1082 │ 1081
utils/snapshot.h │ 1863 │ 1282 │ 581
nodes/tidbitmap.h │ 1508 │ 924 │ 584
utils/sortsupport.h │ 1489 │ 1076 │ 413
access/skey.h │ 1020 │ 909 │ 111
access/genam.h │ 976 │ 393 │ 583
access/amapi.h │ 725 │ 140 │ 585
utils/typcache.h │ 667 │ 85 │ 582
access/brin_internal.h │ 607 │ 21 │ 586
access/ginblock.h │ 606 │ 20 │ 586
access/brin_tuple.h │ 601 │ 15 │ 586
access/gin_tuple.h │ 588 │ 2 │ 586
(25 filas)
Patch 0002 doesn't cause a reduction in as many header files, but it's
still quite impressive that htup_details.h is no longer included by a
thousand header files:
include │ orig # included │ final # included │ difference
────────────────────────┼─────────────────┼──────────────────┼────────────
nodes/nodes.h │ 5130 │ 5050 │ 80
nodes/pg_list.h │ 4092 │ 4007 │ 85
storage/off.h │ 3424 │ 2957 │ 467
catalog/pg_attribute.h │ 3207 │ 3110 │ 97
access/tupdesc.h │ 3205 │ 3108 │ 97
storage/itemptr.h │ 3119 │ 2636 │ 483
access/htup.h │ 2778 │ 2283 │ 495
access/transam.h │ 2540 │ 1923 │ 617
storage/bufpage.h │ 1882 │ 1268 │ 614
access/tupmacs.h │ 1692 │ 1077 │ 615
access/htup_details.h │ 1375 │ 500 │ 875
Patch 0003 has a negiglible impact though (which makes sense given what
it does):
include │ orig # included │ final # included │ difference
─────────────────────┼─────────────────┼──────────────────┼────────────
storage/off.h │ 2957 │ 2954 │ 3
storage/itemptr.h │ 2636 │ 2633 │ 3
access/htup.h │ 2283 │ 2280 │ 3
executor/tuptable.h │ 1115 │ 1112 │ 3
(4 filas)
But patch 0004 again removes inclusion of a large number of headers,
though from not as many .c files:
include │ orig # included │ final # included │ difference
──────────────────────────────┼─────────────────┼──────────────────┼────────────
nodes/nodes.h │ 5050 │ 5008 │ 42
nodes/pg_list.h │ 4007 │ 3964 │ 43
common/relpath.h │ 3600 │ 3574 │ 26
nodes/bitmapset.h │ 3562 │ 3531 │ 31
catalog/pg_attribute.h │ 3110 │ 3067 │ 43
access/tupdesc.h │ 3108 │ 3065 │ 43
storage/off.h │ 2954 │ 2913 │ 41
storage/itemptr.h │ 2633 │ 2592 │ 41
utils/relcache.h │ 2389 │ 2350 │ 39
access/htup.h │ 2280 │ 2243 │ 37
storage/spin.h │ 2217 │ 2177 │ 40
nodes/primnodes.h │ 1975 │ 1965 │ 10
storage/dsm.h │ 1972 │ 1952 │ 20
storage/fd.h │ 1948 │ 1863 │ 85
port/atomics.h │ 1751 │ 1746 │ 5
lib/pairingheap.h │ 1629 │ 1606 │ 23
storage/proclist_types.h │ 1508 │ 1492 │ 16
utils/snapshot.h │ 1282 │ 1275 │ 7
storage/bufpage.h │ 1268 │ 1246 │ 22
executor/tuptable.h │ 1112 │ 1090 │ 22
utils/dsa.h │ 1082 │ 1077 │ 5
access/tupmacs.h │ 1077 │ 1055 │ 22
utils/sortsupport.h │ 1076 │ 1033 │ 43
storage/fileset.h │ 1073 │ 1030 │ 43
nodes/memnodes.h │ 1028 │ 1007 │ 21
storage/sharedfileset.h │ 1027 │ 984 │ 43
utils/memutils.h │ 1026 │ 1005 │ 21
nodes/tidbitmap.h │ 924 │ 917 │ 7
utils/queryenvironment.h │ 913 │ 892 │ 21
access/skey.h │ 909 │ 902 │ 7
access/itup.h │ 785 │ 763 │ 22
nodes/plannodes.h │ 694 │ 672 │ 22
storage/condition_variable.h │ 676 │ 654 │ 22
utils/tuplestore.h │ 635 │ 613 │ 22
executor/instrument.h │ 601 │ 579 │ 22
nodes/miscnodes.h │ 600 │ 578 │ 22
access/attmap.h │ 593 │ 571 │ 22
utils/logtape.h │ 588 │ 566 │ 22
utils/tuplesort.h │ 585 │ 563 │ 22
lib/simplehash.h │ 581 │ 559 │ 22
access/tupconvert.h │ 575 │ 553 │ 22
utils/sharedtuplestore.h │ 573 │ 551 │ 22
nodes/execnodes.h │ 570 │ 548 │ 22
(43 filas)
My inclination at this point is to apply all four, because the
modularity wins are pretty obvious. Of course there are other ways to
look at the data (in particular: number of .h files that each other .h
cascades to), but I'm going to stop here.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Oh, great altar of passive entertainment, bestow upon me thy discordant images
at such speed as to render linear thought impossible" (Calvin a la TV)
Attachment | Content-Type | Size |
---|---|---|
0001-Remove-brin-gin_tuple.h-from-tuplesort.h.patch | text/x-diff | 10.3 KB |
0002-Don-t-include-access-htup_details.h-in-executor-tupt.patch | text/x-diff | 15.4 KB |
0003-don-t-include-executor-tuptable.h-in-access-tupconve.patch | text/x-diff | 851 bytes |
0004-Don-t-include-execnodes.h-in-brin.h-or-gin.h.patch | text/x-diff | 2.0 KB |
build-include-database | text/plain | 3.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Dmitry Koval | 2025-10-02 13:26:00 | Re: Add SPLIT PARTITION/MERGE PARTITIONS commands |
Previous Message | Nazir Bilal Yavuz | 2025-10-02 12:52:19 | Re: split func.sgml to separated individual sgml files |