optimize build time (move stuff from hpp to cpp) #1787

pcaspers · 2023-09-18T14:41:21Z

I am looking into optimizing the build time of ORE. I am using ClangBuildAnalyzer for this. Some optimizations will affect QuantLib, so I want to check first if these changes are welcome or not.

I started with "expensive headers". Both for ORE and QuantLib observable.hpp is high up in the list. For the QuantLib build

**** Expensive headers:
209293 ms: /Users/peter.caspers/QuantLib/ql/patterns/observable.hpp (included 825 times, avg 253 ms), included via:

By moving implementations from hpp to cpp the build time goes from

  Parsing (frontend):         2045.0 s
  Codegen & opts (backend):   1602.3 s

to

  Parsing (frontend):         2085.9 s
  Codegen & opts (backend):   1568.2 s

so no improvement so far! However, this change is not ready yet, I'd like to extend the optimization to the thread-safe code section and apply similar optimizations to other headers as well.

I am never sure, whether removing the inline qualifier affects the performance (or size) of the generated code. My rough understanding is that for modern compilers it should not. But I've not investigated that in any more detail.

Could you let me know whether you are okay with changes like the one in this PR. I am unsure whether we would apply those in our fork without our mothership doing the same, since a lot of stuff is moved around. @lballabio

coveralls · 2023-09-18T16:05:14Z

coverage: 71.913% (+0.001%) from 71.912% when pulling 29eede9 on pcaspers:optimize_build_time into cd33939 on lballabio:master.

lballabio · 2023-09-18T19:44:01Z

I'm ok with it. As for inline, I think modern compilers can do without it but only if full-program optimization is enabled at link time.

sweemer · 2023-09-19T11:39:39Z

I was also looking at this a while back with Build Insights but the rabbit hole went a bit too deep for me :) I might try to take another look soon if I can.

If I recall correctly, ql/errors.hpp was one of the expensive ones pointed out by Build Insights, so it's interesting that you actually added it to more places. Since this header is included directly or transitively by almost all translation units, I believe that speeding it up should provide bang for the buck.

pcaspers · 2023-09-19T11:41:32Z

yes I am looking at errors.hpp right now :-)

lballabio · 2023-09-19T11:42:47Z

I'm guessing the problem is the inclusion of sstream to compose error messages.

pcaspers · 2023-09-19T12:21:56Z

I'm guessing the problem is the inclusion of sstream to compose error messages.

Yes it's a big contributor:

159628 ms: /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk/usr/include/c++/v1/ostream (included 1120 times, avg 142 ms), included via:
  67x: errors.hpp sstream istream 
...

pcaspers · 2023-09-19T14:12:25Z

after simplifying errors.hpp:

  Parsing (frontend):         1982.0 s
  Codegen & opts (backend):   1493.0 s

pcaspers · 2023-09-19T15:30:29Z

after moving some functions from cubicinterpolation.hpp -> cpp

  Parsing (frontend):         1887.5 s
  Codegen & opts (backend):   1375.4 s

pcaspers · 2023-09-19T15:32:22Z

instrument.hpp, array.hpp, date.hpp, calendar.hpp, matrix.hpp are the next candidates to move stuff from hpp -> cpp

…alendar.hpp, matrix.hpp

pcaspers · 2023-09-22T17:32:05Z

after this:

  Parsing (frontend):         1781.8 s
  Codegen & opts (backend):   1305.7 s

pcaspers · 2023-09-22T17:33:56Z

so far we went from 3647s to 3086s which is -15%

lballabio

A few comments. Also, instead of optimizing single headers, I'd look into unity builds if I were you. You can enable them automatically in VC++ and CMake, and our configure script supports them as well. It would save a lot more time with a lot less work, and might make these changes less relevant.

lballabio · 2023-09-23T17:16:09Z

ql/errors.hpp

