Use std::set instead of std::unordered_set The recommendation in Chromium is to avoid using std::unordered_set and instead to use base::flat_set if the number of items is small or std::set if unsure. Given that there are many targets, use std::set<>. Remove some unnecessary include of unorderset_set. Bug: none Change-Id: I2a9d1026e4458a89f3bdb41ab6869cd11d193acb Reviewed-on: https://gn-review.googlesource.com/c/gn/+/11402 Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
diff --git a/src/gn/ninja_build_writer.cc b/src/gn/ninja_build_writer.cc index 28e2fa6..9fb6a7d 100644 --- a/src/gn/ninja_build_writer.cc +++ b/src/gn/ninja_build_writer.cc
@@ -8,8 +8,8 @@ #include <fstream> #include <map> +#include <set> #include <sstream> -#include <unordered_set> #include "base/command_line.h" #include "base/files/file_util.h" @@ -25,6 +25,7 @@ #include "gn/ninja_utils.h" #include "gn/pool.h" #include "gn/scheduler.h" +#include "gn/string_atom.h" #include "gn/switches.h" #include "gn/target.h" #include "gn/trace.h" @@ -339,7 +340,7 @@ void NinjaBuildWriter::WriteAllPools() { // Compute the pools referenced by all tools of all used toolchains. - std::unordered_set<const Pool*> used_pools; + std::set<const Pool*> used_pools; for (const auto& pair : used_toolchains_) { for (const auto& tool : pair.second->tools()) { if (tool.second->pool().ptr) @@ -466,8 +467,8 @@ // Track rules as we generate them so we don't accidentally write a phony // rule that collides with something else. // GN internally generates an "all" target, so don't duplicate it. - std::unordered_set<std::string> written_rules; - written_rules.insert("all"); + std::set<StringAtom, StringAtom::PtrCompare> written_rules; + written_rules.insert(StringAtom("all")); // Set if we encounter a target named "//:default". const Target* default_target = nullptr; @@ -532,7 +533,9 @@ // with "./". std::string output_string(output.value()); NormalizePath(&output_string); - if (!written_rules.insert(output_string).second) { + const StringAtom output_string_atom(output_string); + + if (!written_rules.insert(output_string_atom).second) { *err = GetDuplicateOutputError(default_toolchain_targets_, output); return false; } @@ -541,14 +544,14 @@ // First prefer the short names of toplevel targets. for (const Target* target : toplevel_targets) { - if (written_rules.insert(target->label().name()).second) - WritePhonyRule(target, target->label().name()); + if (written_rules.insert(target->label().name_atom()).second) + WritePhonyRule(target, target->label().name_atom()); } // Next prefer short names of toplevel dir targets. for (const Target* target : toplevel_dir_targets) { - if (written_rules.insert(target->label().name()).second) - WritePhonyRule(target, target->label().name()); + if (written_rules.insert(target->label().name_atom()).second) + WritePhonyRule(target, target->label().name_atom()); } // Write out the names labels of executables. Many toolchains will produce @@ -563,7 +566,7 @@ // toplevel build rule. for (const auto& pair : exes) { const Counts& counts = pair.second; - const std::string& short_name = counts.last_seen->label().name(); + const StringAtom& short_name = counts.last_seen->label().name_atom(); if (counts.count == 1 && written_rules.insert(short_name).second) WritePhonyRule(counts.last_seen, short_name); } @@ -571,7 +574,7 @@ // Write short names when those names are unique and not already taken. for (const auto& pair : short_names) { const Counts& counts = pair.second; - const std::string& short_name = counts.last_seen->label().name(); + const StringAtom& short_name = counts.last_seen->label().name_atom(); if (counts.count == 1 && written_rules.insert(short_name).second) WritePhonyRule(counts.last_seen, short_name); } @@ -583,19 +586,22 @@ // Write the long name "foo/bar:baz" for the target "//foo/bar:baz". std::string long_name = label.GetUserVisibleName(false); base::TrimString(long_name, "/", &long_name); - if (written_rules.insert(long_name).second) - WritePhonyRule(target, long_name); + const StringAtom long_name_atom(long_name); + if (written_rules.insert(long_name_atom).second) + WritePhonyRule(target, long_name_atom); // Write the directory name with no target name if they match // (e.g. "//foo/bar:bar" -> "foo/bar"). if (FindLastDirComponent(label.dir()) == label.name()) { std::string medium_name = DirectoryWithNoLastSlash(label.dir()); base::TrimString(medium_name, "/", &medium_name); + const StringAtom medium_name_atom(medium_name); + // That may have generated a name the same as the short name of the // target which we already wrote. if (medium_name != label.name() && - written_rules.insert(medium_name).second) - WritePhonyRule(target, medium_name); + written_rules.insert(medium_name_atom).second) + WritePhonyRule(target, medium_name_atom); } } @@ -614,7 +620,7 @@ if (default_target) { // Use the short name when available - if (written_rules.find("default") != written_rules.end()) { + if (written_rules.find(StringAtom("default")) != written_rules.end()) { out_ << "\ndefault default" << std::endl; } else { out_ << "\ndefault ";
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc index 17f9c08..ddff182 100644 --- a/src/gn/ninja_c_binary_target_writer.cc +++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -10,7 +10,6 @@ #include <cstring> #include <set> #include <sstream> -#include <unordered_set> #include "base/strings/string_util.h" #include "gn/c_substitution_type.h" @@ -881,7 +880,7 @@ bool NinjaCBinaryTargetWriter::CheckForDuplicateObjectFiles( const std::vector<OutputFile>& files) const { - std::unordered_set<std::string> set; + std::set<std::string> set; for (const auto& file : files) { if (!set.insert(file.value()).second) { Err err(
diff --git a/src/gn/string_atom.cc b/src/gn/string_atom.cc index b4cd9f7..7842fdd 100644 --- a/src/gn/string_atom.cc +++ b/src/gn/string_atom.cc
@@ -9,7 +9,6 @@ #include <mutex> #include <set> #include <string> -#include <unordered_set> #include <vector> #include "gn/hash_table_base.h"
diff --git a/src/gn/unique_vector.h b/src/gn/unique_vector.h index ce5380b..91bf726 100644 --- a/src/gn/unique_vector.h +++ b/src/gn/unique_vector.h
@@ -8,7 +8,6 @@ #include <stddef.h> #include <algorithm> -#include <unordered_set> #include <vector> #include "hash_table_base.h"