Skip to content

Comments

Include cleanup: reduce header bloat#60490

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

Include cleanup: reduce header bloat#60490
MaxKellermann wants to merge 138 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
mds/Capability: include cleanup
746fd36 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/JournalPointer: include cleanup
1b6bf27 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/SnapRealm: include cleanup
7f3d072 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/OpenFileTable: include cleanup
c8e244b 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/MDBalancer: un-inline methods to reduce header dependencies
8e763c0 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/LogSegment: un-inline methods to reduce header dependencies
72e7179 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/mdstypes: move metareqid_t to separate header
0f82890 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/BatchOp: include cleanup
f676791 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/MDSPinger: include cleanup
48b5a6c 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/FSMapUser: remove unused local variable
be1a3e3 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/FSMapUser: include cleanup
6a6bc01 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
include/cephfs/types.h: move mds_rank_t to rank.h
b1dab38 
To reduce header dependencies and compile times.

Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/SessionMap: forward-declare MDRequestImpl
0e09733 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/DamageTable: include cleanup
90ad9d9 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/MetricAggregator: include cleanup
e68012f 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/Anchor: include cleanup
495f363 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/Mantle: include cleanup
b331179 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
mds/MDLog: include cleanup
c5c2905 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
common/mclock_common: forward-declare classes MonClient/CephContext
1324290 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
osd/scheduler/OpSchedulerItem: include cleanup
c47de9a 
Signed-off-by: Max Kellermann <[email protected]>
@MaxKellermann
common/mclock_common: include cleanup
00f173c 
Signed-off-by: Max Kellermann <[email protected]>
@github-actions
Copy link

github-actions bot commented Dec 15, 2025

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@github-actions
Copy link

github-actions bot commented Feb 13, 2026

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Feb 13, 2026
@MaxKellermann
Copy link
Member Author

MaxKellermann commented Feb 15, 2026

This PR isn't stale, it's just that all my other PRs which were based on this one have been ignored for the past 5 months with no explanation and no review. I will update this PR as soon as there is feedback for the other PRs. Until then, I will keep posting useless comments (to all PRs) to prevent the bot from auto-closing it.

@github-actions github-actions bot removed the stale label Feb 16, 2026
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 needs-rebase 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