-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Unify SystemPool and lib/support/Pool #9590
Conversation
0253154
to
0ffbb6d
Compare
4d4ffc1
to
48189c8
Compare
34d4344
to
d112201
Compare
Size increase report for "gn_qpg-example-build" from 8e86c9e
Full report output
|
Size increase report for "nrfconnect-example-build" from 8e86c9e
Full report output
|
Size increase report for "esp32-example-build" from 8e86c9e
Full report output
|
@kghost - conflicts |
Is this superseded by #9748? It appears that PR already includes most or all of this content. If so, we should close this or at least mark it for no-merge. |
template <typename... Args> | ||
T * CreateObject(Args &&... args) | ||
{ | ||
std::lock_guard<std::recursive_mutex> lock(mutex); |
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.
No, we should not change std::mutex to std::recursive_mutex, we already have consensus before not using recursive_mutex in our SDK, this will hide some problems which is extremely difficult to debug
|
||
void ReleaseObject(T * object) | ||
{ | ||
std::lock_guard<std::recursive_mutex> lock(mutex); |
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.
We should use std::mutex instead of std::recursive_mutex
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
This stale pull request has been automatically closed. Thank you for your contributions. |
#### Problem We have too many pool allocators. Previous PRs (project-chip#11428, project-chip#11487) transitionally use `BitMapObjectPool` where previously `System::ObjectPool` had been used, but this lost the ability to configure heap allocation. #### Change overview - Add a heap allocator (from project-chip#9590) - Add allocation selection (from project-chip#11371) - Use this for `System::Timer` (complementing project-chip#11487) Co-authored-by: Zang MingJie <[email protected]> Co-authored-by: C Freeman <[email protected]> A future PR will use this for Inet pools (complementing project-chip#11428); that is not done here because it would conflict with other Inet changes under way. #### Testing CI; no changes to functionality. CI should show `.bss` decreases corresponding to increases in project-chip#11487.
#### Problem We have too many pool allocators. Previous PRs (project-chip#11428, project-chip#11487) transitionally use `BitMapObjectPool` where previously `System::ObjectPool` had been used, but this lost the ability to configure heap allocation. #### Change overview - Add a heap allocator (from project-chip#9590) - Add allocation selection (from project-chip#11371) - Use this for `System::Timer` (complementing project-chip#11487) Co-authored-by: Zang MingJie <[email protected]> Co-authored-by: C Freeman <[email protected]> A future PR will use this for Inet pools (complementing project-chip#11428); that is not done here because it would conflict with other Inet changes under way. #### Testing Added heap versions of unit tests in TestPool. (A future PR will add `System::Object`-style statistics and re-unify most of these tests.) CI should show `.bss` decreases corresponding to increases in project-chip#11487.
#### Problem We have too many pool allocators. Previous PRs (project-chip#11428, project-chip#11487) transitionally use `BitMapObjectPool` where previously `System::ObjectPool` had been used, but this lost the ability to configure heap allocation. #### Change overview - Add a heap allocator (from project-chip#9590) - Add allocation selection (from project-chip#11371) - Use this for `System::Timer` (complementing project-chip#11487) Co-authored-by: Zang MingJie <[email protected]> Co-authored-by: C Freeman <[email protected]> A future PR will use this for Inet pools (complementing project-chip#11428); that is not done here because it would conflict with other Inet changes under way. #### Testing Added heap versions of unit tests in TestPool. (A future PR will add `System::Object`-style statistics and re-unify most of these tests.) CI should show `.bss` decreases corresponding to increases in project-chip#11487.
#### Problem We have too many pool allocators. Previous PRs (project-chip#11428, project-chip#11487) transitionally use `BitMapObjectPool` where previously `System::ObjectPool` had been used, but this lost the ability to configure heap allocation. #### Change overview - Add a heap allocator (from project-chip#9590) - Add allocation selection (from project-chip#11371) - Use this for `System::Timer` (complementing project-chip#11487) Co-authored-by: Zang MingJie <[email protected]> Co-authored-by: C Freeman <[email protected]> A future PR will use this for Inet pools (complementing project-chip#11428); that is not done here because it would conflict with other Inet changes under way. #### Testing Added heap versions of unit tests in TestPool. (A future PR will add `System::Object`-style statistics and re-unify most of these tests.) CI should show `.bss` decreases corresponding to increases in project-chip#11487.
#### Problem We have too many pool allocators. Previous PRs (project-chip#11428, project-chip#11487) transitionally use `BitMapObjectPool` where previously `System::ObjectPool` had been used, but this lost the ability to configure heap allocation. #### Change overview - Add a heap allocator (from project-chip#9590) - Add allocation selection (from project-chip#11371) - Use this for `System::Timer` (complementing project-chip#11487) Co-authored-by: Zang MingJie <[email protected]> Co-authored-by: C Freeman <[email protected]> A future PR will use this for Inet pools (complementing project-chip#11428); that is not done here because it would conflict with other Inet changes under way. #### Testing Added heap versions of unit tests in TestPool. (A future PR will add `System::Object`-style statistics and re-unify most of these tests.) CI should show `.bss` decreases corresponding to increases in project-chip#11487.
#### Problem We have too many pool allocators. Previous PRs (#11428, #11487) transitionally use `BitMapObjectPool` where previously `System::ObjectPool` had been used, but this lost the ability to configure heap allocation. #### Change overview - Add a heap allocator (from #9590) - Add allocation selection (from #11371) - Use this for `System::Timer` (complementing #11487) - Factor out common code. - Use a heap-allocated list to track heap-allocated objects. - More unit tests. Co-authored-by: Zang MingJie <[email protected]> Co-authored-by: C Freeman <[email protected]> A future PR will use this for Inet pools (complementing #11428); that is not done here because it would conflict with other Inet changes under way. #### Testing Added heap versions of unit tests in TestPool. (A future PR will add `System::Object`-style statistics and re-unify most of these tests.) CI should show `.bss` decreases corresponding to increases in #11487.
Problem
We have too many pool implementations
Change overview
Unify the interface of system/SystemPool and lib/support/pool. This PR change the interface of SystemPool to match lib/support/Pool. The pool provides following interface functions:
And also provides statistics interface:
Following features are decoupled from the pool implementation:
DeferredRelease
: this feature should be orthogonal to pool implementation, the target class should implement it by itself.Files changes:
Removed
Implementation on platforms with heap (POSIX):
Implementation on platforms w/o heap (Embedded):
Statistics:
Select implementation based on pre-defined macro
Note: this PR depends on
#9674 (merged)and #9678Testing
Verified on posix platform.
Next PR will remove lib/support/Pool and migrate all usage to SystemPool.