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

Skip destructors within TrimUtil #120

Merged
merged 1 commit into from
Nov 24, 2024
Merged

Skip destructors within TrimUtil #120

merged 1 commit into from
Nov 24, 2024

Conversation

elliotgoodrich
Copy link
Owner

Based on benchmarks, running the destructor for all the objects in BuildContext takes 1-2% of the total time. Instead we can skip it entirely by extending the lifetime of this object until we call std::_Exit - ending the application and skipping destructing all objects on the stack.

Using a std::shared_ptr<void> avoids us having to define BuildContext in the header file and bring in a load more include directives. Another alternative is to make TrimUtil instantiable and have BuildContext as a (perhaps pimple) member variable. However, this would require us to either reset all members in BuildContext when we call trim, or to ban trim being called multiple times. Neither are as easy as the current chosen solution - even if it is a bit strange.

Based on benchmarks, running the destructor for all the objects in
`BuildContext` takes 1-2% of the total time.  Instead we can skip it
entirely by extending the lifetime of this object until we call
`std::_Exit` - ending the application and skipping destructing all
objects on the stack.

Using a `std::shared_ptr<void>` avoids us having to define
`BuildContext` in the header file and bring in a load more include
directives.  Another alternative is to make `TrimUtil` instantiable and
have `BuildContext` as a (perhaps pimple) member variable.  However,
this would require us to either reset all members in `BuildContext` when
we call `trim`, or to ban `trim` being called multiple times.  Neither
are as easy as the current chosen solution - even if it is a bit
strange.
@elliotgoodrich elliotgoodrich merged commit 603949f into main Nov 24, 2024
8 checks passed
@elliotgoodrich elliotgoodrich deleted the reduce-dtor-time branch November 24, 2024 20:37
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.

1 participant