[Lldb-commits] [PATCH] D125505: Cache tilde expansions to avoid redundant lookups

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

We have tilde's in our dSYMs internally, and expanding these can involve a network access that is slow for some people.  I'm not worried about tilde expanding to a different filepath during the lifetime of a single lldb session, and I believe people will see very few different tildes ever expanded -- I'd be surprised if anyone exceed five different tilde's -- so this would be something easy to cache in lldb given how TildeExpressionResolver::ResolveFullPath() is structured.

This patch creates a simple vector of <ConstString, std::string>'s with the tilde names (ConstString to make comparisons cheap) and the expanded filepath.  The method accepts a filepath as a StringRef and returns an expanded path in a SmallVectorImpl<char>, but I don't think the specific container used is important one way or the other.  I'm not wedded to this impl though.

This bit of code has been untouched in years, but adding Jonas who did touch it a couple years ago.

rdar://77091379


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125505

Files:
  lldb/source/Utility/TildeExpressionResolver.cpp


Index: lldb/source/Utility/TildeExpressionResolver.cpp
===================================================================
--- lldb/source/Utility/TildeExpressionResolver.cpp
+++ lldb/source/Utility/TildeExpressionResolver.cpp
@@ -11,6 +11,7 @@
 #include <cassert>
 #include <system_error>
 
+#include "lldb/Utility/ConstString.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/FileSystem.h"
@@ -75,6 +76,7 @@
 
 bool TildeExpressionResolver::ResolveFullPath(
     StringRef Expr, llvm::SmallVectorImpl<char> &Output) {
+  static std::vector<std::tuple<ConstString, std::string>> g_tildes_expanded;
   if (!Expr.startswith("~")) {
     Output.assign(Expr.begin(), Expr.end());
     return false;
@@ -84,11 +86,27 @@
   StringRef Left =
       Expr.take_until([](char c) { return path::is_separator(c); });
 
-  if (!ResolveExact(Left, Output)) {
-    Output.assign(Expr.begin(), Expr.end());
-    return false;
+  ConstString tilde_name(Left);
+  std::string expanded_path;
+  for (const auto &it : g_tildes_expanded) {
+    if (std::get<0>(it) == tilde_name) {
+      expanded_path = std::get<1>(it);
+      break;
+    }
   }
 
-  Output.append(Expr.begin() + Left.size(), Expr.end());
+  if (expanded_path.empty()) {
+    if (!ResolveExact(Left, Output)) {
+      Output.assign(Expr.begin(), Expr.end());
+      return false;
+    }
+    expanded_path = std::string(Output.data(), Output.size());
+    g_tildes_expanded.push_back(
+        std::tuple<ConstString, std::string>(tilde_name, expanded_path));
+  } else {
+    StringRef expanded_strref(expanded_path);
+    Output.assign(expanded_path.begin(), expanded_path.end());
+  }
+  Output.append(Expr.begin() + tilde_name.GetLength(), Expr.end());
   return true;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D125505.429090.patch
Type: text/x-patch
Size: 1770 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220512/e79ae33f/attachment.bin>