refactor: re-order headers and forward declarations to improve compile time#5693
Merged
PastaPastaPasta merged 15 commits intodashpay:developfrom Nov 17, 2023
Merged
refactor: re-order headers and forward declarations to improve compile time#5693PastaPastaPasta merged 15 commits intodashpay:developfrom
PastaPastaPasta merged 15 commits intodashpay:developfrom
Conversation
UdjinM6
approved these changes
Nov 14, 2023
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
make -j7 clean && ./autogen.sh && ./configure --enable-crash-hooks --enable-werror --enable-suppress-external-warnings --prefix=
pwd/depends/aarch64-apple-darwin22.6.0 --disable-ccache && time make -j7
develop:
make -j7 1011.18s user 77.76s system 636% cpu 2:51.15 total
make -j7 1014.88s user 78.29s system 634% cpu 2:52.28 total
5693:
make -j7 1011.06s user 80.08s system 620% cpu 2:55.93 total
make -j7 1004.36s user 78.48s system 636% cpu 2:50.02 total
i.e. no improvement in compilation time for me but I like these changes anyway so
utACK
Good job! The circular dependency "llmq/debug -> llmq/dkgsessionhandler -> llmq/debug" is no longer present. Please remove it from EXPECTED_CIRCULAR_DEPENDENCIES in test/lint/lint-circular-dependencies.sh to make sure this circular dependency is not accidentally reintroduced. Good job! The circular dependency "llmq/debug -> llmq/dkgsessionhandler -> llmq/dkgsession -> llmq/debug" is no longer present. Please remove it from EXPECTED_CIRCULAR_DEPENDENCIES in test/lint/lint-circular-dependencies.sh to make sure this circular dependency is not accidentally reintroduced.
UdjinM6
force-pushed
the
build-optimize-headers
branch
from
6a39a73 to
8518694
Compare
|
rebased from GH GUI to fix |
PastaPastaPasta
approved these changes
Nov 17, 2023
Member
PastaPastaPasta
left a comment
PastaPastaPasta
left a comment
There was a problem hiding this comment.
utACK for squash merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Some headers include other heavy headers, such as
logging.h,tinyformat.h,iostream. These headers are heavy and increase compilation time on scale of whole project drastically because can be used in many other headers.What was done?
Moved many heavy includes from headers to cpp files to optimize compilation time.
In some places added forward declarations if it is reasonable.
As side effect removed 2 circular dependencies:
How Has This Been Tested?
Run build 2 times before refactoring and after refactoring:
make clean && sleep 10s; time make -j18Before refactoring:
After refactoring:
~5% of improvement for compilation time. That's not huge, but that's worth to get merged
There're several more refactorings TODO but better to do them later by backports:
Breaking Changes
N/A
Checklist: