public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [committed 1/5] libstdc++: Reduce memory usage in std::stacktrace::current
@ 2022-04-12 21:41 Jonathan Wakely
  2022-04-12 21:41 ` [committed 2/5] libstdc++: Use nothrow new in std::stacktrace Jonathan Wakely
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Jonathan Wakely @ 2022-04-12 21:41 UTC (permalink / raw)
  To: libstdc++, gcc-patches

Tested powerpc64-linux, pushed to trunk.

-- >8 --

This adds an alternative callback for use in the overload of
basic_stacktrace::current that takes a max_depth parameter. The new
callback will not allow the container to grow past the initial capacity,
which is set to the specified maximum depth.  This avoids allocating
memory for hundreds of frames only to discard them again because of a
small maximum depth limit.

For larger maximum depths the normal callback is used, with a smaller
initial capacity that can grow as needed. The container will be resized
to the given max depth after the entire backtrace has been produced
(relying on the fact that std::stacktrace_entry objects are trivially
destructible to elide their destruction).

Currently the value for "larger" limits is 128, so a max depth <= 128
will allocate capacity for exactly that many frames. A larger max depth
(or an unspecified max depth) will use an initial capacity of 64 frames
and grow as needed. Since each frame is only a uintptr_t value it might
be reasonable to increase the first value so that memory usage can be
capped for larger maximum depths.

This change also delays the creation of the libbacktrace state until we
actually need it, so that the state is not created if allocation fails.

libstdc++-v3/ChangeLog:

	* include/std/stacktrace (basic_stacktrace::current): Replace
	calls to _M_reserve and _S_curr_cb with call to _M_prepare.
	Check return value of backtrace_simple when max depth given.
	(basic_stacktrace::_M_reserve): Remove.
	(basic_stacktrace::_S_curr_cb): Remove.
	(basic_stacktrace::_M_prepare(size_type)): New function to
	reserve initial capacity and return callback.
	(basic_stacktrace::_Impl::_M_allocate): Remove check for 0 < n
	and remove redundant zeroing of _M_frames and _M_capacity.
	(basic_stacktrace::_Impl::_M_push_back): Add [[unlikely]]
	attribute. Assign _Impl instead of swapping.
	* testsuite/19_diagnostics/stacktrace/current.cc: New test.
---
 libstdc++-v3/include/std/stacktrace           | 110 ++++++++++--------
 .../19_diagnostics/stacktrace/current.cc      |  86 ++++++++++++++
 2 files changed, 148 insertions(+), 48 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/19_diagnostics/stacktrace/current.cc

diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
index 79038e803f2..5f928f10dee 100644
--- a/libstdc++-v3/include/std/stacktrace
+++ b/libstdc++-v3/include/std/stacktrace
@@ -237,15 +237,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       static basic_stacktrace
       current(const allocator_type& __alloc = allocator_type()) noexcept
       {
-	auto __state = stacktrace_entry::_S_init();
 	basic_stacktrace __ret(__alloc);
-	if (!__ret._M_reserve(64)) [[unlikely]]
-	  return __ret;
-
-	if (__glibcxx_backtrace_simple(__state, 1, _S_curr_cb(),
-				       nullptr, std::__addressof(__ret)))
-	  __ret._M_clear();
-
+	if (auto __cb = __ret._M_prepare()) [[likely]]
+	  {
+	    auto __state = stacktrace_entry::_S_init();
+	    if (__glibcxx_backtrace_simple(__state, 1, __cb, nullptr,
+					   std::__addressof(__ret)))
+	      __ret._M_clear();
+	  }
 	return __ret;
       }
 
@@ -254,16 +253,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       current(size_type __skip,
 	      const allocator_type& __alloc = allocator_type()) noexcept
       {
-	auto __state = stacktrace_entry::_S_init();
 	basic_stacktrace __ret(__alloc);
 	if (__skip >= __INT_MAX__) [[unlikely]]
 	  return __ret;
-	if (!__ret._M_reserve(64)) [[unlikely]]
-	  return __ret;
-
-	if (__glibcxx_backtrace_simple(__state, __skip + 1, _S_curr_cb(),
-				       nullptr, std::__addressof(__ret)))
-	  __ret._M_clear();
+	if (auto __cb = __ret._M_prepare()) [[likely]]
+	  {
+	    auto __state = stacktrace_entry::_S_init();
+	    if (__glibcxx_backtrace_simple(__state, __skip + 1, __cb, nullptr,
+					   std::__addressof(__ret)))
+	      __ret._M_clear();
+	  }
 
 	return __ret;
       }
@@ -275,19 +274,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       {
 	__glibcxx_assert(__skip <= (size_type(-1) - __max_depth));
 
-	auto __state = stacktrace_entry::_S_init();
 	basic_stacktrace __ret(__alloc);
-	if (__max_depth == 0 || __skip >= __INT_MAX__) [[unlikely]]
+	if (__max_depth == 0) [[unlikely]]
 	  return __ret;
-	if (!__ret._M_reserve(std::min<int>(__max_depth, 64))) [[unlikely]]
+	if (__skip >= __INT_MAX__) [[unlikely]]
 	  return __ret;
-
-	if (__glibcxx_backtrace_simple(__state, __skip + 1, _S_curr_cb(),
-				       nullptr, std::__addressof(__ret)))
-	  __ret._M_clear();
-	else if (__ret.size() > __max_depth)
-	  __ret.resize(__max_depth);
-
+	if (auto __cb = __ret._M_prepare(__max_depth)) [[likely]]
+	  {
+	    auto __state = stacktrace_entry::_S_init();
+	    int __err = __glibcxx_backtrace_simple(__state, __skip + 1, __cb,
+						   nullptr,
+						   std::__addressof(__ret));
+	    if (__err < 0)
+	      __ret._M_clear();
+	    else if (__ret.size() > __max_depth)
+	      __ret._M_impl._M_size = __max_depth;
+	  }
 	return __ret;
       }
 
@@ -524,12 +526,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
 
     private:
-      bool
-      _M_reserve(size_type __n) noexcept
-      {
-	return _M_impl._M_allocate(_M_alloc, __n) != nullptr;
-      }
-
+      // Precondition: _M_capacity != 0
       bool
       _M_push_back(const value_type& __x) noexcept
       {
@@ -543,18 +540,37 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_M_impl._M_deallocate(_M_alloc);
       }
 
-      static auto
-      _S_curr_cb() noexcept
+      // Precondition: __max_depth != 0
+      auto
+      _M_prepare(size_type __max_depth = -1) noexcept
       -> int (*) (void*, uintptr_t)
       {
-	return [](void* __data, uintptr_t __pc) {
+	auto __cb = +[](void* __data, uintptr_t __pc) {
 	  auto& __s = *static_cast<basic_stacktrace*>(__data);
 	  stacktrace_entry __f;
 	  __f._M_pc = __pc;
-	  if (__s._M_push_back(__f))
-	    return 0;
-	  return 1;
+	  if (__s._M_push_back(__f)) [[likely]]
+	    return 0; // continue tracing
+	  return -1; // stop tracing due to error
 	};
+
+	if (__max_depth > 128)
+	  __max_depth = 64; // soft limit, _M_push_back will reallocate
+	else
+	  __cb = [](void* __data, uintptr_t __pc) {
+	    auto& __s = *static_cast<basic_stacktrace*>(__data);
+	    stacktrace_entry __f;
+	    __f._M_pc = __pc;
+	    if (__s.size() == __s._M_impl._M_capacity) [[unlikely]]
+	      return 1; // stop tracing due to reaching max depth
+	    if (__s._M_push_back(__f)) [[likely]]
+	      return 0; // continue tracing
+	    return -1; // stop tracing due to error
+	  };
+
+	if (_M_impl._M_allocate(_M_alloc, __max_depth)) [[likely]]
+	  return __cb;
+	return nullptr;
       }
 
       struct _Impl
@@ -573,24 +589,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return std::min(__size_max, __alloc_max);
 	}
 
-	// Precondition: _M_frames == nullptr
+	// Precondition: _M_frames == nullptr && __n != 0
 	pointer
 	_M_allocate(allocator_type& __alloc, size_type __n) noexcept
 	{
-	  __try
+	  if (__n <= _S_max_size(__alloc)) [[likely]]
 	    {
-	      if (0 < __n && __n <= _S_max_size(__alloc)) [[likely]]
+	      __try
 		{
 		  _M_frames = __alloc.allocate(__n);
 		  _M_capacity = __n;
 		  return _M_frames;
 		}
+	      __catch (...)
+		{
+		}
 	    }
-	  __catch (...)
-	    {
-	    }
-	  _M_frames = nullptr;
-	  _M_capacity = 0;
 	  return nullptr;;
 	}
 
@@ -612,11 +626,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  _M_size = 0;
 	}
 
+	// Precondition: _M_capacity != 0
 	bool
 	_M_push_back(allocator_type& __alloc,
 		     const stacktrace_entry& __f) noexcept
 	{
-	  if (_M_size == _M_capacity)
+	  if (_M_size == _M_capacity) [[unlikely]]
 	    {
 	      _Impl __tmp;
 	      if (auto __f = __tmp._M_allocate(__alloc, _M_capacity * 2))
@@ -624,13 +639,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      else
 		return false;
 	      _M_deallocate(__alloc);
-	      std::swap(*this, __tmp);
+	      *this = __tmp;
 	    }
 	  stacktrace_entry* __addr = std::to_address(_M_frames + _M_size++);
 	  std::construct_at(__addr, __f);
 	  return true;
 	}
-
       };
 
       [[no_unique_address]] allocator_type  _M_alloc{};
diff --git a/libstdc++-v3/testsuite/19_diagnostics/stacktrace/current.cc b/libstdc++-v3/testsuite/19_diagnostics/stacktrace/current.cc
new file mode 100644
index 00000000000..184e23b460e
--- /dev/null
+++ b/libstdc++-v3/testsuite/19_diagnostics/stacktrace/current.cc
@@ -0,0 +1,86 @@
+// { dg-options "-std=gnu++23 -lstdc++_libbacktrace" }
+// { dg-do run { target c++23 } }
+// { dg-require-effective-target stacktrace }
+
+#include <stacktrace>
+#include <memory>
+#include <new>
+#include "testsuite_hooks.h"
+
+template<typename T>
+struct Allocator
+{
+  using value_type = T;
+  using propagate_on_container_move_assignment = std::true_type;
+
+  explicit
+  Allocator(unsigned max = -1u) : max_size(max) { }
+
+  template<typename U>
+    Allocator(const Allocator<U>& a) : max_size(a.max_size) { }
+
+  T*
+  allocate(std::size_t n)
+  {
+    if (n > max_size)
+      throw std::bad_alloc();
+
+    return std::allocator<T>().allocate(n);
+  }
+
+  void
+  deallocate(T* p, std::size_t n) noexcept
+  {
+    std::allocator<T>().deallocate(p, n);
+  }
+
+  bool operator==(const Allocator&) const = default;
+
+private:
+  unsigned max_size;
+};
+
+[[gnu::optimize("O0")]]
+void
+test_max_depth()
+{
+  using Stacktrace = std::basic_stacktrace<Allocator<std::stacktrace_entry>>;
+  using Alloc = typename Stacktrace::allocator_type;
+
+  [] { [] { [] { [] { [] { [] { [] { [] {
+    auto t = Stacktrace::current();
+    VERIFY( ! t.empty() );
+    const auto n = t.size(); // total number of frames
+    t = Stacktrace::current(8);
+    VERIFY( t.size() == (n - 8) );
+    t = Stacktrace::current(n);
+    VERIFY( t.empty() );
+    t = Stacktrace::current(n - 2);
+    VERIFY( t.size() == 2 );
+    t = Stacktrace::current(2, 6);
+    VERIFY( t.size() == 6 );
+    t = Stacktrace::current(n - 2, 6);
+    VERIFY( t.size() == 2 );
+
+    t = Stacktrace::current(Alloc(3));
+    // Full stacktrace is larger than 3 frames, so allocation fails:
+    VERIFY( t.empty() );
+    t = Stacktrace::current(3, Alloc(2));
+    // Stacktrace still too large after skipping 3 frames, so allocation fails:
+    VERIFY( t.empty() );
+    t = Stacktrace::current(0, 3, Alloc(3));
+    // Capacity for exactly 3 frames is allocated:
+    VERIFY( t.size() == 3 );
+    t = Stacktrace::current(2, 4, Alloc(4));
+    // Capacity for exactly 4 frames is allocated:
+    VERIFY( t.size() == 4 );
+    t = Stacktrace::current(0, 4, Alloc(3));
+    // Capacity for exactly 4 frames is requested, but allocation fails:
+    VERIFY( t.empty() );
+  }(); }(); }(); }(); }(); }(); }(); }();
+}
+
+int main()
+{
+  test_max_depth();
+}
-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [committed 2/5] libstdc++: Use nothrow new in std::stacktrace
  2022-04-12 21:41 [committed 1/5] libstdc++: Reduce memory usage in std::stacktrace::current Jonathan Wakely
@ 2022-04-12 21:41 ` Jonathan Wakely
  2022-04-12 21:41 ` [committed 3/5] libstdc++: Use allocator to construct std::stacktrace_entry objects Jonathan Wakely
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Jonathan Wakely @ 2022-04-12 21:41 UTC (permalink / raw)
  To: libstdc++, gcc-patches

Tested powerpc64-linux, pushed to trunk.

-- >8 --

We can avoid the overhead of handling a bad_alloc exception from
std::allocator<std::stacktrace_entry>::allocate by just calling the
nothrow operator new instead.

libstdc++-v3/ChangeLog:

	* include/std/stacktrace (basic_stacktrace::_Impl::_M_allocate):
	Use nothrow new instead of try block for std::allocator.
	(basic_stacktrace::_Impl::_M_deallocate): Use delete for
	std::allocator.
---
 libstdc++-v3/include/std/stacktrace | 42 ++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
index 5f928f10dee..f36c5a9abef 100644
--- a/libstdc++-v3/include/std/stacktrace
+++ b/libstdc++-v3/include/std/stacktrace
@@ -30,6 +30,7 @@
 
 #if __cplusplus > 202002L && _GLIBCXX_HAVE_STACKTRACE
 #include <compare>
+#include <new>
 #include <string>
 #include <sstream>
 #include <bits/stl_algobase.h>
@@ -589,23 +590,43 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return std::min(__size_max, __alloc_max);
 	}
 
+#if __has_builtin(__builtin_operator_new) >= 201802L
+# define _GLIBCXX_OPERATOR_NEW __builtin_operator_new
+# define _GLIBCXX_OPERATOR_DELETE __builtin_operator_delete
+#else
+# define _GLIBCXX_OPERATOR_NEW ::operator new
+# define _GLIBCXX_OPERATOR_DELETE ::operator delete
+#endif
+
 	// Precondition: _M_frames == nullptr && __n != 0
 	pointer
 	_M_allocate(allocator_type& __alloc, size_type __n) noexcept
 	{
 	  if (__n <= _S_max_size(__alloc)) [[likely]]
 	    {
-	      __try
+	      if constexpr (is_same_v<allocator_type, allocator<value_type>>)
 		{
-		  _M_frames = __alloc.allocate(__n);
-		  _M_capacity = __n;
-		  return _M_frames;
+		  __n *= sizeof(value_type);
+		  void* const __p = _GLIBCXX_OPERATOR_NEW (__n, nothrow_t{});
+		  if (__p == nullptr) [[unlikely]]
+		    return nullptr;
+		  _M_frames = static_cast<pointer>(__p);
 		}
-	      __catch (...)
+	      else
 		{
+		  __try
+		    {
+		      _M_frames = __alloc.allocate(__n);
+		    }
+		  __catch (const std::bad_alloc&)
+		    {
+		      return nullptr;
+		    }
 		}
+	      _M_capacity = __n;
+	      return _M_frames;
 	    }
-	  return nullptr;;
+	  return nullptr;
 	}
 
 	void
@@ -613,12 +634,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	{
 	  if (_M_capacity)
 	    {
-	      __alloc.deallocate(_M_frames, _M_capacity);
+	      if constexpr (is_same_v<allocator_type, allocator<value_type>>)
+		_GLIBCXX_OPERATOR_DELETE (static_cast<void*>(_M_frames),
+					  _M_capacity * sizeof(value_type));
+	      else
+		__alloc.deallocate(_M_frames, _M_capacity);
 	      _M_frames = nullptr;
 	      _M_capacity = 0;
 	    }
 	}
 
+#undef _GLIBCXX_OPERATOR_DELETE
+#undef _GLIBCXX_OPERATOR_NEW
+
 	void
 	_M_destroy() noexcept
 	{
-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [committed 3/5] libstdc++: Use allocator to construct std::stacktrace_entry objects
  2022-04-12 21:41 [committed 1/5] libstdc++: Reduce memory usage in std::stacktrace::current Jonathan Wakely
  2022-04-12 21:41 ` [committed 2/5] libstdc++: Use nothrow new in std::stacktrace Jonathan Wakely
