Details
- Reviewers
EricWF mclow.lists
Diff Detail
- Repository
- rCXX libc++
Event Timeline
test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp | ||
---|---|---|
12 | This test comes from Eric's D27402. The default-initialization of a const pool_options object actually depends on a DR, so I needed to add UNSUPPORTED: apple-clang-7. I don't know how to determine the list of all unsupporty targets. |
- Split up the unit tests.
- Refactor to shrink the memory layout of the resource object itself. Before this patch sizeof(unsynchronized_pool_resource)==56. After this patch sizeof(unsynchronized_pool_resource) == 40.
src/experimental/memory_resource.cpp | ||
---|---|---|
235 | I now see how to avoid doing this: instead of keeping a separate free-list per each chunk, I should keep one big free-list per pool, where blocks from all chunks in the same pool are mixed together. This is okay because we never deallocate one chunk unless we're deallocating *all* chunks. |
Bugfix and shrink {un,}synchronized_pool_resource.
The old implementation was severely broken in two ways:
- It accidentally sometimes trusted the user's bytes and align arguments to do_deallocate, which didn't match the bytes and alignment stored in the actual ad-hoc chunk header. The new test case catches this bug.
- It relied on pointer comparison (undefined behavior) to test whether the user-supplied pointer belonged to each chunk in turn. Fortunately this is unnecessary. The new code follows Boost.Container by keeping just one list of "vacancies" per pool (not one per chunk, as I'd been doing). The new code is faster (deallocation is now O(1) not O(chunks-in-this-pool)), smaller (there are O(pools) free-list pointers instead of O(chunks)), and avoids undefined behavior.
Implement similar cosmetic cleanup to D47111, but for the pool resources this time.
I think the argument for keeping do_allocate and do_deallocate in the .cpp file is stronger for these guys than for monotonic_buffer_resource (where Eric convinced me to move them into the .h file for the benefit of the inliner). However, I'm willing to be persuaded (as I was in that case).
Rebased. Added _NOEXCEPT to upstream_resource() and options() (this is OK per [res.on.exception.handling]/5).
Rebased on D47111.
Replaced all instances of __release with __release_ptr, to match what @EricWF did in https://github.com/llvm-mirror/libcxx/commit/8e365750a0299ee0cbef70a3f3492c7716f81d5b under the commit message "Avoid name conflict with kernel headers."
That's fine, but then we should have a test for that.
We have the macro LIBCPP_ASSERT_NOEXCEPT specifically for this purpose (libc++ - specific noexcept markings).
I now see how to avoid doing this: instead of keeping a separate free-list per each chunk, I should keep one big free-list per pool, where blocks from all chunks in the same pool are mixed together. This is okay because we never deallocate one chunk unless we're deallocating *all* chunks.
I'll rework the patch to eliminate this undefined behavior and keep a free-list per pool.