[libcxx-commits] [libcxx] 8cf76e9 - Partially inline basic_string copy constructor in UNSTABLE
Martijn Vels via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Mar 3 14:56:03 PST 2020
Author: Martijn Vels
Date: 2020-03-03T17:49:25-05:00
New Revision: 8cf76e913b867a98a9843aa1b3d782632ed5d930
URL: https://github.com/llvm/llvm-project/commit/8cf76e913b867a98a9843aa1b3d782632ed5d930
DIFF: https://github.com/llvm/llvm-project/commit/8cf76e913b867a98a9843aa1b3d782632ed5d930.diff
LOG: Partially inline basic_string copy constructor in UNSTABLE
his change splits the copy constructor up inlining short initialization, and explicitly outlining long initialization into __init_copy_ctor_external() which is the externally instantiated slow path.
For unstable ABI, this has the following changes:
remove basic_string(const basic_string&)
remove basic_string(const basic_string&, const Allocator&)
add __init_copy_ctor_external(const value_type*, size_type)
Quick local benchmark for Copy:
Master
```
---------------------------------------------------------------
Benchmark Time CPU Iterations
---------------------------------------------------------------
BM_StringCopy_Empty 3.50 ns 3.51 ns 199326720
BM_StringCopy_Small 3.50 ns 3.51 ns 199510016
BM_StringCopy_Large 15.7 ns 15.7 ns 45230080
BM_StringCopy_Huge 1503 ns 1503 ns 464896
```
```
---------------------------------------------------------------
Benchmark Time CPU Iterations
---------------------------------------------------------------
BM_StringCopy_Empty 1.99 ns 2.00 ns 356471808
BM_StringCopy_Small 3.29 ns 3.30 ns 203425792
BM_StringCopy_Large 13.3 ns 13.3 ns 52948992
BM_StringCopy_Huge 1472 ns 1472 ns 475136
```
Author: Martijn Vels <martijn.vels at gmail.com>
Reviewers: EricWF, mclow.list
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73223
Added:
libcxx/test/libcxx/strings/basic.string/string.cons/copy_shrunk_long.pass.cpp
Modified:
libcxx/include/__string
libcxx/include/string
Removed:
################################################################################
diff --git a/libcxx/include/__string b/libcxx/include/__string
index 45060c282d59..d81301453658 100644
--- a/libcxx/include/__string
+++ b/libcxx/include/__string
@@ -140,15 +140,14 @@ _LIBCPP_BEGIN_NAMESPACE_STD
_Func(_LIBCPP_FUNC_VIS basic_string<_CharType>& basic_string<_CharType>::replace(size_type, size_type, value_type const*, size_type)) \
_Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::size_type basic_string<_CharType>::rfind(value_type const*, size_type, size_type) const) \
_Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__init(value_type const*, size_type, size_type)) \
- _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::basic_string(basic_string const&)) \
_Func(_LIBCPP_FUNC_VIS basic_string<_CharType>& basic_string<_CharType>::replace(size_type, size_type, value_type const*)) \
- _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::basic_string(basic_string const&, std::allocator<_CharType> const&)) \
_Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::size_type basic_string<_CharType>::find_last_not_of(value_type const*, size_type, size_type) const) \
_Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::~basic_string()) \
_Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::size_type basic_string<_CharType>::find_first_not_of(value_type const*, size_type, size_type) const) \
_Func(_LIBCPP_FUNC_VIS basic_string<_CharType>& basic_string<_CharType>::insert(size_type, size_type, value_type)) \
_Func(_LIBCPP_FUNC_VIS basic_string<_CharType>& basic_string<_CharType>::operator=(value_type)) \
_Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__init(value_type const*, size_type)) \
+ _Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__init_copy_ctor_external(value_type const*, size_type)) \
_Func(_LIBCPP_FUNC_VIS const _CharType& basic_string<_CharType>::at(size_type) const) \
_Func(_LIBCPP_FUNC_VIS basic_string<_CharType>& basic_string<_CharType>::insert(size_type, value_type const*, size_type)) \
_Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::size_type basic_string<_CharType>::find_first_of(value_type const*, size_type, size_type) const) \
diff --git a/libcxx/include/string b/libcxx/include/string
index c2a4220e276a..ffa8c66f7e35 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1549,6 +1549,16 @@ private:
inline
void __init(size_type __n, value_type __c);
+ // Slow path for the (inlined) copy constructor for 'long' strings.
+ // Always externally instantiated and not inlined.
+ // Requires that __s is zero terminated.
+ // The main reason for this function to exist is because for unstable, we
+ // want to allow inlining of the copy constructor. However, we don't want
+ // to call the __init() functions as those are marked as inline which may
+ // result in over-aggressive inlining by the compiler, where our aim is
+ // to only inline the fast path code directly in the ctor.
+ void __init_copy_ctor_external(const value_type* __s, size_type __sz);
+
template <class _InputIterator>
inline
_EnableIf
@@ -1850,7 +1860,9 @@ basic_string<_CharT, _Traits, _Allocator>::basic_string(const basic_string& __st
if (!__str.__is_long())
__r_.first().__r = __str.__r_.first().__r;
else
- __init(_VSTD::__to_address(__str.__get_long_pointer()), __str.__get_long_size());
+ __init_copy_ctor_external(_VSTD::__to_address(__str.__get_long_pointer()),
+ __str.__get_long_size());
+
#if _LIBCPP_DEBUG_LEVEL >= 2
__get_db()->__insert_c(this);
#endif
@@ -1864,7 +1876,8 @@ basic_string<_CharT, _Traits, _Allocator>::basic_string(
if (!__str.__is_long())
__r_.first().__r = __str.__r_.first().__r;
else
- __init(_VSTD::__to_address(__str.__get_long_pointer()), __str.__get_long_size());
+ __init_copy_ctor_external(_VSTD::__to_address(__str.__get_long_pointer()),
+ __str.__get_long_size());
#if _LIBCPP_DEBUG_LEVEL >= 2
__get_db()->__insert_c(this);
#endif
@@ -1890,6 +1903,25 @@ basic_string<_CharT, _Traits, _Allocator>::basic_string(basic_string&& __str)
#endif
}
+template <class _CharT, class _Traits, class _Allocator>
+void basic_string<_CharT, _Traits, _Allocator>::__init_copy_ctor_external(
+ const value_type* __s, size_type __sz) {
+ pointer __p;
+ if (__sz < __min_cap) {
+ __p = __get_short_pointer();
+ __set_short_size(__sz);
+ } else {
+ if (__sz > max_size())
+ this->__throw_length_error();
+ size_t __cap = __recommend(__sz);
+ __p = __alloc_traits::allocate(__alloc(), __cap + 1);
+ __set_long_pointer(__p);
+ __set_long_cap(__cap + 1);
+ __set_long_size(__sz);
+ }
+ traits_type::copy(_VSTD::__to_address(__p), __s, __sz + 1);
+}
+
template <class _CharT, class _Traits, class _Allocator>
inline
basic_string<_CharT, _Traits, _Allocator>::basic_string(basic_string&& __str, const allocator_type& __a)
diff --git a/libcxx/test/libcxx/strings/basic.string/string.cons/copy_shrunk_long.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.cons/copy_shrunk_long.pass.cpp
new file mode 100644
index 000000000000..ec2fe6c3fdb7
--- /dev/null
+++ b/libcxx/test/libcxx/strings/basic.string/string.cons/copy_shrunk_long.pass.cpp
@@ -0,0 +1,50 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <string>
+
+// basic_string(const basic_string<charT,traits,Allocator>& str);
+
+#include <string>
+#include <cassert>
+
+#include "test_macros.h"
+#include "test_allocator.h"
+#include "min_allocator.h"
+
+template <class S>
+void
+test()
+{
+ // Tests that a long string holding a SSO size string results in
+ // an SSO copy constructed value.
+ S s1("1234567890123456789012345678901234567890123456789012345678901234567890");
+ s1.resize(7);
+ S s2(s1);
+ LIBCPP_ASSERT(s2.__invariants());
+ assert(s2 == s1);
+ assert(s2.capacity() < sizeof(S));
+}
+
+int main(int, char**)
+{
+ {
+ typedef test_allocator<char> A;
+ typedef std::basic_string<char, std::char_traits<char>, A> S;
+ test<S>();
+ }
+#if TEST_STD_VER >= 11
+ {
+ typedef min_allocator<char> A;
+ typedef std::basic_string<char, std::char_traits<char>, A> S;
+ test<S>();
+ }
+#endif
+
+ return 0;
+}
More information about the libcxx-commits
mailing list