@ 2022-04-12 21:41 ` Jonathan Wakely
  2022-04-12 21:41 ` [committed 4/5] libstdc++: shrink-to-fit in std::basic_stacktrace::current(skip, max) Jonathan Wakely
  2022-04-12 21:41 ` [committed 5/5] libstdc++: Prefer to use mmap instead of malloc in libbacktrace Jonathan Wakely
  3 siblings, 0 replies; 5+ messages in thread
From: Jonathan Wakely @ 2022-04-12 21:41 UTC (permalink / raw)
  To: libstdc++, gcc-patches

Tested powerpc64-linux, pushed to trunk.

-- >8 --

Because std::basic_stacktrace<A> is an allocator-aware container its
elements should be initialized using allocator_traits<A>::construct and
destroyed using allocator_traits<A>::destroy.

This adds new _M_clone and _M_assign helper functions to construct
elements correctly and uses those functions instead of calling
std::uninitialized_copy_n.

The _Impl::_M_destroy function needs to be passed an allocator to
destroy the elements correctly, so is replaced by _M_resize which can
also be used to trim the container to a smaller size.

Because destroying and creating std::stacktrace_entry objects is cheap,
the copy/move assignment operators can just destroy all existing
elements and use _Impl._M_clone or _Impl._M_assign to create new ones.

libstdc++-v3/ChangeLog:

	* include/std/stacktrace (basic_stacktrace): Use _Impl::_M_clone
	or _Impl::_M_assign to initialize elements in allocated storage.
	(basic_stacktrace::_M_clear()): Use _Impl::_M_resize instead of
	_Impl::_M_destroy.
	(basic_stacktrace::_Impl::_M_destroy()): Replace with ...
	(basic_stacktrace::_Impl::_M_resize(size_type, allocator&)): New
	function.
	(basic_stacktrace::_Impl::_M_push_back): Use _M_xclone. Construct
	new element using allocator.
	(basic_stacktrace::_Impl::_M_clone): New function.
	(basic_stacktrace::_Impl::_M_xclone): New function.
	(basic_stacktrace::_Impl::_M_assign): New function.
---
 libstdc++-v3/include/std/stacktrace | 92 +++++++++++++++--------------
 1 file changed, 47 insertions(+), 45 deletions(-)

diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
index f36c5a9abef..382d900a822 100644
--- a/libstdc++-v3/include/std/stacktrace
+++ b/libstdc++-v3/include/std/stacktrace
@@ -289,7 +289,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    if (__err < 0)
 	      __ret._M_clear();
 	    else if (__ret.size() > __max_depth)
-	      __ret._M_impl._M_size = __max_depth;
+	      __ret._M_impl._M_resize(__max_depth, __ret._M_alloc);
 	  }
 	return __ret;
       }
@@ -318,11 +318,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       : _M_alloc(__alloc)
       {
 	if (const auto __s = __other._M_impl._M_size)
-	  if (auto __f = _M_impl._M_allocate(_M_alloc, __s))
-	    {
-	      std::uninitialized_copy_n(__other.begin(), __s, __f);
-	      _M_impl._M_size = __s;
-	    }
+	  _M_impl = __other._M_impl._M_clone(_M_alloc);
       }
 
       basic_stacktrace(basic_stacktrace&& __other,
@@ -334,11 +330,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	else if (_M_alloc == __other._M_alloc)
 	  _M_impl = std::__exchange(__other._M_impl, {});
 	else if (const auto __s = __other._M_impl._M_size)
-	  if (auto __f = _M_impl._M_allocate(_M_alloc, __s))
-	    {
-	      std::uninitialized_copy_n(__other.begin(), __s, __f);
-	      _M_impl._M_size = __s;
-	    }
+	  _M_impl = __other._M_impl._M_clone(_M_alloc);
       }
 
       basic_stacktrace&
@@ -370,19 +362,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    if constexpr (__pocca)
 	      _M_alloc = __other._M_alloc;
 
-	    if (auto __f = _M_impl._M_allocate(_M_alloc, __s))
-	      {
-		std::uninitialized_copy_n(__other.begin(), __s, __f);
-		_M_impl._M_size = __s;
-	      }
+	    _M_impl = __other._M_impl._M_clone(_M_alloc);
 	  }
 	else
 	  {
-	    // Current storage is large enough and can be freed by whichever
-	    // allocator we will have after this function returns.
-	    auto __to = std::copy_n(__other.begin(), __s, begin());
-	    std::destroy(__to, end());
-	    _M_impl._M_size = __s;
+	    // Current storage is large enough.
+	    _M_impl._M_resize(0, _M_alloc);
+	    _M_impl._M_assign(__other._M_impl, _M_alloc);
 
 	    if constexpr (__pocca)
 	      _M_alloc = __other._M_alloc;
@@ -418,23 +404,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      {
 		// Need to allocate new storage.
 		_M_clear();
-
-		if (auto __f = _M_impl._M_allocate(_M_alloc, __s))
-		  {
-		    std::uninitialized_copy_n(__other.begin(), __s, __f);
-		    _M_impl._M_size = __s;
-		  }
+		_M_impl = __other._M_impl._M_clone(_M_alloc);
 	      }
 	    else
 	      {
 		// Current storage is large enough.
-		auto __first = __other.begin();
-		auto __mid = __first + std::min(__s, _M_impl._M_size);
-		auto __last = __other.end();
-		auto __to = std::copy(__first, __mid, begin());
-		__to = std::uninitialized_copy(__mid, __last, __to);
-		std::destroy(__to, end());
-		_M_impl._M_size = __s;
+		_M_impl._M_resize(0, _M_alloc);
+		_M_impl._M_assign(__other._M_impl, _M_alloc);
 	      }
 	  }
 
@@ -527,7 +503,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
 
     private:
-      // Precondition: _M_capacity != 0
       bool
       _M_push_back(const value_type& __x) noexcept
       {
@@ -537,7 +512,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       void
       _M_clear() noexcept
       {
-	_M_impl._M_destroy();
+	_M_impl._M_resize(0, _M_alloc);
 	_M_impl._M_deallocate(_M_alloc);
       }
 
@@ -647,32 +622,59 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #undef _GLIBCXX_OPERATOR_DELETE
 #undef _GLIBCXX_OPERATOR_NEW
 
+	// Precondition: __n <= _M_size
 	void
-	_M_destroy() noexcept
+	_M_resize(size_type __n, allocator_type& __alloc) noexcept
 	{
-	  std::destroy_n(_M_frames, _M_size);
-	  _M_size = 0;
+	  for (size_type __i = __n; __i < _M_size; ++__i)
+	    _AllocTraits::destroy(__alloc, &_M_frames[__i]);
+	  _M_size = __n;
 	}
 
-	// Precondition: _M_capacity != 0
 	bool
 	_M_push_back(allocator_type& __alloc,
 		     const stacktrace_entry& __f) noexcept
 	{
 	  if (_M_size == _M_capacity) [[unlikely]]
 	    {
-	      _Impl __tmp;
-	      if (auto __f = __tmp._M_allocate(__alloc, _M_capacity * 2))
-		std::uninitialized_copy_n(_M_frames, _M_size, __f);
-	      else
+	      _Impl __tmp = _M_xclone(_M_capacity ? _M_capacity : 8, __alloc);
+	      if (!__tmp._M_capacity) [[unlikely]]
 		return false;
+	      _M_resize(0, __alloc);
 	      _M_deallocate(__alloc);
 	      *this = __tmp;
 	    }
 	  stacktrace_entry* __addr = std::to_address(_M_frames + _M_size++);
-	  std::construct_at(__addr, __f);
+	  _AllocTraits::construct(__alloc, __addr, __f);
 	  return true;
 	}
+
+	// Precondition: _M_size != 0
+	_Impl
+	_M_clone(allocator_type& __alloc) const noexcept
+	{
+	  return _M_xclone(_M_size, __alloc);
+	}
+
+	// Precondition: _M_size != 0 || __extra != 0
+	_Impl
+	_M_xclone(size_type __extra, allocator_type& __alloc) const noexcept
+	{
+	  _Impl __i;
+	  if (__i._M_allocate(__alloc, _M_size + __extra)) [[likely]]
+	    __i._M_assign(*this, __alloc);
+	  return __i;
+	}
+
+	// Precondition: _M_capacity >= __other._M_size
+	void
+	_M_assign(const _Impl& __other, allocator_type& __alloc) noexcept
+	{
+	  std::__uninitialized_copy_a(__other._M_frames,
+				      __other._M_frames + __other._M_size,
+				      _M_frames, __alloc);
+	  _M_size = __other._M_size;
+	}
       };
 
       [[no_unique_address]] allocator_type  _M_alloc{};
-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [committed 4/5] libstdc++: shrink-to-fit in std::basic_stacktrace::current(skip, max)
  2022-04-12 21:41 [committed 1/5] libstdc++: Reduce memory usage in std::stacktrace::current Jonathan Wakely
  2022-04-12 21:41 ` [committed 2/5] libstdc++: Use nothrow new in std::stacktrace Jonathan Wakely
  2022-04-12 21:41 ` [committed 3/5] libstdc++: Use allocator to construct std::stacktrace_entry objects Jonathan Wakely
@ 2022-04-12 21:41 ` Jonathan Wakely
  2022-04-12 21:41 ` [committed 5/5] libstdc++: Prefer to use mmap instead of malloc in libbacktrace Jonathan Wakely
  3 siblings, 0 replies; 5+ messages in thread
From: Jonathan Wakely @ 2022-04-12 21:41 UTC (permalink / raw)
  To: libstdc++, gcc-patches

Tested powerpc64-linux, pushed to trunk.

-- >8 --

If a large stacktrace is reduced to a max depth that is less than half
the capacity it will now be reallocated to remove the unused capacity.

libstdc++-v3/ChangeLog:

	* include/std/stacktrace (basic_stacktrace::current): Reallocate
	a smaller container if the unused capacity is larger than the
	used size.
---
 libstdc++-v3/include/std/stacktrace | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/libstdc++-v3/include/std/stacktrace b/libstdc++-v3/include/std/stacktrace
index 382d900a822..98ce9231150 100644
--- a/libstdc++-v3/include/std/stacktrace
+++ b/libstdc++-v3/include/std/stacktrace
@@ -289,7 +289,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    if (__err < 0)
 	      __ret._M_clear();
 	    else if (__ret.size() > __max_depth)
-	      __ret._M_impl._M_resize(__max_depth, __ret._M_alloc);
+	      {
+		__ret._M_impl._M_resize(__max_depth, __ret._M_alloc);
+
+		if (__ret._M_impl._M_capacity / 2 >= __max_depth)
+		  {
+		    // shrink to fit
+		    _Impl __tmp = __ret._M_impl._M_clone(__ret._M_alloc);
+		    if (__tmp._M_capacity)
+		      {
+			__ret._M_clear();
+			__ret._M_impl = __tmp;
+		      }
+		  }
+	      }
 	  }
 	return __ret;
       }
-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [committed 5/5] libstdc++: Prefer to use mmap instead of malloc in libbacktrace
  2022-04-12 21:41 [committed 1/5] libstdc++: Reduce memory usage in std::stacktrace::current Jonathan Wakely
                   ` (2 preceding siblings ...)
  2022-04-12 21:41 ` [committed 4/5] libstdc++: shrink-to-fit in std::basic_stacktrace::current(skip, max) Jonathan Wakely
@ 2022-04-12 21:41 ` Jonathan Wakely
  3 siblings, 0 replies; 5+ messages in thread
