Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile Time Improvement: Isolate boost::filesystem as a depenendency of cello.cpp #320

Merged

Conversation

mabruzzo
Copy link
Contributor

Out of curiosity, I was using clang's -ftime-trace flag to test profile where most of the compile time was being spent. Unsurprisingly, a lot of the time (maybe 30% - 50% per source file is spent reading in header files). A lot of this time is spent reading the <charm++> and C++ standard library headers. I hope to look at that in the future.

Somewhat surprisingly, every source file was spending ~0.4 seconds just reading <boost/filesystem> (this may be a slight underestimate). Given that we have over 220 source files, that's about 80 seconds per build (in serial). For context, a serial build on my machine probably takes ~15 minutes (maybe more). This was not very rigorous, but it seemed like an easy win to slightly improve build times by a few percent.

… of cello.cpp

Out of curiosity, I was using clang's -ftime-trace flag to test profile where most of the compile time was being spent. Unsurprisingly, a lot of the time (maybe 30% - 50% per source file is spent reading in header files). A lot of this time is spent reading the ``<charm++>`` and C++ standard library headers. I hope to look at that in the future.

Somewhat surprisingly, every source file was spending ~0.4 seconds just reading <boost/filesystem> (this may be a slight underestimate). Given that we have over 220 source files, that's about 80 seconds per build (in serial). For context, a serial build on my machine probably takes ~15 minutes (maybe more). This was not very rigorous, but it seemed like an easy win.
Copy link
Contributor

@gregbryan gregbryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good (but will need to be modified since png was also removed)

@mabruzzo mabruzzo requested a review from jobordner July 26, 2023 17:39
@jwise77
Copy link
Contributor

jwise77 commented Feb 19, 2024

@mabruzzo Can you resolve the conflicts? Then we can merge since I've marked this as a 1 reviewer PR.

@bwoshea
Copy link
Contributor

bwoshea commented Feb 20, 2024

This PR now passes all of the tests and is up-to-date with the tip of main. Time to merge!

@bwoshea bwoshea merged commit 26d0e04 into enzo-project:main Feb 20, 2024
7 checks passed
@mabruzzo mabruzzo deleted the isolate-boost-filesystem-dependency branch March 19, 2024 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants