[libc++][NFC] Use __constexpr_memmove instead of copy_n in <__string/char_traits.h>

[philnik777]

copy_n has been used to allow constant evaluation of char_traits. We now have __constexpr_memmove, which copy_n just forwards to. We can call __constexpr_memmove directly, avoiding a bunch of instantiations. This reduces the time it takes to include <string> from 321ms to 285ms.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

copy_n has been used to allow constant evaluation of char_traits. We now have __constexpr_memmove, which copy_n just forwards to. We can call __constexpr_memmove directly, avoiding a bunch of instantiations. This reduces the time it takes to include <string> from 321ms to 285ms.

---

Full diff: https://github.com/llvm/llvm-project/pull/85920.diff

1 Files Affected:

• (modified) libcxx/include/__string/char_traits.h (+5-6)

diff --git a/libcxx/include/__string/char_traits.h b/libcxx/include/__string/char_traits.h
index 5880d3a22db2e7..47ed1057caaab1 100644
--- a/libcxx/include/__string/char_traits.h
+++ b/libcxx/include/__string/char_traits.h
@@ -9,7 +9,6 @@
 #ifndef _LIBCPP___STRING_CHAR_TRAITS_H
 #define _LIBCPP___STRING_CHAR_TRAITS_H
 
-#include <__algorithm/copy_n.h>
 #include <__algorithm/fill_n.h>
 #include <__algorithm/find_end.h>
 #include <__algorithm/find_first_of.h>
@@ -144,7 +143,7 @@ struct _LIBCPP_TEMPLATE_VIS char_traits<char> {
   copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
     _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(!std::__is_pointer_in_range(__s1, __s1 + __n, __s2),
                                           "char_traits::copy: source and destination ranges overlap");
-    std::copy_n(__s2, __n, __s1);
+    std::__constexpr_memmove(__s1, __s2, __element_count(__n));
     return __s1;
   }
 
@@ -221,7 +220,7 @@ struct _LIBCPP_TEMPLATE_VIS char_traits<wchar_t> {
   copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
     _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(!std::__is_pointer_in_range(__s1, __s1 + __n, __s2),
                                           "char_traits::copy: source and destination ranges overlap");
-    std::copy_n(__s2, __n, __s1);
+    std::__constexpr_memmove(__s1, __s2, __element_count(__n));
     return __s1;
   }
 
@@ -287,7 +286,7 @@ struct _LIBCPP_TEMPLATE_VIS char_traits<char8_t> {
   copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
     _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(!std::__is_pointer_in_range(__s1, __s1 + __n, __s2),
                                           "char_traits::copy: source and destination ranges overlap");
-    std::copy_n(__s2, __n, __s1);
+    std::__constexpr_memmove(__s1, __s2, __element_count(__n));
     return __s1;
   }
 
@@ -366,7 +365,7 @@ struct _LIBCPP_TEMPLATE_VIS char_traits<char16_t> {
   copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
     _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(!std::__is_pointer_in_range(__s1, __s1 + __n, __s2),
                                           "char_traits::copy: source and destination ranges overlap");
-    std::copy_n(__s2, __n, __s1);
+    std::__constexpr_memmove(__s1, __s2, __element_count(__n));
     return __s1;
   }
 
@@ -454,7 +453,7 @@ struct _LIBCPP_TEMPLATE_VIS char_traits<char32_t> {
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 static char_type*
   copy(char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT {
-    std::copy_n(__s2, __n, __s1);
+    std::__constexpr_memmove(__s1, __s2, __element_count(__n));
     return __s1;
   }
 

[ldionne]

LGTM. However, generally speaking I would be careful about not blindly removing useful layers of abstraction just to reduce compilation times. I'm not saying that's what this patch is doing, just warning that it might be easy to fall in that trap.

[mordante]

LGTM! +1 to what Louis said.

[philnik777]

LGTM. However, generally speaking I would be careful about not blindly removing useful layers of abstraction just to reduce compilation times. I'm not saying that's what this patch is doing, just warning that it might be easy to fall in that trap.

I absolutely agree. I've been thinking about whether this is worth it for a while. If __constexpr_memmove didn't already exist and have almost the same API as copy_n I almost certainly wouldn't have considered this. In most cases it's also not that much of a concern, since most of our code is templated and thus doesn't instantiate its dependencies unconditionally.