From: Jonathan Wakely @ 2022-04-12 21:41 UTC (permalink / raw)
  To: libstdc++, gcc-patches

Tested powerpc64-linux, pushed to trunk.

-- >8 --

As reported in PR libbacktrace/105240, libbacktrace leaks memory when
using malloc for allocations. I originally thought it would be simpler
to just use malloc unconditionally (because it's supported on all
targets) but the leaks make that problematic.

This adds libbacktrace's detection for mmap to the libstdc++
configury, so that we use mmap.c and mmapio.c when possible. This avoids
the leaks seen previously, at least on linux.

libstdc++-v3/ChangeLog:

	* acinclude.m4 (GLIBCXX_ENABLE_BACKTRACE): Check for mmap.
	* config.h.in: Regenerate.
	* configure: Regenerate.
---
 libstdc++-v3/acinclude.m4 | 35 +++++++++++++++++++----
 libstdc++-v3/config.h.in  |  3 ++
 libstdc++-v3/configure    | 59 +++++++++++++++++++++++++++++++++------
 3 files changed, 83 insertions(+), 14 deletions(-)

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index f53461c85a5..eac8aeda48b 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -5003,18 +5003,41 @@ elf64) elfsize=64 ;;
 esac
 BACKTRACE_CPPFLAGS="$BACKTRACE_CPPFLAGS -DBACKTRACE_ELF_SIZE=$elfsize"
 
