Fix Label::hash() computation.
This CL fixes a subtle bug where the hash_ member of a Label
instance was not properly initialized after a call to ::Resolve()
in label.cc, and a few other trivial cases. This prevented the
proper use of std::unordered_map<Label, ...> types.
+ Slightly speed up Label::ComputeHash() by avoiding string hashing
whenever possible, using the newly-introduced StringAtom::ptr_hash()
method (which replicates std::hash<StringAtom>()()).
Measurements show no impact on performance or RAM usage.
Change-Id: I11b6645709cc8c8ff266bb5174c1bdd102919332
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/12580
Commit-Queue: David Turner <digit@google.com>
Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/label.cc b/src/gn/label.cc
index c78b00e..6914962 100644
--- a/src/gn/label.cc
+++ b/src/gn/label.cc
@@ -289,6 +289,8 @@
input_string, &ret.dir_, &ret.name_, &ret.toolchain_dir_,
&ret.toolchain_name_, err))
return Label();
+
+ ret.hash_ = ret.ComputeHash();
return ret;
}
diff --git a/src/gn/label.h b/src/gn/label.h
index 1458fcc..e2a0281 100644
--- a/src/gn/label.h
+++ b/src/gn/label.h
@@ -70,8 +70,8 @@
std::string GetUserVisibleName(const Label& default_toolchain) const;
bool operator==(const Label& other) const {
- return name_.SameAs(other.name_) && dir_ == other.dir_ &&
- toolchain_dir_ == other.toolchain_dir_ &&
+ return hash_ == other.hash_ && name_.SameAs(other.name_) &&
+ dir_ == other.dir_ && toolchain_dir_ == other.toolchain_dir_ &&
toolchain_name_.SameAs(other.toolchain_name_);
}
bool operator!=(const Label& other) const { return !operator==(other); }
@@ -101,7 +101,8 @@
size_t hash() const { return hash_; }
private:
- Label(SourceDir dir, StringAtom name) : dir_(dir), name_(name) {}
+ Label(SourceDir dir, StringAtom name)
+ : dir_(dir), name_(name), hash_(ComputeHash()) {}
Label(SourceDir dir,
StringAtom name,
@@ -110,13 +111,14 @@
: dir_(dir),
name_(name),
toolchain_dir_(toolchain_dir),
- toolchain_name_(toolchain_name) {}
+ toolchain_name_(toolchain_name),
+ hash_(ComputeHash()) {}
size_t ComputeHash() const {
size_t h0 = dir_.hash();
- size_t h1 = name_.hash();
+ size_t h1 = name_.ptr_hash();
size_t h2 = toolchain_dir_.hash();
- size_t h3 = toolchain_name_.hash();
+ size_t h3 = toolchain_name_.ptr_hash();
return ((h3 * 131 + h2) * 131 + h1) * 131 + h0;
}
diff --git a/src/gn/source_dir.h b/src/gn/source_dir.h
index d8b3c28..2a694cf 100644
--- a/src/gn/source_dir.h
+++ b/src/gn/source_dir.h
@@ -128,7 +128,7 @@
bool operator!=(const SourceDir& other) const { return !operator==(other); }
bool operator<(const SourceDir& other) const { return value_ < other.value_; }
- size_t hash() const { return value_.hash(); }
+ size_t hash() const { return value_.ptr_hash(); }
private:
friend class SourceFile;
diff --git a/src/gn/source_file.h b/src/gn/source_file.h
index b13a400..467c119 100644
--- a/src/gn/source_file.h
+++ b/src/gn/source_file.h
@@ -98,7 +98,7 @@
}
bool operator==(const SourceFile& other) const {
- return value_ == other.value_;
+ return value_.SameAs(other.value_);
}
bool operator!=(const SourceFile& other) const { return !operator==(other); }
bool operator<(const SourceFile& other) const {
diff --git a/src/gn/string_atom.h b/src/gn/string_atom.h
index 541399e..c590e0b 100644
--- a/src/gn/string_atom.h
+++ b/src/gn/string_atom.h
@@ -119,8 +119,10 @@
size_t hash() const { return std::hash<std::string>()(value_); }
- // Use the following structs to implement containers that use StringAtom
- // values as keys, but only compare/hash the pointer values for speed.
+ // Use the following method and structs to implement containers that
+ // use StringAtom values as keys, but only compare/hash the pointer
+ // values for speed.
+ //
// E.g.:
// using FastSet = std::unordered_set<StringAtom, PtrHash, PtrEqual>;
// using FastMap = std::map<StringAtom, Value, PtrCompare>;
@@ -128,9 +130,11 @@
// IMPORTANT: Note that FastMap above is ordered based in the StringAtom
// pointer value, not the string content.
//
+ size_t ptr_hash() const { return std::hash<const std::string*>()(&value_); }
+
struct PtrHash {
size_t operator()(const StringAtom& key) const noexcept {
- return std::hash<const std::string*>()(&key.value_);
+ return key.ptr_hash();
}
};
