[libcxx] r285530 - Improve performance of constructing filesystem::path from strings.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Sun Oct 30 18:53:50 2016
New Revision: 285530

URL: http://llvm.org/viewvc/llvm-project?rev=285530&view=rev
Log:
Improve performance of constructing filesystem::path from strings.

This patch fixes a performance bug when constructing or appending to a path
from a string or c-string. Previously we called 'push_back' to append every
single character. This caused multiple re-allocation and copies when at most
one reallocation is necessary. The new behavior is to simply call
`string::append` so it can correctly handle reallocation.

For large strings this change is a ~4x improvement. This also makes our path
faster to construct than libstdc++'s.

Modified:
    libcxx/trunk/benchmarks/filesystem.bench.cpp
    libcxx/trunk/include/experimental/filesystem
    libcxx/trunk/src/experimental/filesystem/path.cpp

Modified: libcxx/trunk/benchmarks/filesystem.bench.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/benchmarks/filesystem.bench.cpp?rev=285530&r1=285529&r2=285530&view=diff
==============================================================================
--- libcxx/trunk/benchmarks/filesystem.bench.cpp (original)
+++ libcxx/trunk/benchmarks/filesystem.bench.cpp Sun Oct 30 18:53:50 2016
@@ -21,11 +21,27 @@ void BM_PathConstructString(benchmark::S
     benchmark::DoNotOptimize(P.native().data());
   }
 }
-BENCHMARK_CAPTURE(BM_PathConstructString, iterate_elements,
+BENCHMARK_CAPTURE(BM_PathConstructString, large_string,
   getRandomStringInputs)->Arg(TestNumInputs);
 
 
 template <class GenInputs>
+void BM_PathConstructCStr(benchmark::State &st, GenInputs gen) {
+  using namespace fs;
+  const auto in = gen(st.range(0));
+  path PP;
+  for (auto& Part : in)
+    PP /= Part;
+  benchmark::DoNotOptimize(PP.native().data());
+  while (st.KeepRunning()) {
+    const path P(PP.native().c_str());
+    benchmark::DoNotOptimize(P.native().data());
+  }
+}
+BENCHMARK_CAPTURE(BM_PathConstructCStr, large_string,
+  getRandomStringInputs)->Arg(TestNumInputs);
+
+template <class GenInputs>
 void BM_PathIterateMultipleTimes(benchmark::State &st, GenInputs gen) {
   using namespace fs;
   const auto in = gen(st.range(0));
@@ -85,6 +101,4 @@ void BM_PathIterateOnceBackwards(benchma
 BENCHMARK_CAPTURE(BM_PathIterateOnceBackwards, iterate_elements,
   getRandomStringInputs)->Arg(TestNumInputs);
 
-
-
 BENCHMARK_MAIN()

Modified: libcxx/trunk/include/experimental/filesystem
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/filesystem?rev=285530&r1=285529&r2=285530&view=diff
==============================================================================
--- libcxx/trunk/include/experimental/filesystem (original)
+++ libcxx/trunk/include/experimental/filesystem Sun Oct 30 18:53:50 2016
@@ -623,10 +623,10 @@ struct _PathCVT {
 
 template <>
 struct _PathCVT<char> {
+
     template <class _Iter>
     static void __append_range(string& __dest, _Iter __b, _Iter __e) {
-        for (; __b != __e; ++__b)
-            __dest.push_back(*__b);
+        __dest.append(__b, __e);
     }
 
     template <class _Iter>
@@ -640,7 +640,8 @@ struct _PathCVT<char> {
     static void __append_source(string& __dest, _Source const& __s)
     {
         using _Traits = __is_pathable<_Source>;
-        __append_range(__dest, _Traits::__range_begin(__s), _Traits::__range_end(__s));
+        __append_range(__dest, _Traits::__range_begin(__s),
+                               _Traits::__range_end(__s));
     }
 };
 

Modified: libcxx/trunk/src/experimental/filesystem/path.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/path.cpp?rev=285530&r1=285529&r2=285530&view=diff
==============================================================================
--- libcxx/trunk/src/experimental/filesystem/path.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/path.cpp Sun Oct 30 18:53:50 2016
@@ -42,8 +42,7 @@ private:
 public:
   PathParser(string_view_t P, string_view_t E, unsigned char S)
       : Path(P), RawEntry(E), State(static_cast<ParserState>(S)) {
-    assert(S != 0);
-    assert(S != PS_BeforeBegin);
+    // S cannot be '0' or PS_BeforeBegin.
   }
 
   static PathParser CreateBegin(string_view_t P) noexcept {
@@ -94,7 +93,6 @@ public:
 
     case PS_InFilenames: {
       PosPtr SepEnd = consumeSeparator(Start, End);
-      assert(SepEnd);
       if (SepEnd != End) {
         PosPtr TkEnd = consumeName(SepEnd, End);
         if (TkEnd)
@@ -131,7 +129,6 @@ public:
                          SepEnd + 1, RStart + 1);
       } else {
         PosPtr TkStart = consumeName(RStart, REnd);
-        assert(TkStart);
         if (TkStart == REnd + 2 && consumeSeparator(TkStart, REnd) == REnd)
           return makeState(PS_InRootName, Path.data(), RStart + 1);
         else
@@ -192,14 +189,10 @@ public:
 
 private:
   void makeState(ParserState NewState, PosPtr Start, PosPtr End) noexcept {
-    assert(NewState != PS_BeforeBegin && NewState != PS_AtEnd);
     State = NewState;
-    assert(Start < End);
-    assert(Start >= &Path.front() && End <= &Path.back() + 1);
     RawEntry = string_view_t(Start, End - Start);
   }
   void makeState(ParserState NewState) noexcept {
-    assert(NewState == PS_BeforeBegin || NewState == PS_AtEnd);
     State = NewState;
     RawEntry = {};
   }