-  ALLOC_FILE=alloc.lo
-  AC_SUBST(ALLOC_FILE)
-  VIEW_FILE=read.lo
-  AC_SUBST(VIEW_FILE)
-
   AC_MSG_CHECKING([whether to build libbacktrace support])
   if test "$enable_libstdcxx_backtrace" == "auto"; then
     enable_libstdcxx_backtrace=no
   fi
   if test "$enable_libstdcxx_backtrace" == "yes"; then
     BACKTRACE_SUPPORTED=1
-    BACKTRACE_USES_MALLOC=1
+
+    AC_CHECK_HEADERS(sys/mman.h)
+    case "${host}" in
+      *-*-msdosdjgpp) # DJGPP has sys/man.h, but no mmap
+	have_mmap=no ;;
+      *-*-*)
+	have_mmap="$ac_cv_header_sys_mman_h" ;;
+    esac
+
+    if test "$have_mmap" = "no"; then
+      VIEW_FILE=read.lo
+      ALLOC_FILE=alloc.lo
+    else
+      VIEW_FILE=mmapio.lo
+      AC_PREPROC_IFELSE([AC_LANG_SOURCE([
+    #include <sys/mman.h>
+    #if !defined(MAP_ANONYMOUS) && !defined(MAP_ANON)
+      #error no MAP_ANONYMOUS
+    #endif
+    ])], [ALLOC_FILE=mmap.lo], [ALLOC_FILE=alloc.lo])
+    fi
+    AC_SUBST(VIEW_FILE)
+    AC_SUBST(ALLOC_FILE)
+
+    BACKTRACE_USES_MALLOC=0
+    if test "$ALLOC_FILE" = "alloc.lo"; then
+      BACKTRACE_USES_MALLOC=1
+    fi
+
     if test "$ac_has_gthreads" = "yes"; then
       BACKTRACE_SUPPORTS_THREADS=1
     else
