[libcxx-commits] [libcxx] [libc++] Avoid string reallocation in `std::filesystem::path::lexically_relative` (PR #152964)

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Timothy Choi (tinnamchoi)

<details>
<summary>Changes</summary>

Improves runtime by around 20 to 40%.

```
Benchmark                                                           Time             CPU      Time Old      Time New       CPU Old       CPU New
------------------------------------------------------------------------------------------------------------------------------------------------
BM_LexicallyRelative/small_path/2                                -0.2111         -0.2082           229           181           228           180
BM_LexicallyRelative/small_path/4                                -0.2579         -0.2550           455           338           452           337
BM_LexicallyRelative/small_path/8                                -0.2643         -0.2616           844           621           838           619
BM_LexicallyRelative/small_path/16                               -0.2582         -0.2556          1562          1158          1551          1155
BM_LexicallyRelative/small_path/32                               -0.2518         -0.2496          3023          2262          3004          2254
BM_LexicallyRelative/small_path/64                               -0.2806         -0.2775          6344          4564          6295          4549
BM_LexicallyRelative/small_path/128                              -0.2165         -0.2137         11762          9216         11683          9186
BM_LexicallyRelative/small_path/256                              -0.2672         -0.2645         24499         17953         24324         17891
BM_LexicallyRelative/large_path/2                                -0.3268         -0.3236           426           287           422           285
BM_LexicallyRelative/large_path/4                                -0.3274         -0.3248           734           494           729           492
BM_LexicallyRelative/large_path/8                                -0.3586         -0.3560          1409           904          1399           901
BM_LexicallyRelative/large_path/16                               -0.3978         -0.3951          2764          1665          2743          1659
BM_LexicallyRelative/large_path/32                               -0.3934         -0.3908          5323          3229          5283          3218
BM_LexicallyRelative/large_path/64                               -0.3629         -0.3605         10340          6587         10265          6564
BM_LexicallyRelative/large_path/128                              -0.3450         -0.3423         19379         12694         19233         12649
BM_LexicallyRelative/large_path/256                              -0.3097         -0.3054         36293         25052         35943         24965
```


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


2 Files Affected:

- (modified) libcxx/src/filesystem/path.cpp (+3-1) 
- (modified) libcxx/test/benchmarks/filesystem.bench.cpp (+21) 


``````````diff
diff --git a/libcxx/src/filesystem/path.cpp b/libcxx/src/filesystem/path.cpp
index 9f7dc54fdf156..624cf785577eb 100644
--- a/libcxx/src/filesystem/path.cpp
+++ b/libcxx/src/filesystem/path.cpp
@@ -292,7 +292,9 @@ path path::lexically_relative(const path& base) const {
   // return a path constructed with 'n' dot-dot elements, followed by the
   // elements of '*this' after the mismatch.
   path Result;
-  // FIXME: Reserve enough room in Result that it won't have to re-allocate.
+  constexpr size_t ElemSize = 2; // ".."
+  constexpr size_t SeparatorSize = 1; // separator is always a single char
+  Result.__reserve(ElemCount * (ElemSize + SeparatorSize) + SeparatorSize + PP.Path.size());
   while (ElemCount--)
     Result /= PATHSTR("..");
   for (; PP; ++PP)
diff --git a/libcxx/test/benchmarks/filesystem.bench.cpp b/libcxx/test/benchmarks/filesystem.bench.cpp
index dc6b0ac537f7e..f2e596238a922 100644
--- a/libcxx/test/benchmarks/filesystem.bench.cpp
+++ b/libcxx/test/benchmarks/filesystem.bench.cpp
@@ -171,4 +171,25 @@ BENCHMARK_CAPTURE(BM_LexicallyNormal, large_path, getRandomPaths, /*PathLen*/ 32
     ->Range(2, 256)
     ->Complexity();
 
+template <class GenInput>
+void BM_LexicallyRelative(benchmark::State& st, GenInput gen, size_t PathLen) {
+  using fs::path;
+  auto BasePath = gen(st.range(0), PathLen);
+  auto TargetPath = gen(st.range(0), PathLen);
+  benchmark::DoNotOptimize(&BasePath);
+  benchmark::DoNotOptimize(&TargetPath);
+  while (st.KeepRunning()) {
+    benchmark::DoNotOptimize(TargetPath.lexically_relative(BasePath));
+  }
+  st.SetComplexityN(st.range(0));
+}
+BENCHMARK_CAPTURE(BM_LexicallyRelative, small_path, getRandomPaths, /*PathLen*/ 5)
+    ->RangeMultiplier(2)
+    ->Range(2, 256)
+    ->Complexity();
+BENCHMARK_CAPTURE(BM_LexicallyRelative, large_path, getRandomPaths, /*PathLen*/ 32)
+    ->RangeMultiplier(2)
+    ->Range(2, 256)
+    ->Complexity();
+
 BENCHMARK_MAIN();

``````````

</details>


https://github.com/llvm/llvm-project/pull/152964