-
Notifications
You must be signed in to change notification settings - Fork 59
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
Reducing build time of openscenario_interpreter
by utilizing precompiled headers
#1370
base: master
Are you sure you want to change the base?
Conversation
Checklist for reviewers ☑️All references to "You" in the following text refer to the code reviewer.
|
"<nlohmann/json.hpp>" | ||
"<rclcpp/rclcpp.hpp>" | ||
"<traffic_simulator/api/api.hpp>" | ||
"<pugixml.hpp>" | ||
|
||
"<openscenario_interpreter/expression.hpp>" | ||
"<openscenario_interpreter/object.hpp>" | ||
"<openscenario_interpreter/pointer.hpp>" | ||
"<openscenario_interpreter/record.hpp>" | ||
"<openscenario_interpreter/scope.hpp>" | ||
"<openscenario_interpreter/simulator_core.hpp>") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please sort lexicographically, unless there is a technical reason why declaration order matters. Also, please remove blank lines.
I have confirmed that the openscenario_interpreter build is 3 minutes shorter in GitHub Actions as well. |
Description
Abstract
This pull request reduces the build time for
opendscenario_interpreter
by using precompiled headers.Background
In the current codebase, large header files were being parsed multiple times and cause longer build time. To address this, I used Clang's
-ftime-trace
to identify the headers that were taking the most time to parse and updatedCMakeLists.txt
to ensure they are precompiled.Details
In my environment, the build time of
openscenario_interpreter
was reduced from 9m 21s to 7m 59s with 12 parallel builds.References
N/A
Destructive Changes
N/A
Known Limitations
N/A