diff --git a/libstdc++-v3/config.h.in b/libstdc++-v3/config.h.in
index f6212de9268..f30a8c51c45 100644
--- a/libstdc++-v3/config.h.in
+++ b/libstdc++-v3/config.h.in
@@ -420,6 +420,9 @@
 /* Define to 1 if you have the <sys/machine.h> header file. */
 #undef HAVE_SYS_MACHINE_H
 
+/* Define to 1 if you have the <sys/mman.h> header file. */
+#undef HAVE_SYS_MMAN_H
+
 /* Define to 1 if you have the <sys/param.h> header file. */
 #undef HAVE_SYS_PARAM_H
 
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index ef80912d0b9..35dc3f49383 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -681,8 +681,8 @@ BACKTRACE_SUPPORTS_THREADS
 BACKTRACE_USES_MALLOC
 BACKTRACE_SUPPORTED
 BACKTRACE_CPPFLAGS
-VIEW_FILE
 ALLOC_FILE
+VIEW_FILE
 FORMAT_FILE
 ENABLE_FILESYSTEM_TS_FALSE
 ENABLE_FILESYSTEM_TS_TRUE
@@ -16190,7 +16190,7 @@ ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
 
     ac_save_CXXFLAGS="$CXXFLAGS"
 
-        cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+                cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
     #if ! defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2
