[gcc(refs/users/aoliva/heads/testme)] libstdc++: Make pmr::synchronized_pool_resource work without libpthread (PR 94936)
Alexandre Oliva
aoliva@gcc.gnu.org
Tue May 19 09:07:40 GMT 2020
https://gcc.gnu.org/g:ec40967f1323069da3a5a45286f71fa4f80926df
commit ec40967f1323069da3a5a45286f71fa4f80926df
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Mon May 4 13:34:23 2020 +0100
libstdc++: Make pmr::synchronized_pool_resource work without libpthread (PR 94936)
I implicitly assumed that programs using pmr::synchronized_pool_resource
would also be using multiple threads, and so the weak symbols in
gthr-posix.h would be resolved by linking to libpthread. If that isn't
true then it crashes when trying to use pthread_key_create.
This commit makes the pool resource check __gthread_active_p() before
using thread-specific data, and just use a single set of memory pools
when there's only a single thread.
PR libstdc++/94936
* src/c++17/memory_resource.cc (synchronized_pool_resource::_TPools):
Add comment about single-threaded behaviour.
(synchronized_pool_resource::_TPools::move_nonempty_chunks()): Hoist
class member access out of loop.
(synchronized_pool_resource::synchronized_pool_resource())
(synchronized_pool_resource::~synchronized_pool_resource())
(synchronized_pool_resource::release()): Check __gthread_active_p
before creating and/or deleting the thread-specific data key.
(synchronized_pool_resource::_M_thread_specific_pools()): Adjust
assertions.
(synchronized_pool_resource::do_allocate(size_t, size_t)): Add fast
path for single-threaded case.
(synchronized_pool_resource::do_deallocate(void*, size_t, size_t)):
Likewise. Return if unable to find a pool that owns the allocation.
* testsuite/20_util/synchronized_pool_resource/allocate_single.cc:
New test.
* testsuite/20_util/synchronized_pool_resource/cons_single.cc: New
test.
* testsuite/20_util/synchronized_pool_resource/release_single.cc: New
test.
Diff:
---
libstdc++-v3/ChangeLog | 24 ++++
libstdc++-v3/src/c++17/memory_resource.cc | 127 ++++++++++++++-------
.../synchronized_pool_resource/allocate_single.cc | 24 ++++
.../synchronized_pool_resource/cons_single.cc | 24 ++++
.../synchronized_pool_resource/release_single.cc | 24 ++++
5 files changed, 179 insertions(+), 44 deletions(-)
diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 7f11117365e..0624bb733dd 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,27 @@
+2020-05-04 Jonathan Wakely <jwakely@redhat.com>
+
+ PR libstdc++/94936
+ * src/c++17/memory_resource.cc (synchronized_pool_resource::_TPools):
+ Add comment about single-threaded behaviour.
+ (synchronized_pool_resource::_TPools::move_nonempty_chunks()): Hoist
+ class member access out of loop.
+ (synchronized_pool_resource::synchronized_pool_resource())
+ (synchronized_pool_resource::~synchronized_pool_resource())
+ (synchronized_pool_resource::release()): Check __gthread_active_p
+ before creating and/or deleting the thread-specific data key.
+ (synchronized_pool_resource::_M_thread_specific_pools()): Adjust
+ assertions.
+ (synchronized_pool_resource::do_allocate(size_t, size_t)): Add fast
+ path for single-threaded case.
+ (synchronized_pool_resource::do_deallocate(void*, size_t, size_t)):
+ Likewise. Return if unable to find a pool that owns the allocation.
+ * testsuite/20_util/synchronized_pool_resource/allocate_single.cc:
+ New test.
+ * testsuite/20_util/synchronized_pool_resource/cons_single.cc: New
+ test.
+ * testsuite/20_util/synchronized_pool_resource/release_single.cc: New
+ test.
+
2020-05-03 Jonathan Wakely <jwakely@redhat.com>
PR libstdc++/94933
diff --git a/libstdc++-v3/src/c++17/memory_resource.cc b/libstdc++-v3/src/c++17/memory_resource.cc
index 56a87844da0..1acab19e306 100644
--- a/libstdc++-v3/src/c++17/memory_resource.cc
+++ b/libstdc++-v3/src/c++17/memory_resource.cc
@@ -1043,7 +1043,8 @@ namespace pmr
* exposition as _M_tpools[_M_key]).
* The first element, _M_tpools[0], contains "orphaned chunks" which were
* allocated by a thread which has since exited, and so there is no
- * _M_tpools[_M_key] for that thread.
+ * _M_tpools[_M_key] for that thread. Orphaned chunks are never reused,
+ * they're only held in _M_tpools[0] so they can be deallocated.
* A thread can access its own thread-specific set of pools via _M_key
* while holding a shared lock on _M_mx. Accessing _M_impl._M_unpooled
* or _M_tpools[0] or any other thread's _M_tpools[_M_key] requires an
@@ -1052,6 +1053,10 @@ namespace pmr
* any dereference of that pointer requires an exclusive lock.
* The _M_impl._M_opts and _M_impl._M_npools members are immutable,
* and can safely be accessed concurrently.
+ *
+ * In a single-threaded program (i.e. __gthread_active_p() == false)
+ * the pool resource only needs one set of pools and never has orphaned
+ * chunks, so just uses _M_tpools[0] directly, and _M_tpools->next is null.
*/
extern "C" {
@@ -1092,14 +1097,16 @@ namespace pmr
void move_nonempty_chunks()
{
__glibcxx_assert(pools);
+ __glibcxx_assert(__gthread_active_p());
if (!pools)
return;
- memory_resource* r = owner.upstream_resource();
+ memory_resource* const r = owner.upstream_resource();
+ auto* const shared = owner._M_tpools->pools;
// move all non-empty chunks to the shared _TPools
for (int i = 0; i < owner._M_impl._M_npools; ++i)
for (auto& c : pools[i]._M_chunks)
if (!c.empty())
- owner._M_tpools->pools[i]._M_chunks.insert(std::move(c), r);
+ shared[i]._M_chunks.insert(std::move(c), r);
}
synchronized_pool_resource& owner;
@@ -1133,8 +1140,9 @@ namespace pmr
memory_resource* upstream)
: _M_impl(opts, upstream)
{
- if (int err = __gthread_key_create(&_M_key, destroy_TPools))
- __throw_system_error(err);
+ if (__gthread_active_p())
+ if (int err = __gthread_key_create(&_M_key, destroy_TPools))
+ __throw_system_error(err);
exclusive_lock l(_M_mx);
_M_tpools = _M_alloc_shared_tpools(l);
}
@@ -1143,7 +1151,8 @@ namespace pmr
synchronized_pool_resource::~synchronized_pool_resource()
{
release();
- __gthread_key_delete(_M_key); // does not run destroy_TPools
+ if (__gthread_active_p())
+ __gthread_key_delete(_M_key); // does not run destroy_TPools
}
void
@@ -1152,8 +1161,11 @@ namespace pmr
exclusive_lock l(_M_mx);
if (_M_tpools)
{
- __gthread_key_delete(_M_key); // does not run destroy_TPools
- __gthread_key_create(&_M_key, destroy_TPools);
+ if (__gthread_active_p())
+ {
+ __gthread_key_delete(_M_key); // does not run destroy_TPools
+ __gthread_key_create(&_M_key, destroy_TPools);
+ }
polymorphic_allocator<_TPools> a(upstream_resource());
// destroy+deallocate each _TPools
do
@@ -1175,10 +1187,11 @@ namespace pmr
synchronized_pool_resource::_M_thread_specific_pools() noexcept
{
__pool_resource::_Pool* pools = nullptr;
+ __glibcxx_assert(__gthread_active_p());
if (auto tp = static_cast<_TPools*>(__gthread_getspecific(_M_key)))
{
- pools = tp->pools;
- __glibcxx_assert(tp->pools);
+ pools = tp->pools;
+ // __glibcxx_assert(tp->pools);
}
return pools;
}
@@ -1189,24 +1202,34 @@ namespace pmr
do_allocate(size_t bytes, size_t alignment)
{
const auto block_size = std::max(bytes, alignment);
- if (block_size <= _M_impl._M_opts.largest_required_pool_block)
+ const pool_options opts = _M_impl._M_opts;
+ if (block_size <= opts.largest_required_pool_block)
{
const ptrdiff_t index = pool_index(block_size, _M_impl._M_npools);
- memory_resource* const r = upstream_resource();
- const pool_options opts = _M_impl._M_opts;
- {
- // Try to allocate from the thread-specific pool
- shared_lock l(_M_mx);
- if (auto pools = _M_thread_specific_pools()) // [[likely]]
- {
- // Need exclusive lock to replenish so use try_allocate:
- if (void* p = pools[index].try_allocate())
- return p;
- // Need to take exclusive lock and replenish pool.
- }
- // Need to allocate or replenish thread-specific pools using
- // upstream resource, so need to hold exclusive lock.
- }
+ if (__gthread_active_p())
+ {
+ // Try to allocate from the thread-specific pool.
+ shared_lock l(_M_mx);
+ if (auto pools = _M_thread_specific_pools()) // [[likely]]
+ {
+ // Need exclusive lock to replenish so use try_allocate:
+ if (void* p = pools[index].try_allocate())
+ return p;
+ // Need to take exclusive lock and replenish pool.
+ }
+ // Need to allocate or replenish thread-specific pools using
+ // upstream resource, so need to hold exclusive lock.
+ }
+ else // single-threaded
+ {
+ if (!_M_tpools) // [[unlikely]]
+ {
+ exclusive_lock dummy(_M_mx);
+ _M_tpools = _M_alloc_shared_tpools(dummy);
+ }
+ return _M_tpools->pools[index].allocate(upstream_resource(), opts);
+ }
+
// N.B. Another thread could call release() now lock is not held.
exclusive_lock excl(_M_mx);
if (!_M_tpools) // [[unlikely]]
@@ -1214,7 +1237,7 @@ namespace pmr
auto pools = _M_thread_specific_pools();
if (!pools)
pools = _M_alloc_tpools(excl)->pools;
- return pools[index].allocate(r, opts);
+ return pools[index].allocate(upstream_resource(), opts);
}
exclusive_lock l(_M_mx);
return _M_impl.allocate(bytes, alignment); // unpooled allocation
@@ -1230,30 +1253,45 @@ namespace pmr
{
const ptrdiff_t index = pool_index(block_size, _M_impl._M_npools);
__glibcxx_assert(index != -1);
- {
- shared_lock l(_M_mx);
- auto pools = _M_thread_specific_pools();
- if (pools)
- {
- // No need to lock here, no other thread is accessing this pool.
- if (pools[index].deallocate(upstream_resource(), p))
- return;
- }
- // Block might have come from a different thread's pool,
- // take exclusive lock and check every pool.
- }
+ if (__gthread_active_p())
+ {
+ shared_lock l(_M_mx);
+ if (auto pools = _M_thread_specific_pools())
+ {
+ // No need to lock here, no other thread is accessing this pool.
+ if (pools[index].deallocate(upstream_resource(), p))
+ return;
+ }
+ // Block might have come from a different thread's pool,
+ // take exclusive lock and check every pool.
+ }
+ else // single-threaded
+ {
+ __glibcxx_assert(_M_tpools != nullptr);
+ if (_M_tpools) // [[likely]]
+ _M_tpools->pools[index].deallocate(upstream_resource(), p);
+ return;
+ }
+
// TODO store {p, bytes, alignment} somewhere and defer returning
// the block to the correct thread-specific pool until we next
// take the exclusive lock.
+
exclusive_lock excl(_M_mx);
+ auto my_pools = _M_thread_specific_pools();
for (_TPools* t = _M_tpools; t != nullptr; t = t->next)
{
- if (t->pools) // [[likely]]
- {
- if (t->pools[index].deallocate(upstream_resource(), p))
- return;
- }
+ if (t->pools != my_pools)
+ if (t->pools) // [[likely]]
+ {
+ if (t->pools[index].deallocate(upstream_resource(), p))
+ return;
+ }
}
+ // Not necessarily an error to reach here, release() could have been
+ // called on another thread between releasing the shared lock and
+ // acquiring the exclusive lock.
+ return;
}
exclusive_lock l(_M_mx);
_M_impl.deallocate(p, bytes, alignment);
@@ -1265,6 +1303,7 @@ namespace pmr
-> _TPools*
{
__glibcxx_assert(_M_tpools != nullptr);
+ __glibcxx_assert(__gthread_active_p());
// dump_list(_M_tpools);
polymorphic_allocator<_TPools> a(upstream_resource());
_TPools* p = a.allocate(1);
diff --git a/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/allocate_single.cc b/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/allocate_single.cc
new file mode 100644
index 00000000000..d5b7b162146
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/allocate_single.cc
@@ -0,0 +1,24 @@
+// Copyright (C) 2018-2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run }
+// { dg-options "-std=gnu++17" }
+// { dg-require-effective-target c++17 }
+// { dg-require-gthreads "" }
+
+// This runs the same tests as allocate.cc but without -pthread
+#include "./allocate.cc"
diff --git a/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/cons_single.cc b/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/cons_single.cc
new file mode 100644
index 00000000000..060cd7628e5
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/cons_single.cc
@@ -0,0 +1,24 @@
+// Copyright (C) 2018-2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run }
+// { dg-options "-std=gnu++17" }
+// { dg-require-effective-target c++17 }
+// { dg-require-gthreads "" }
+
+// This runs the same tests as cons.cc but without -pthread
+#include "./cons.cc"
diff --git a/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/release_single.cc b/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/release_single.cc
new file mode 100644
index 00000000000..32e80c1be3f
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/synchronized_pool_resource/release_single.cc
@@ -0,0 +1,24 @@
+// Copyright (C) 2018-2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run }
+// { dg-options "-std=gnu++17" }
+// { dg-require-effective-target c++17 }
+// { dg-require-gthreads "" }
+
+// This runs the same tests as release.cc but without -pthread
+#include "./release.cc"
More information about the Libstdc++-cvs
mailing list