* [PATCH] Improve handling of pool_options::largest_required_pool_block
@ 2018-11-13 22:59 Jonathan Wakely
2018-11-13 23:44 ` Jonathan Wakely
0 siblings, 1 reply; 2+ messages in thread
From: Jonathan Wakely @ 2018-11-13 22:59 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1133 bytes --]
Make the munge_options function round the largest_required_pool_block
value to a multiple of the smallest pool size (currently 8 bytes) to
avoid pools with odd sizes.
Ensure there is a pool large enough for blocks of the requested size.
Previously when largest_required_pool_block was exactly equal to one of
the pool_sizes[] values there would be no pool of that size. This patch
increases _M_npools by one, so there is a pool at least as large as the
requested value. It also reduces the size of the largest pool to be no
larger than needed.
* src/c++17/memory_resource.cc (munge_options): Round up value of
largest_required_pool_block to multiple of smallest pool size. Round
excessively large values down to largest pool size.
(select_num_pools): Increase number of pools by one unless it exactly
matches requested largest_required_pool_block.
(__pool_resource::_M_alloc_pools()): Make largest pool size equal
largest_required_pool_block.
* testsuite/20_util/unsynchronized_pool_resource/options.cc: Check
that pool_options::largest_required_pool_block is set appropriately.
Tested x86_64-linux, committed to trunk.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 7307 bytes --]
commit dbf2d43429e86dc2aa9b158520d154e5637e2f6e
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Mon Nov 12 22:56:53 2018 +0000
Improve handling of pool_options::largest_required_pool_block
Make the munge_options function round the largest_required_pool_block
value to a multiple of the smallest pool size (currently 8 bytes) to
avoid pools with odd sizes.
Ensure there is a pool large enough for blocks of the requested size.
Previously when largest_required_pool_block was exactly equal to one of
the pool_sizes[] values there would be no pool of that size. This patch
increases _M_npools by one, so there is a pool at least as large as the
requested value. It also reduces the size of the largest pool to be no
larger than needed.
* src/c++17/memory_resource.cc (munge_options): Round up value of
largest_required_pool_block to multiple of smallest pool size. Round
excessively large values down to largest pool size.
(select_num_pools): Increase number of pools by one unless it exactly
matches requested largest_required_pool_block.
(__pool_resource::_M_alloc_pools()): Make largest pool size equal
largest_required_pool_block.
* testsuite/20_util/unsynchronized_pool_resource/options.cc: Check
that pool_options::largest_required_pool_block is set appropriately.
diff --git a/libstdc++-v3/src/c++17/memory_resource.cc b/libstdc++-v3/src/c++17/memory_resource.cc
index 719cb9f1d29..691a2999de6 100644
--- a/libstdc++-v3/src/c++17/memory_resource.cc
+++ b/libstdc++-v3/src/c++17/memory_resource.cc
@@ -830,6 +830,19 @@ namespace pmr
namespace {
+ constexpr size_t pool_sizes[] = {
+ 8, 16, 24,
+ 32, 48,
+ 64, 80, 96, 112,
+ 128, 192,
+ 256, 320, 384, 448,
+ 512, 768,
+ 1024, 1536,
+ 2048, 3072,
+ 1<<12, 1<<13, 1<<14, 1<<15, 1<<16, 1<<17,
+ 1<<20, 1<<21, 1<<22 // 4MB should be enough for anybody
+ };
+
pool_options
munge_options(pool_options opts)
{
@@ -860,29 +873,25 @@ namespace pmr
}
else
{
- // TODO round to preferred granularity ?
+ // Round to preferred granularity
+ static_assert(std::__ispow2(pool_sizes[0]));
+ constexpr size_t mask = pool_sizes[0] - 1;
+ opts.largest_required_pool_block += mask;
+ opts.largest_required_pool_block &= ~mask;
}
if (opts.largest_required_pool_block < big_block::min)
{
opts.largest_required_pool_block = big_block::min;
}
+ else if (opts.largest_required_pool_block > std::end(pool_sizes)[-1])
+ {
+ // Setting _M_opts to the largest pool allows users to query it:
+ opts.largest_required_pool_block = std::end(pool_sizes)[-1];
+ }
return opts;
}
- const size_t pool_sizes[] = {
- 8, 16, 24,
- 32, 48,
- 64, 80, 96, 112,
- 128, 192,
- 256, 320, 384, 448,
- 512, 768,
- 1024, 1536,
- 2048, 3072,
- 1<<12, 1<<13, 1<<14, 1<<15, 1<<16, 1<<17,
- 1<<20, 1<<21, 1<<22 // 4MB should be enough for anybody
- };
-
inline int
pool_index(size_t block_size, int npools)
{
@@ -898,9 +907,10 @@ namespace pmr
{
auto p = std::lower_bound(std::begin(pool_sizes), std::end(pool_sizes),
opts.largest_required_pool_block);
- if (int npools = p - std::begin(pool_sizes))
- return npools;
- return 1;
+ const int n = p - std::begin(pool_sizes);
+ if (p == std::end(pool_sizes) || *p == opts.largest_required_pool_block)
+ return n;
+ return n + 1;
}
} // namespace
@@ -971,7 +981,11 @@ namespace pmr
_Pool* p = alloc.allocate(_M_npools);
for (int i = 0; i < _M_npools; ++i)
{
- const size_t block_size = pool_sizes[i];
+ // For last pool use largest_required_pool_block
+ const size_t block_size = (i+1 == _M_npools)
+ ? _M_opts.largest_required_pool_block
+ : pool_sizes[i];
+
// Decide on initial number of blocks per chunk.
// Always have at least 16 blocks per chunk:
const size_t min_blocks_per_chunk = 16;
diff --git a/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/options.cc b/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/options.cc
index bfa8a8ce23c..a3e4c44aff0 100644
--- a/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/options.cc
+++ b/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/options.cc
@@ -20,6 +20,13 @@
#include <memory_resource>
#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+
+bool eq(const std::pmr::pool_options& lhs, const std::pmr::pool_options& rhs)
+{
+ return lhs.max_blocks_per_chunk == rhs.max_blocks_per_chunk
+ && lhs.largest_required_pool_block == rhs.largest_required_pool_block;
+}
void
test01()
@@ -30,13 +37,62 @@ test01()
VERIFY( opts.largest_required_pool_block != 0 );
std::pmr::unsynchronized_pool_resource r1(opts);
- auto [max_blocks_per_chunk, largest_required_pool_block ] = r1.options();
- VERIFY( max_blocks_per_chunk == opts.max_blocks_per_chunk );
- VERIFY( largest_required_pool_block == opts.largest_required_pool_block );
+ const auto opts1 = r1.options();
+ VERIFY( eq(opts, opts1) );
+
+ std::pmr::unsynchronized_pool_resource r2(std::pmr::pool_options{0, 0});
+ const auto opts2 = r2.options();
+ VERIFY( eq(opts, opts2) );
+}
+
+void
+test02()
+{
+ std::pmr::pool_options opts{0, 0};
+ std::size_t num_allocs = 0;
+
+ __gnu_test::memory_resource test_mr;
+
+ std::pmr::unsynchronized_pool_resource r1(opts, &test_mr);
+ opts = r1.options();
+ // opts.largest_required_pool_block should be set to the block size of
+ // the largest pool (this is a GNU extension). Confirm this by checking
+ // that allocations larger than opts.largest_required_pool_block come
+ // directly from the upstream allocator, test_mr, not from r1's pools.
+
+ // The following should result in a "large" allocation direct from upstream:
+ (void) r1.allocate(opts.largest_required_pool_block + 1);
+ num_allocs = test_mr.number_of_active_allocations();
+ // This should result in another "large" allocation direct from upstream:
+ (void) r1.allocate(opts.largest_required_pool_block + 1);
+ // Which means the number of upstream allocations should have increased:
+ VERIFY( test_mr.number_of_active_allocations() > num_allocs );
+ r1.release();
+
+ // Repeat with a user-specified block size:
+ opts.largest_required_pool_block = 64;
+ std::pmr::unsynchronized_pool_resource r2(opts, &test_mr);
+ opts = r2.options();
+ (void) r2.allocate(opts.largest_required_pool_block + 1);
+ num_allocs = test_mr.number_of_active_allocations();
+ (void) r2.allocate(opts.largest_required_pool_block + 1);
+ VERIFY( test_mr.number_of_active_allocations() > num_allocs );
+ r2.release();
+
+ // Repeat with an odd user-specified block size:
+ opts.largest_required_pool_block = 71;
+ std::pmr::unsynchronized_pool_resource r3(opts, &test_mr);
+ opts = r3.options();
+ (void) r3.allocate(opts.largest_required_pool_block + 1);
+ num_allocs = test_mr.number_of_active_allocations();
+ (void) r3.allocate(opts.largest_required_pool_block + 1);
+ VERIFY( test_mr.number_of_active_allocations() > num_allocs );
+ r3.release();
}
int
main()
{
test01();
+ test02();
}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Improve handling of pool_options::largest_required_pool_block
2018-11-13 22:59 [PATCH] Improve handling of pool_options::largest_required_pool_block Jonathan Wakely
@ 2018-11-13 23:44 ` Jonathan Wakely
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Wakely @ 2018-11-13 23:44 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
On 13/11/18 22:59 +0000, Jonathan Wakely wrote:
>@@ -898,9 +907,10 @@ namespace pmr
> {
> auto p = std::lower_bound(std::begin(pool_sizes), std::end(pool_sizes),
> opts.largest_required_pool_block);
>- if (int npools = p - std::begin(pool_sizes))
>- return npools;
>- return 1;
>+ const int n = p - std::begin(pool_sizes);
>+ if (p == std::end(pool_sizes) || *p == opts.largest_required_pool_block)
This is wrong, it still chooses one pool too few when the block_size
matches an element of pool_sizes[].
>+ return n;
>+ return n + 1;
> }
Fixed by this patch, tested x86_64-linux and committed to trunk.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 938 bytes --]
commit 01882ab88871d2bcb34ad141505b96b317eefccb
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue Nov 13 23:26:08 2018 +0000
Fix error when selecting number of memory pools
* src/c++17/memory_resource.cc (select_num_pools): Fix off-by-one
error when block_size is equal to one of the values in the array.
diff --git a/libstdc++-v3/src/c++17/memory_resource.cc b/libstdc++-v3/src/c++17/memory_resource.cc
index cb91e5147ce..605bdd53950 100644
--- a/libstdc++-v3/src/c++17/memory_resource.cc
+++ b/libstdc++-v3/src/c++17/memory_resource.cc
@@ -892,7 +892,7 @@ namespace pmr
auto p = std::lower_bound(std::begin(pool_sizes), std::end(pool_sizes),
opts.largest_required_pool_block);
const int n = p - std::begin(pool_sizes);
- if (p == std::end(pool_sizes) || *p == opts.largest_required_pool_block)
+ if (p == std::end(pool_sizes))
return n;
return n + 1;
}
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-11-13 23:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-13 22:59 [PATCH] Improve handling of pool_options::largest_required_pool_block Jonathan Wakely
2018-11-13 23:44 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).