Add a small speed up to PathDecanonicalized

Avoid a mandatory allocation in `PathDecanonicalized` by taking an output
`std::string*` to append to. Improve its performance by swapping
`strchr` with `std::string::find`, which can take advantage of knowing
the length of the string.

We can also return as soon as we run out of forwardslashes that need to be
turned into backslashes, instead of iterating through all
forwardslashes. If backslashes are the common path separator in ninja
build files on Windows, it may be worthwhile canonicalizing paths into
backslashes-only, which would now allow us to skip the loop entirely in
`PathDecanonicalized` in the case of paths with no forwardslashes.

Rename this function to `AppendPathDecanonicalized` to reflect its new
functionality.

Running `manifest_parser_perftest.exe` on Windows gives initial timings
of:

    min 549ms  max 613ms  avg 573.1ms

and timings after this change of:

    min 534ms  max 597ms  avg 559.9ms

which is a 2-3% improvement.

Author: elliotgoodrich

src/graph.cc

@@ -484,18 +484,20 @@ string EdgeEnv::LookupVariable(const string& var) {
 std::string EdgeEnv::MakePathList(const Node* const* const span,
                                   const size_t size, const char sep) const {
   string result;
+  std::string temp_path;
   for (const Node* const* i = span; i != span + size; ++i) {
     if (!result.empty())
       result.push_back(sep);
-    const string& path = (*i)->PathDecanonicalized();
     if (escape_in_out_ == kShellEscape) {
+      temp_path.clear();
+      (*i)->AppendPathDecanonicalized(&temp_path);
 #ifdef _WIN32
-      GetWin32EscapedString(path, &result);
+      GetWin32EscapedString(temp_path, &result);
 #else
-      GetShellEscapedString(path, &result);
+      GetShellEscapedString(temp_path, &result);
 #endif
     } else {
-      result.append(path);
+      (*i)->AppendPathDecanonicalized(&result);
     }
   }
   return result;
@@ -580,18 +582,29 @@ bool Edge::maybe_phonycycle_diagnostic() const {
 }
 
 // static
-string Node::PathDecanonicalized(const string& path, uint64_t slash_bits) {
-  string result = path;
+void Node::AppendPathDecanonicalized(std::string* output, StringPiece path,
+                                     uint64_t slash_bits) {
+  if (path.empty()) {
+    return;
+  }
+  output->append(path.begin(), path.end());
 #ifdef _WIN32
-  uint64_t mask = 1;
-  for (char* c = &result[0]; (c = strchr(c, '/')) != NULL;) {
-    if (slash_bits & mask)
+  char* data = &(*output)[0];
+  char* end = data + output->size();
+  char* c = end - path.size();
+  while (slash_bits != 0) {
+    c = static_cast<char*>(memchr(c, '/', end - c));
+
+    // If `memchr` returns null then there are too many bits in `slash_bits`,
+    // which happens when we pass `~0` in `build.cc` instead of calculating it.
+    if (!c)
+      return;
+    if (slash_bits & 1)
       *c = '\\';
-    c++;
-    mask <<= 1;
+    ++c;
+    slash_bits >>= 1;
   }
 #endif
-  return result;
 }
 
 void Node::Dump(const char* prefix) const {
@@ -788,22 +801,23 @@ void InputsCollector::VisitNode(const Node* node) {
 
 std::vector<std::string> InputsCollector::GetInputsAsStrings(
     bool shell_escape) const {
-  std::vector<std::string> result;
-  result.reserve(inputs_.size());
+  std::vector<std::string> result(inputs_.size());
+  auto out = result.begin();
 
+  std::string temp_path;
   for (const Node* input : inputs_) {
-    std::string unescaped = input->PathDecanonicalized();
     if (shell_escape) {
-      std::string path;
+      temp_path.clear();
+      input->AppendPathDecanonicalized(&temp_path);
 #ifdef _WIN32
-      GetWin32EscapedString(unescaped, &path);
+      GetWin32EscapedString(temp_path, &*out);
 #else
-      GetShellEscapedString(unescaped, &path);
+      GetShellEscapedString(temp_path, &*out);
 #endif
-      result.push_back(std::move(path));
     } else {
-      result.push_back(std::move(unescaped));
+      input->AppendPathDecanonicalized(&*out);
     }
+    ++out;
   }
   return result;
 }

src/graph.h

@@ -81,11 +81,11 @@ struct Node {
 
   const std::string& path() const { return path_; }
   /// Get |path()| but use slash_bits to convert back to original slash styles.
-  std::string PathDecanonicalized() const {
-    return PathDecanonicalized(path_, slash_bits_);
+  void AppendPathDecanonicalized(std::string* output) const {
+    return AppendPathDecanonicalized(output, path_, slash_bits_);
   }
-  static std::string PathDecanonicalized(const std::string& path,
-                                         uint64_t slash_bits);
+  static void AppendPathDecanonicalized(std::string* output, StringPiece path,
+                                        uint64_t slash_bits);
   uint64_t slash_bits() const { return slash_bits_; }
 
   TimeStamp mtime() const { return mtime_; }
@@ -122,7 +122,7 @@ struct Node {
   std::string path_;
 
   /// Set bits starting from lowest for backslashes that were normalized to
-  /// forward slashes by CanonicalizePath. See |PathDecanonicalized|.
+  /// forward slashes by CanonicalizePath. See |AppendPathDecanonicalized|.
   uint64_t slash_bits_ = 0;
 
   /// Possible values of mtime_:

src/graph_test.cc

@@ -619,10 +619,35 @@ TEST_F(GraphTest, Decanonicalize) {
   EXPECT_EQ(root_nodes[1]->path(), "out/out2/out3/out4");
   EXPECT_EQ(root_nodes[2]->path(), "out3");
   EXPECT_EQ(root_nodes[3]->path(), "out4/foo");
-  EXPECT_EQ(root_nodes[0]->PathDecanonicalized(), "out\\out1");
-  EXPECT_EQ(root_nodes[1]->PathDecanonicalized(), "out\\out2/out3\\out4");
-  EXPECT_EQ(root_nodes[2]->PathDecanonicalized(), "out3");
-  EXPECT_EQ(root_nodes[3]->PathDecanonicalized(), "out4\\foo");
+
+  std::string decanon;
+  root_nodes[0]->AppendPathDecanonicalized(&decanon);
+  EXPECT_EQ(decanon, "out\\out1");
+
+  // Check we are indeed appending to the end
+  decanon = "text at the start:";
+  root_nodes[0]->AppendPathDecanonicalized(&decanon);
+  EXPECT_EQ(decanon, "text at the start:out\\out1");
+
+  decanon.clear();
+  root_nodes[1]->AppendPathDecanonicalized(&decanon);
+  EXPECT_EQ(decanon, "out\\out2/out3\\out4");
+
+  decanon.clear();
+  root_nodes[2]->AppendPathDecanonicalized(&decanon);
+  EXPECT_EQ(decanon, "out3");
+
+  decanon.clear();
+  root_nodes[3]->AppendPathDecanonicalized(&decanon);
+  EXPECT_EQ(decanon, "out4\\foo");
+
+  // In build.cc there is one place we pass `~0` as the slash_bits
+  // because we assume the output is only backslashes and the existing
+  // code doesn't bother to calculate it.  Ensure this doesn't cause
+  // a problem by indicating there are more backslashes then there are.
+  decanon = "/path/=";
+  Node::AppendPathDecanonicalized(&decanon, "a/b/c", ~0);
+  EXPECT_EQ(decanon, "/path/=a\\b\\c");
 }
 #endif
 

src/ninja.cc

@@ -351,8 +351,9 @@ Node* NinjaMain::CollectTarget(const char* cpath, string* err) {
     }
     return node;
   } else {
-    *err =
-        "unknown target '" + Node::PathDecanonicalized(path, slash_bits) + "'";
+    *err = "unknown target '";
+    Node::AppendPathDecanonicalized(err, path, slash_bits);
+    *err += "'";
     if (path == "clean") {
       *err += ", did you mean 'ninja -t clean'?";
     } else if (path == "help") {