Skip to content

Include cleanup: reduce header bloat#60490

Draft
MaxKellermann wants to merge 106 commits intoceph:mainfrom
MaxKellermann:includes2
Draft

Include cleanup: reduce header bloat#60490
MaxKellermann wants to merge 106 commits intoceph:mainfrom
MaxKellermann:includes2

Conversation

@MaxKellermann
Copy link
Member

@MaxKellermann MaxKellermann commented Oct 25, 2024 •
edited
Loading

This is a continuation of #60253 (and contains many of its commits). It is a demonstration of what can be done do reduce header bloat.
Ceph build times are painfully slow and each compilation unit takes about 2 GB RAM, which means parallel builds require excessive amounts of CPU and RAM. This is because the header dependency tree is very large.
This draft PR shows a few ideas how to improve this:

  • include only headers that are needed
  • split headers so includers can decide what parts they need
  • un-inline functions/methods so their dependencies don't need to be included in the public header and the compiler doesn't need to generate code in each compilation unit that includes the header

Prior to this PR a clean build of ceph-mds:

3657.90user 222.19system 2:46.14elapsed 2335%CPU (0avgtext+0avgdata 3838844maxresident)k
0inputs+0outputs (24major+81216038minor)pagefaults 0swaps

 Performance counter stats for 'time ninja -C build/debug ceph-mds':

      3,891,960.89 msec task-clock:u                     #   23.424 CPUs utilized             
                 0      context-switches:u               #    0.000 /sec                      
                 0      cpu-migrations:u                 #    0.000 /sec                      
        81,140,096      page-faults:u                    #   20.848 K/sec                     
 9,183,874,937,949      cycles:u                         #    2.360 GHz                       
 9,098,969,848,371      instructions:u                   #    0.99  insn per cycle            
 2,021,603,305,694      branches:u                       #  519.431 M/sec                     
    53,365,675,475      branch-misses:u                  #    2.64% of all branches           

     166.151033259 seconds time elapsed

    3657.906470000 seconds user
     222.190450000 seconds sys

After this PR:

2621.21user 152.68system 2:05.10elapsed 2217%CPU (0avgtext+0avgdata 3271296maxresident)k
0inputs+0outputs (0major+56495236minor)pagefaults 0swaps

 Performance counter stats for 'time ninja -C build/debug -k50 ceph-mds':

      2,784,514.32 msec task-clock:u                     #   22.258 CPUs utilized             
                 0      context-switches:u               #    0.000 /sec                      
                 0      cpu-migrations:u                 #    0.000 /sec                      
        56,476,974      page-faults:u                    #   20.283 K/sec                     
 6,628,349,851,908      cycles:u                         #    2.380 GHz                       
 6,567,858,023,127      instructions:u                   #    0.99  insn per cycle            
 1,452,547,427,587      branches:u                       #  521.652 M/sec                     
    41,054,543,018      branch-misses:u                  #    2.83% of all branches           

     125.103906411 seconds time elapsed

    2621.212510000 seconds user
     152.683572000 seconds sys

This is a speedup of 28%, and much more can be done (still a lot of header bloat, and type erasure might also help a lot).

Note that this is a quick'n'dirty draft PR. Only ceph-mds builds and all other executables will fail, some due to pre-existing bugs because includes were missing, others because I didn't adapt to splitted headers yet.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)

@MaxKellermann MaxKellermann force-pushed the includes2 branch 2 times, most recently from 41aa080 to 2d0d1d2 Compare October 27, 2024 13:30
@MaxKellermann MaxKellermann force-pushed the includes2 branch 10 times, most recently from ad8aa26 to 41f34dd Compare October 30, 2024 14:10
@MaxKellermann MaxKellermann mentioned this pull request Oct 30, 2024
Merged
14 tasks
@MaxKellermann MaxKellermann force-pushed the includes2 branch 7 times, most recently from 96c47bf to ab8e62f Compare November 2, 2024 22:44
@MaxKellermann
common/error_code: un-inline functions to reduce header dependencies
eb75101 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
common/escape: include cleanup
ffccce9 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
common/escape: use std::optional instead of boost::optional
27b4843 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
common/strescape: include cleanup
150268b 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
include/xlist: move ostream operator to xlist_print.h
dcd19cd 
Downgrade <iostream> to <iosfwd>.

Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
osd/PG: include cleanup
1c896b2 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
erasure-code/consistency/RadosCommands: include cleanup
4e27002 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/mdstypes: move old_inode_t to separate header
27c1c5f 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/Capability: include cleanup
5ec4344 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/JournalPointer: include cleanup
c4ea445 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/SnapRealm: include cleanup
5b73aa3 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/OpenFileTable: include cleanup
0549dbe 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/MDBalancer: un-inline methods to reduce header dependencies
f3d4489 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/LogSegment: un-inline methods to reduce header dependencies
b8f507e 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/mdstypes: move metareqid_t to separate header
bea5f53 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/BatchOp: include cleanup
67dcf1b 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/MDSPinger: include cleanup
b974103 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/FSMapUser: remove unused local variable
5a5fa94 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/FSMapUser: include cleanup
ee3059f 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
include/cephfs/types.h: move mds_rank_t to rank.h
decc87e 
To reduce header dependencies and compile times.

Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/SessionMap: forward-declare MDRequestImpl
49d74f2 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/DamageTable: include cleanup
20d2b19 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/MetricAggregator: include cleanup
76300fe 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/Anchor: include cleanup
e506d7d 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/Mantle: include cleanup
dc4131d 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
mds/MDLog: include cleanup
2a776d9 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
common/mclock_common: forward-declare classes MonClient/CephContext
3f429d9 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
osd/scheduler/OpSchedulerItem: include cleanup
70d13ec 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann
common/mclock_common: include cleanup
4320ffb 
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
@MaxKellermann MaxKellermann mentioned this pull request Mar 19, 2026
Open
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bluestore build/ops cephfs Ceph File System common core crimson libcephsqlite ceph SQLite3 VFS mgr mon nvmeof rbd rgw tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@MaxKellermann @ceph-jenkins