@@ -77463,11 +77463,6 @@ elf64) elfsize=64 ;;
 esac
 BACKTRACE_CPPFLAGS="$BACKTRACE_CPPFLAGS -DBACKTRACE_ELF_SIZE=$elfsize"
 
-  ALLOC_FILE=alloc.lo
-
-  VIEW_FILE=read.lo
-
-
   { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build libbacktrace support" >&5
 $as_echo_n "checking whether to build libbacktrace support... " >&6; }
   if test "$enable_libstdcxx_backtrace" == "auto"; then
@@ -77475,7 +77470,55 @@ $as_echo_n "checking whether to build libbacktrace support... " >&6; }
   fi
   if test "$enable_libstdcxx_backtrace" == "yes"; then
     BACKTRACE_SUPPORTED=1
-    BACKTRACE_USES_MALLOC=1
+
+    for ac_header in sys/mman.h
+do :
+  ac_fn_c_check_header_mongrel "$LINENO" "sys/mman.h" "ac_cv_header_sys_mman_h" "$ac_includes_default"
+if test "x$ac_cv_header_sys_mman_h" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_SYS_MMAN_H 1
+_ACEOF
+
+fi
+
+done
+
+    case "${host}" in
+      *-*-msdosdjgpp) # DJGPP has sys/man.h, but no mmap
+	have_mmap=no ;;
+      *-*-*)
+	have_mmap="$ac_cv_header_sys_mman_h" ;;
+    esac
+
+    if test "$have_mmap" = "no"; then
+      VIEW_FILE=read.lo
+      ALLOC_FILE=alloc.lo
+    else
+      VIEW_FILE=mmapio.lo
+      cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+    #include <sys/mman.h>
+    #if !defined(MAP_ANONYMOUS) && !defined(MAP_ANON)
+      #error no MAP_ANONYMOUS
+    #endif
+
+_ACEOF
+if ac_fn_c_try_cpp "$LINENO"; then :
+  ALLOC_FILE=mmap.lo
+else
+  ALLOC_FILE=alloc.lo
+fi
+rm -f conftest.err conftest.i conftest.$ac_ext
+    fi
+
+
+
+    BACKTRACE_USES_MALLOC=0
+    if test "$ALLOC_FILE" = "alloc.lo"; then
+      BACKTRACE_USES_MALLOC=1
+    fi
+
     if test "$ac_has_gthreads" = "yes"; then
       BACKTRACE_SUPPORTS_THREADS=1
     else
-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-04-12 21:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-12 21:41 [committed 1/5] libstdc++: Reduce memory usage in std::stacktrace::current Jonathan Wakely
2022-04-12 21:41 ` [committed 2/5] libstdc++: Use nothrow new in std::stacktrace Jonathan Wakely
2022-04-12 21:41 ` [committed 3/5] libstdc++: Use allocator to construct std::stacktrace_entry objects Jonathan Wakely
2022-04-12 21:41 ` [committed 4/5] libstdc++: shrink-to-fit in std::basic_stacktrace::current(skip, max) Jonathan Wakely
2022-04-12 21:41 ` [committed 5/5] libstdc++: Prefer to use mmap instead of malloc in libbacktrace 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).