-
Notifications
You must be signed in to change notification settings - Fork 119
Improve post-teardown performance #810
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
base: main
Are you sure you want to change the base?
Changes from all commits
657bc67
bbd7edc
ecf4a5c
1680012
e0c80df
898e77d
52cf718
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,7 +93,7 @@ namespace snmalloc | |
| // we need to record if we are already in that state as we will not | ||
| // receive another teardown call, so each operation needs to release | ||
| // the underlying data structures after the call. | ||
| static inline thread_local bool teardown_called{false}; | ||
| static inline thread_local size_t times_teardown_called{0}; | ||
|
|
||
| public: | ||
| /** | ||
|
|
@@ -114,8 +114,9 @@ namespace snmalloc | |
| if (alloc == &default_alloc) | ||
| return; | ||
|
|
||
| teardown_called = true; | ||
| alloc->flush(); | ||
| times_teardown_called++; | ||
| if (bits::is_pow2(times_teardown_called) || times_teardown_called < 128) | ||
| alloc->flush(); | ||
| AllocPool<Config>::release(alloc); | ||
| alloc = const_cast<Alloc*>(&default_alloc); | ||
|
Comment on lines
+117
to
121
|
||
| } | ||
|
|
@@ -131,7 +132,7 @@ namespace snmalloc | |
| template<typename Restart, typename... Args> | ||
| SNMALLOC_SLOW_PATH static auto check_init_slow(Restart r, Args... args) | ||
| { | ||
| bool post_teardown = teardown_called; | ||
| bool post_teardown = times_teardown_called > 0; | ||
|
|
||
| alloc = AllocPool<Config>::acquire(); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| #include <cstdlib> | ||
| #include <snmalloc/snmalloc.h> | ||
| #include <test/measuretime.h> | ||
| #include <test/setup.h> | ||
| #include <vector> | ||
|
|
||
| using namespace snmalloc; | ||
|
|
||
| void fill(std::vector<void*>& out, size_t count, size_t size) | ||
| { | ||
| out.reserve(count); | ||
| for (size_t i = 0; i < count; i++) | ||
| { | ||
| out.push_back(snmalloc::alloc<Uninit>(size)); | ||
| } | ||
| } | ||
|
|
||
| void drain(const char* label, std::vector<void*>& vec, size_t size) | ||
| { | ||
| MeasureTime m; | ||
| m << label << " (" << vec.size() << " x " << size << " B)"; | ||
| for (void* p : vec) | ||
| { | ||
| snmalloc::dealloc(p, size); | ||
| } | ||
| vec.clear(); | ||
| } | ||
|
|
||
| int main(int, char**) | ||
| { | ||
| setup(); | ||
| // Issue #809: perf when many objects are freed after the allocator has | ||
| // already been finalised (e.g. static/global teardown). Keep counts equal | ||
| // for baseline and post-teardown to isolate the teardown cost. | ||
| constexpr size_t alloc_count = 1 << 18; | ||
| constexpr size_t obj_size = 64; | ||
|
|
||
| std::vector<void*> ptrs; | ||
| fill(ptrs, alloc_count, obj_size); | ||
| drain("Baseline dealloc before finalise", ptrs, obj_size); | ||
|
|
||
| // Simulate the allocator already being torn down before remaining frees | ||
| // (post-main / static destruction path from #809). | ||
| ThreadAlloc::teardown(); | ||
|
|
||
| fill(ptrs, alloc_count, obj_size); | ||
| drain("Immediate dealloc after teardown", ptrs, obj_size); | ||
|
|
||
| return 0; | ||
| } |
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.
When
SNMALLOC_PTHREAD_FORK_PROTECTIONis enabled,ensure_init()only callspthread_atforkunder#ifdef SNMALLOC_PTHREAD_ATFORK_WORKS. If that macro is not consistently defined by the build (and it currently isn’t exported from CMake), fork protection silently becomes a no-op even though the option is ON. Consider making thepthread_atforkpath depend solely onSNMALLOC_PTHREAD_FORK_PROTECTION(given the CMake fatal-error guard), or ensure theSNMALLOC_PTHREAD_ATFORK_WORKSdefine is always provided when detected.