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

Feature: pool allocator #66

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
Open

Feature: pool allocator #66

wants to merge 13 commits into from

Conversation

liss-h
Copy link
Contributor

@liss-h liss-h commented Dec 12, 2024

Adds a pool/arena allocator with a number of arenas for different allocation sizes (number and allocation sizes determined at compile time)

@liss-h liss-h marked this pull request as ready for review January 16, 2025 12:23
@liss-h liss-h marked this pull request as draft January 16, 2025 12:23
@liss-h liss-h marked this pull request as ready for review January 16, 2025 12:45
@liss-h liss-h requested a review from nkaralis January 16, 2025 12:52
Copy link

@nkaralis nkaralis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR. I have some questions and comments.

pool<bucket_sizes...> *pool_;

public:
explicit constexpr pool_allocator(pool<bucket_sizes...> &parent_pool) noexcept

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the allocator create its own pool (instead of relying on one to be constructed)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could use a shared_ptr but it cannot exclusively own the pool, we need multiple allocator instances (of different types, e.g. pool_allocator<int> pool_allocator<long>) to allocate in the same pool.

This is for std container purposes and our own usecase of allocating multiple different node types.

I was pondering if this makes sense but ultimately decided to use a similar design to what metall uses.

I'm still somewhat unsure as the shared_ptr design would indeed be more convenient to use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was basing this design on metall, but I guess they are only doing it this way because they are working with persistent memory. So maybe the shared ptr design makes more sense afterall

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bigerl opinion?

Pros of current design: best perf (although a refcount increase per copy is really not that expensive)
Cons of current design: easy UB source, inconvenient to use (you always need to take care to keep the pool alive)

tests/tests_pool_allocator.cpp Show resolved Hide resolved
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.

2 participants