[libcxx] r285532 - Optimize filesystem::path by providing weaker exception guarantees.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Sun Oct 30 19:46:26 PDT 2016


Author: ericwf
Date: Sun Oct 30 21:46:25 2016
New Revision: 285532

URL: http://llvm.org/viewvc/llvm-project?rev=285532&view=rev
Log:
Optimize filesystem::path by providing weaker exception guarantees.

path uses string::append to construct, append, and concatenate paths. Unfortunatly
string::append has a strong exception safety guaranteed and if it can't prove
that the iterator operations don't throw then it will allocate a temporary
string copy to append to. However this extra allocation and copy is very
undesirable for path which doesn't have the same exception guarantees.

To work around this this patch adds string::__append_forward_unsafe which exposes
the std::string::append interface for forward iterators without enforcing
that the iterator is noexcept.


Modified:
    libcxx/trunk/benchmarks/CMakeLists.txt
    libcxx/trunk/benchmarks/filesystem.bench.cpp
    libcxx/trunk/include/experimental/filesystem
    libcxx/trunk/include/string

Modified: libcxx/trunk/benchmarks/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/benchmarks/CMakeLists.txt?rev=285532&r1=285531&r2=285532&view=diff
==============================================================================
--- libcxx/trunk/benchmarks/CMakeLists.txt (original)
+++ libcxx/trunk/benchmarks/CMakeLists.txt Sun Oct 30 21:46:25 2016
@@ -69,6 +69,7 @@ set(BENCHMARK_NATIVE_INSTALL ${CMAKE_CUR
 set(BENCHMARK_TEST_COMPILE_FLAGS
     -std=c++14 -O2
     -I${BENCHMARK_LIBCXX_INSTALL}/include
+    -I${LIBCXX_SOURCE_DIR}/test/support
 )
 set(BENCHMARK_TEST_LIBCXX_COMPILE_FLAGS
     -nostdinc++

Modified: libcxx/trunk/benchmarks/filesystem.bench.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/benchmarks/filesystem.bench.cpp?rev=285532&r1=285531&r2=285532&view=diff
==============================================================================
--- libcxx/trunk/benchmarks/filesystem.bench.cpp (original)
+++ libcxx/trunk/benchmarks/filesystem.bench.cpp Sun Oct 30 21:46:25 2016
@@ -2,6 +2,7 @@
 
 #include "benchmark/benchmark_api.h"
 #include "GenerateInput.hpp"
+#include "test_iterators.h"
 
 namespace fs = std::experimental::filesystem;
 
@@ -41,6 +42,39 @@ void BM_PathConstructCStr(benchmark::Sta
 BENCHMARK_CAPTURE(BM_PathConstructCStr, large_string,
   getRandomStringInputs)->Arg(TestNumInputs);
 
+
+template <template <class...> class ItType, class GenInputs>
+void BM_PathConstructIter(benchmark::State &st, GenInputs gen) {
+  using namespace fs;
+  using Iter = ItType<std::string::const_iterator>;
+  const auto in = gen(st.range(0));
+  path PP;
+  for (auto& Part : in)
+    PP /= Part;
+  auto Start = Iter(PP.native().begin());
+  auto End = Iter(PP.native().end());
+  benchmark::DoNotOptimize(PP.native().data());
+  benchmark::DoNotOptimize(Start);
+  benchmark::DoNotOptimize(End);
+  while (st.KeepRunning()) {
+    const path P(Start, End);
+    benchmark::DoNotOptimize(P.native().data());
+  }
+}
+template <class GenInputs>
+void BM_PathConstructInputIter(benchmark::State &st, GenInputs gen) {
+  BM_PathConstructIter<input_iterator>(st, gen);
+}
+template <class GenInputs>
+void BM_PathConstructForwardIter(benchmark::State &st, GenInputs gen) {
+  BM_PathConstructIter<forward_iterator>(st, gen);
+}
+BENCHMARK_CAPTURE(BM_PathConstructInputIter, large_string,
+  getRandomStringInputs)->Arg(TestNumInputs);
+BENCHMARK_CAPTURE(BM_PathConstructForwardIter, large_string,
+  getRandomStringInputs)->Arg(TestNumInputs);
+
+
 template <class GenInputs>
 void BM_PathIterateMultipleTimes(benchmark::State &st, GenInputs gen) {
   using namespace fs;

Modified: libcxx/trunk/include/experimental/filesystem
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/filesystem?rev=285532&r1=285531&r2=285532&view=diff
==============================================================================
--- libcxx/trunk/include/experimental/filesystem (original)
+++ libcxx/trunk/include/experimental/filesystem Sun Oct 30 21:46:25 2016
@@ -625,8 +625,18 @@ template <>
 struct _PathCVT<char> {
 
     template <class _Iter>
-    static void __append_range(string& __dest, _Iter __b, _Iter __e) {
-        __dest.append(__b, __e);
+    static typename enable_if<
+        __is_exactly_input_iterator<_Iter>::value
+    >::type __append_range(string& __dest, _Iter __b, _Iter __e) {
+        for (; __b != __e; ++__b)
+            __dest.push_back(*__b);
+    }
+
+    template <class _Iter>
+    static typename enable_if<
+        __is_forward_iterator<_Iter>::value
+    >::type __append_range(string& __dest, _Iter __b, _Iter __e) {
+        __dest.__append_forward_unsafe(__b, __e);
     }
 
     template <class _Iter>

Modified: libcxx/trunk/include/string
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=285532&r1=285531&r2=285532&view=diff
==============================================================================
--- libcxx/trunk/include/string (original)
+++ libcxx/trunk/include/string Sun Oct 30 21:46:25 2016
@@ -926,6 +926,8 @@ public:
     basic_string& append(const value_type* __s, size_type __n);
     basic_string& append(const value_type* __s);
     basic_string& append(size_type __n, value_type __c);
+    template <class _ForwardIterator>
+    basic_string& __append_forward_unsafe(_ForwardIterator, _ForwardIterator);
     template<class _InputIterator>
         typename enable_if
         <
@@ -933,7 +935,12 @@ public:
                 || !__libcpp_string_gets_noexcept_iterator<_InputIterator>::value,
             basic_string&
         >::type
-        append(_InputIterator __first, _InputIterator __last);
+    _LIBCPP_INLINE_VISIBILITY
+    append(_InputIterator __first, _InputIterator __last) {
+      const basic_string __temp (__first, __last, __alloc());
+      append(__temp.data(), __temp.size());
+      return *this;
+    }
     template<class _ForwardIterator>
         typename enable_if
         <
@@ -941,7 +948,11 @@ public:
                 && __libcpp_string_gets_noexcept_iterator<_ForwardIterator>::value,
             basic_string&
         >::type
-        append(_ForwardIterator __first, _ForwardIterator __last);
+    _LIBCPP_INLINE_VISIBILITY
+    append(_ForwardIterator __first, _ForwardIterator __last) {
+      return __append_forward_unsafe(__first, __last);
+    }
+
 #ifndef _LIBCPP_HAS_NO_GENERALIZED_INITIALIZERS
     _LIBCPP_INLINE_VISIBILITY
     basic_string& append(initializer_list<value_type> __il) {return append(__il.begin(), __il.size());}
@@ -2182,21 +2193,6 @@ basic_string<_CharT, _Traits, _Allocator
     traits_type::assign(*++__p, value_type());
 }
 
-template <class _CharT, class _Traits, class _Allocator>
-template<class _InputIterator>
-typename enable_if
-<
-    __is_exactly_input_iterator<_InputIterator>::value
-             || !__libcpp_string_gets_noexcept_iterator<_InputIterator>::value,
-    basic_string<_CharT, _Traits, _Allocator>&
->::type
-basic_string<_CharT, _Traits, _Allocator>::append(_InputIterator __first, _InputIterator __last)
-{
-    const basic_string __temp (__first, __last, __alloc());
-    append(__temp.data(), __temp.size());
-    return *this;
-}
-
 template <class _Tp>
 bool __ptr_in_range (const _Tp* __p, const _Tp* __first, const _Tp* __last)
 {
@@ -2211,14 +2207,12 @@ bool __ptr_in_range (const _Tp1* __p, co
 
 template <class _CharT, class _Traits, class _Allocator>
 template<class _ForwardIterator>
-typename enable_if
-<
-    __is_forward_iterator<_ForwardIterator>::value
-          && __libcpp_string_gets_noexcept_iterator<_ForwardIterator>::value,
-    basic_string<_CharT, _Traits, _Allocator>&
->::type
-basic_string<_CharT, _Traits, _Allocator>::append(_ForwardIterator __first, _ForwardIterator __last)
+basic_string<_CharT, _Traits, _Allocator>&
+basic_string<_CharT, _Traits, _Allocator>::__append_forward_unsafe(
+    _ForwardIterator __first, _ForwardIterator __last)
 {
+    static_assert(__is_forward_iterator<_ForwardIterator>::value,
+                  "function requires a ForwardIterator");
     size_type __sz = size();
     size_type __cap = capacity();
     size_type __n = static_cast<size_type>(_VSTD::distance(__first, __last));




More information about the cfe-commits mailing list