Apply a short-vector optimization to EvalString.

This very often holds only a single RAW token, so we do not need to allocate elements on an std::vector for it in the common case. For a no-op build of Chromium (Linux, Zen 2), this reduces time spent from 5.48 to 5.14 seconds. Note that this opens up for a potential optimization where EvalString::Evaluate() could just return a StringPiece, without making a std::string out of it (which requires allocation; this is about 5% of remaining runtime). However, this would also require that CanonicalizePath() somehow learned to work with StringPiece (presumably allocating a new StringPiece if and only if changes were needed).
digit-google · Nov 4, 2024 · 666e448 · 666e448
1 parent cf774e6
commit 666e448
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 47 deletions.
diff --git a/src/eval_env.cc b/src/eval_env.cc
@@ -16,18 +16,16 @@
 
 #include "eval_env.h"
 
-using namespace std;
-
-string BindingEnv::LookupVariable(const string& var) {
-  map<string, string>::iterator i = bindings_.find(var);
+std::string BindingEnv::LookupVariable(const std::string& var) {
+  auto i = bindings_.find(var);
   if (i != bindings_.end())
     return i->second;
   if (parent_)
     return parent_->LookupVariable(var);
   return "";
 }
 
-void BindingEnv::AddBinding(const string& key, const string& val) {
+void BindingEnv::AddBinding(const std::string& key, const std::string& val) {
   bindings_[key] = val;
 }
 
@@ -36,35 +34,35 @@ void BindingEnv::AddRule(const Rule* rule) {
   rules_[rule->name()] = rule;
 }
 
-const Rule* BindingEnv::LookupRuleCurrentScope(const string& rule_name) {
-  map<string, const Rule*>::iterator i = rules_.find(rule_name);
+const Rule* BindingEnv::LookupRuleCurrentScope(const std::string& rule_name) {
+  auto i = rules_.find(rule_name);
   if (i == rules_.end())
     return NULL;
   return i->second;
 }
 
-const Rule* BindingEnv::LookupRule(const string& rule_name) {
-  map<string, const Rule*>::iterator i = rules_.find(rule_name);
+const Rule* BindingEnv::LookupRule(const std::string& rule_name) {
+  auto i = rules_.find(rule_name);
   if (i != rules_.end())
     return i->second;
   if (parent_)
     return parent_->LookupRule(rule_name);
   return NULL;
 }
 
-void Rule::AddBinding(const string& key, const EvalString& val) {
+void Rule::AddBinding(const std::string& key, const EvalString& val) {
   bindings_[key] = val;
 }
 
-const EvalString* Rule::GetBinding(const string& key) const {
+const EvalString* Rule::GetBinding(const std::string& key) const {
   Bindings::const_iterator i = bindings_.find(key);
   if (i == bindings_.end())
     return NULL;
   return &i->second;
 }
 
 // static
-bool Rule::IsReservedBinding(const string& var) {
+bool Rule::IsReservedBinding(const std::string& var) {
   return var == "command" ||
       var == "depfile" ||
       var == "dyndep" ||
@@ -78,14 +76,13 @@ bool Rule::IsReservedBinding(const string& var) {
       var == "msvc_deps_prefix";
 }
 
-const map<string, const Rule*>& BindingEnv::GetRules() const {
+const std::map<std::string, const Rule*>& BindingEnv::GetRules() const {
   return rules_;
 }
 
-string BindingEnv::LookupWithFallback(const string& var,
-                                      const EvalString* eval,
-                                      Env* env) {
-  map<string, string>::iterator i = bindings_.find(var);
+std::string BindingEnv::LookupWithFallback(const std::string& var,
+                                           const EvalString* eval, Env* env) {
+  auto i = bindings_.find(var);
   if (i != bindings_.end())
     return i->second;
 
@@ -98,52 +95,72 @@ string BindingEnv::LookupWithFallback(const string& var,
   return "";
 }
 
-string EvalString::Evaluate(Env* env) const {
-  string result;
-  for (TokenList::const_iterator i = parsed_.begin(); i != parsed_.end(); ++i) {
-    if (i->second == RAW)
-      result.append(i->first);
+std::string EvalString::Evaluate(Env* env) const {
+  if (parsed_.empty()) {
+    return single_token_;
+  }
+
+  std::string result;
+  for (const auto& pair : parsed_) {
+    if (pair.second == RAW)
+      result.append(pair.first);
     else
-      result.append(env->LookupVariable(i->first));
+      result.append(env->LookupVariable(pair.first));
   }
   return result;
 }
 
 void EvalString::AddText(StringPiece text) {
-  // Add it to the end of an existing RAW token if possible.
-  if (!parsed_.empty() && parsed_.back().second == RAW) {
-    parsed_.back().first.append(text.str_, text.len_);
+  if (parsed_.empty()) {
+    single_token_.append(text.begin(), text.end());
+  } else if (!parsed_.empty() && parsed_.back().second == RAW) {
+    parsed_.back().first.append(text.begin(), text.end());
   } else {
-    parsed_.push_back(make_pair(text.AsString(), RAW));
+    parsed_.push_back(std::make_pair(text.AsString(), RAW));
   }
 }
+
 void EvalString::AddSpecial(StringPiece text) {
-  parsed_.push_back(make_pair(text.AsString(), SPECIAL));
+  if (parsed_.empty() && !single_token_.empty()) {
+    // Going from one to two tokens, so we can no longer apply
+    // our single_token_ optimization and need to push everything
+    // onto the vector.
+    parsed_.push_back(std::make_pair(std::move(single_token_), RAW));
+  }
+  parsed_.push_back(std::make_pair(text.AsString(), SPECIAL));
 }
 
-string EvalString::Serialize() const {
-  string result;
-  for (TokenList::const_iterator i = parsed_.begin();
-       i != parsed_.end(); ++i) {
+std::string EvalString::Serialize() const {
+  std::string result;
+  if (parsed_.empty() && !single_token_.empty()) {
     result.append("[");
-    if (i->second == SPECIAL)
-      result.append("$");
-    result.append(i->first);
+    result += single_token_;
     result.append("]");
+  } else {
+    for (const auto& pair : parsed_) {
+      result.append("[");
+      if (pair.second == SPECIAL)
+        result.append("$");
+      result += pair.first;
+      result.append("]");
+    }
   }
   return result;
 }
 
-string EvalString::Unparse() const {
-  string result;
-  for (TokenList::const_iterator i = parsed_.begin();
-       i != parsed_.end(); ++i) {
-    bool special = (i->second == SPECIAL);
-    if (special)
-      result.append("${");
-    result.append(i->first);
-    if (special)
-      result.append("}");
+std::string EvalString::Unparse() const {
+  std::string result;
+  if (parsed_.empty() && !single_token_.empty()) {
+    result = single_token_;
+  } else {
+    for (const auto& pair : parsed_) {
+      bool special = (pair.second == SPECIAL);
+      if (special)
+        result.append("${");
+      result += pair.first;
+      if (special)
+        result.append("}");
+    }
   }
   return result;
 }
diff --git a/src/eval_env.h b/src/eval_env.h
@@ -39,8 +39,11 @@ struct EvalString {
   /// @return The string with variables not expanded.
   std::string Unparse() const;
 
-  void Clear() { parsed_.clear(); }
-  bool empty() const { return parsed_.empty(); }
+  void Clear() {
+    parsed_.clear();
+    single_token_.clear();
+  }
+  bool empty() const { return parsed_.empty() && single_token_.empty(); }
 
   void AddText(StringPiece text);
   void AddSpecial(StringPiece text);
@@ -53,6 +56,12 @@ struct EvalString {
   enum TokenType { RAW, SPECIAL };
   typedef std::vector<std::pair<std::string, TokenType> > TokenList;
   TokenList parsed_;
+
+  // If we hold only a single RAW token, then we keep it here instead of
+  // pushing it on TokenList. This saves a bunch of allocations for
+  // what is a common case. If parsed_ is nonempty, then this value
+  // must be ignored.
+  std::string single_token_;
 };
 
 /// An invocable build command and associated metadata (description, etc.).

diff --git a/src/string_piece.h b/src/string_piece.h
@@ -63,6 +63,8 @@ struct StringPiece {
     return len_;
   }
 
+  size_t empty() const { return len_ == 0; }
+
   const char* str_;
   size_t len_;
 };