@@ -49,94 +46,58 @@ namespace QuantLib {
        const char* what() const noexcept override;

      private:
-        ext::shared_ptr<std::string> message_;
+        std::string message_;


Technically, having ext::shared_ptr<std::string> makes the copy constructor not throwing, as required by the standard. With std::string, it can throw. On the other hand, if it throws, it probably means we're out of memory anyway so all bets are off. I'd try and measure the effect of shared_ptr only, though.

Right, I had to look that requirement to be honest.

lballabio · 2023-09-23T17:17:40Z

ql/math/array.cpp

+    Real Array::operator[](Size i) const {
+        #if defined(QL_EXTRA_SAFETY_CHECKS)
+        QL_REQUIRE(i<n_,
+                   "index (" << i << ") must be less than " << n_ <<
+                   ": array access out of range");
+        #endif
+        return data_.get()[i];
+    }


Let's at least inline element access, please. I'd also try timing some heavy finite-difference calculations to check that we're not pessimizing run time.

lballabio · 2023-09-23T17:20:52Z

ql/math/matrix.hpp

-    inline Real &Matrix::operator()(Size i, Size j) const {
-        return data_[i*columns()+j];
-    }


Same, let's at least inline element access. This means operator(), operator[]andcolumns`.

lballabio · 2023-09-23T17:22:41Z

ql/math/statistics/incrementalstatistics.hpp

-        downside_accumulator_set downsideAcc_;
+        class accumulator_set;
+        class downside_accumulator_set;
+        accumulator_set* acc_;


I'd go for unique_ptr otherwise you need a copy constructor and operator for this to be safe.

lballabio · 2023-09-23T17:27:38Z

ql/time/date.hpp

-#ifndef QL_HIGH_RESOLUTION_DATE
-    // inline definitions
-
-    inline Weekday Date::weekday() const {


What was the effect of these on compile time?

pcaspers · 2023-09-23T17:42:20Z

A few comments. Also, instead of optimizing single headers, I'd look into unity builds if I were you. You can enable them automatically in VC++ and CMake, and our configure script supports them as well. It would save a lot more time with a lot less work, and might make these changes less relevant.

Yes I wasn't so sure about these changes because of the relatively little gain anyhow. I'll try the unity build, that's a good idea.

pcaspers · 2023-09-23T18:08:36Z

@lballabio we are using ccache in our internal CI and we have frequent builds with relatively few files changed. Do you have experience how this plays together with a unity build?

lballabio · 2023-09-24T15:33:21Z

I think that as long as you don't add or remove files, the unity builds always compile together the same ones, so ccache should keep working. At least that should hold for our configure-based unity build, which merges together all files in the same directory and gives the resulting big file the same name.

pcaspers · 2023-09-25T06:47:45Z

Ok I'll check. Thank you!

pcaspers · 2023-09-28T11:03:41Z

I got the unity build working for ORE. I see a reduction from 50 min -> 32 min which is much better than the gain from the single optimizations here.

github-actions · 2023-12-01T01:51:57Z

This PR was automatically marked as stale because it has been open 60 days with no activity. Remove stale label or comment, or this will be closed in two weeks.

pcaspers · 2023-12-07T09:36:07Z

superseded by precompiled header usage, at least on "our" end (ORE build)

simplify observable.hpp

29eede9

pcaspers marked this pull request as draft September 18, 2023 14:41

simplify errors.hpp

a13a449

pcaspers added 2 commits September 19, 2023 17:33

move implementations from hpp to cpp

2314d49

move stuff from hpp to cpp for instrument.hpp, array.hpp, date.hpp, c…

50788d3

…alendar.hpp, matrix.hpp

pcaspers added 9 commits September 22, 2023 19:44

move the thread-safe part in observable

144ca0b

update make files

7b2f110

missing includes for single-header compile

5cc373b

update windows build

9027ce2

missing include

f602c52

missing return type

0b7ff2a

fix

1a17a42

move accumulator template instantiation to cpp

8a971b7

init members

17bf5e8

lballabio reviewed Sep 23, 2023

View reviewed changes

pcaspers changed the title ~~optimize build time~~ optimize build time (move stuff from hpp to cpp) Oct 1, 2023

pcaspers mentioned this pull request Oct 1, 2023

optimize build time (precompiled headers) #1800

Closed

github-actions bot added the stale label Dec 1, 2023

pcaspers closed this Dec 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

optimize build time (move stuff from hpp to cpp) #1787

optimize build time (move stuff from hpp to cpp) #1787

pcaspers commented Sep 18, 2023 •

edited

Loading

coveralls commented Sep 18, 2023

lballabio commented Sep 18, 2023

sweemer commented Sep 19, 2023

pcaspers commented Sep 19, 2023

lballabio commented Sep 19, 2023

pcaspers commented Sep 19, 2023

pcaspers commented Sep 19, 2023

pcaspers commented Sep 19, 2023

pcaspers commented Sep 19, 2023

pcaspers commented Sep 22, 2023

pcaspers commented Sep 22, 2023

lballabio left a comment

lballabio Sep 23, 2023

pcaspers Sep 23, 2023

lballabio Sep 23, 2023

lballabio Sep 23, 2023

lballabio Sep 23, 2023

pcaspers Sep 23, 2023

lballabio Sep 23, 2023

pcaspers commented Sep 23, 2023

pcaspers commented Sep 23, 2023

lballabio commented Sep 24, 2023

pcaspers commented Sep 25, 2023

pcaspers commented Sep 28, 2023

github-actions bot commented Dec 1, 2023

pcaspers commented Dec 7, 2023

optimize build time (move stuff from hpp to cpp) #1787

optimize build time (move stuff from hpp to cpp) #1787

Conversation

pcaspers commented Sep 18, 2023 • edited Loading

coveralls commented Sep 18, 2023

lballabio commented Sep 18, 2023

sweemer commented Sep 19, 2023

pcaspers commented Sep 19, 2023

lballabio commented Sep 19, 2023

pcaspers commented Sep 19, 2023

pcaspers commented Sep 19, 2023

pcaspers commented Sep 19, 2023

pcaspers commented Sep 19, 2023

pcaspers commented Sep 22, 2023

pcaspers commented Sep 22, 2023

lballabio left a comment

Choose a reason for hiding this comment

lballabio Sep 23, 2023

Choose a reason for hiding this comment

pcaspers Sep 23, 2023

Choose a reason for hiding this comment

lballabio Sep 23, 2023

Choose a reason for hiding this comment

lballabio Sep 23, 2023

Choose a reason for hiding this comment

lballabio Sep 23, 2023

Choose a reason for hiding this comment

pcaspers Sep 23, 2023

Choose a reason for hiding this comment

lballabio Sep 23, 2023

Choose a reason for hiding this comment

pcaspers commented Sep 23, 2023

pcaspers commented Sep 23, 2023

lballabio commented Sep 24, 2023

pcaspers commented Sep 25, 2023

pcaspers commented Sep 28, 2023

github-actions bot commented Dec 1, 2023

pcaspers commented Dec 7, 2023

pcaspers commented Sep 18, 2023 •

edited

Loading