[libcxx-commits] [libcxx] [libc++] ~vector: Don't __clear trivial typed elements (PR #86947)

[Johan Gardell via libcxx-commits]

https://github.com/gardell created https://github.com/llvm/llvm-project/pull/86947

I've been optimizing some code that streams large amounts of binary data using `std::vector<uint8_t>` and in the profiler I can see that the application spends a lot of time in `~vector()`, specifically `__clear()`.

For the `std::vector<bool>` specialization, `__clear` is not called when destructing the vector. I believe this is because `bool` is trivially destructible.

In this PR I propose a template specialization of `__destroy_vector` in the general `vector<T>` case that allows us to suppress the call to `__clear` if the element type in question is `is_trivially_destructible`.

Please let me know if I should clarify something. This is my first submission so I am sure I've gotten a lot of stuff wrong! 

Regards


>From 6917d186bbb197356bdeaf663380976c8080aba1 Mon Sep 17 00:00:00 2001
From: Johan Gardell <grdll at amazon.com>
Date: Thu, 28 Mar 2024 10:38:12 +0100
Subject: [PATCH] [libc++] ~vector: Don't __clear trivial elements

If the element type of a `vector` is `trivially_destructible` do not
call `__clear` when destroying the `vector`.
---
 libcxx/include/vector                         | 10 +++
 ...dtor_no_trivial_element_dtor_call.pass.cpp | 85 +++++++++++++++++++
 2 files changed, 95 insertions(+)
 create mode 100644 libcxx/test/std/containers/sequences/vector/vector.cons/dtor_no_trivial_element_dtor_call.pass.cpp

diff --git a/libcxx/include/vector b/libcxx/include/vector
index 0908482600c533..e0df865366103a 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -347,6 +347,7 @@ template<class T, class charT> requires is-vector-bool-reference<T> // Since C++
 #include <__type_traits/is_allocator.h>
 #include <__type_traits/is_constructible.h>
 #include <__type_traits/is_nothrow_assignable.h>
+#include <__type_traits/is_trivially_destructible.h>
 #include <__type_traits/noexcept_move_assign_container.h>
 #include <__type_traits/type_identity.h>
 #include <__utility/exception_guard.h>
@@ -486,6 +487,15 @@ private:
   public:
     _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI __destroy_vector(vector& __vec) : __vec_(__vec) {}
 
+    template <typename _Tp2 = _Tp, __enable_if_t<is_trivially_destructible<_Tp2>::value, int> = 0>
+    _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void operator()() {
+      if (__vec_.__begin_ != nullptr) {
+        __vec_.__annotate_delete();
+        __alloc_traits::deallocate(__vec_.__alloc(), __vec_.__begin_, __vec_.capacity());
+      }
+    }
+
+    template <typename _Tp2 = _Tp, __enable_if_t<!is_trivially_destructible<_Tp2>::value, int> = 0>
     _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void operator()() {
       if (__vec_.__begin_ != nullptr) {
         __vec_.__clear();
diff --git a/libcxx/test/std/containers/sequences/vector/vector.cons/dtor_no_trivial_element_dtor_call.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.cons/dtor_no_trivial_element_dtor_call.pass.cpp
new file mode 100644
index 00000000000000..5a5399e2fffc68
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector/vector.cons/dtor_no_trivial_element_dtor_call.pass.cpp
@@ -0,0 +1,85 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include <vector>
+#include <cassert>
+
+struct int_alloc
+{
+    typedef int value_type;
+
+    template< class U >
+    struct rebind
+    {
+        typedef int_alloc other;
+    };
+
+    int* allocate(std::size_t n)
+    {
+        return new int[n];
+    }
+    void deallocate(int* p, std::size_t n)
+    {
+        for (std::size_t i = 0; i < n; ++i) {
+            assert(p[i] != 0);
+        }
+        delete p;
+    }
+};
+
+struct with_dtor
+{
+    with_dtor() = default;
+    with_dtor(const with_dtor &) = default;
+
+    int x = 42;
+
+    ~with_dtor() {
+        x = 0;
+    }
+};
+
+struct with_dtor_alloc
+{
+    typedef with_dtor value_type;
+
+    template< class U >
+    struct rebind
+    {
+        typedef with_dtor_alloc other;
+    };
+
+    with_dtor* allocate(std::size_t n)
+    {
+        return new with_dtor[n];
+    }
+    void deallocate(with_dtor* p, std::size_t n)
+    {
+        for (std::size_t i = 0; i < n; ++i) {
+            assert(p[i].x == 0);
+        }
+        delete[] p;
+    }
+};
+
+void tests()
+{
+    {
+        std::vector<with_dtor, with_dtor_alloc> v(5, with_dtor());
+    }
+    {
+        std::vector<int, int_alloc> v(5, 42);
+    }
+}
+
+int main(int, char**)
+{
+    tests();
+    return 0;
+}
+