Skip to content

Commit

Permalink
Replace OrderedSet<> instances with UniqueVector<>.
Browse files Browse the repository at this point in the history
The OrderedSet<> template has a misleading name, because its
semantics are not those of an ordered set of items, but match
completely those of UniqueVector<>. The latter is also slightly
faster and uses less memory.

This CL replaces all instances of OrderedSet<> with UniqueVector<>
ones, and removes the template definition from the source tree.

Profiling shows no noticeable difference in performance or memory usage
with the Chrome and Fuchsia "gn gen" operations.

Change-Id: Ie9b8ecfc0724d07129f69c68db5e1205b49f3970
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/11825
Commit-Queue: David Turner <[email protected]>
Reviewed-by: Sylvain Defresne <[email protected]>
  • Loading branch information
digit-android authored and Commit Bot committed Jun 18, 2021
1 parent 7d80399 commit 170c2db
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 89 deletions.
4 changes: 2 additions & 2 deletions src/gn/desc_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ class TargetDescBuilder : public BaseDescBuilder {
// Libs can be part of any target and get recursively pushed up the chain,
// so display them regardless of target type.
if (what(variables::kLibs)) {
const OrderedSet<LibFile>& all_libs = target_->all_libs();
const UniqueVector<LibFile>& all_libs = target_->all_libs();
if (!all_libs.empty()) {
auto libs = std::make_unique<base::ListValue>();
for (size_t i = 0; i < all_libs.size(); i++)
Expand All @@ -591,7 +591,7 @@ class TargetDescBuilder : public BaseDescBuilder {
}

if (what(variables::kLibDirs)) {
const OrderedSet<SourceDir>& all_lib_dirs = target_->all_lib_dirs();
const UniqueVector<SourceDir>& all_lib_dirs = target_->all_lib_dirs();
if (!all_lib_dirs.empty()) {
auto lib_dirs = std::make_unique<base::ListValue>();
for (size_t i = 0; i < all_lib_dirs.size(); i++)
Expand Down
4 changes: 2 additions & 2 deletions src/gn/ninja_binary_target_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ void NinjaBinaryTargetWriter::WriteLinkerFlags(

// Followed by library search paths that have been recursively pushed
// through the dependency tree.
const OrderedSet<SourceDir>& all_lib_dirs = target_->all_lib_dirs();
const UniqueVector<SourceDir>& all_lib_dirs = target_->all_lib_dirs();
if (!all_lib_dirs.empty()) {
// Since we're passing these on the command line to the linker and not
// to Ninja, we need to do shell escaping.
Expand Down Expand Up @@ -345,7 +345,7 @@ void NinjaBinaryTargetWriter::WriteLibs(std::ostream& out, const Tool* tool) {
// Libraries that have been recursively pushed through the dependency tree.
EscapeOptions lib_escape_opts;
lib_escape_opts.mode = ESCAPE_NINJA_COMMAND;
const OrderedSet<LibFile> all_libs = target_->all_libs();
const UniqueVector<LibFile>& all_libs = target_->all_libs();
for (size_t i = 0; i < all_libs.size(); i++) {
const LibFile& lib_file = all_libs[i];
const std::string& lib_value = lib_file.value();
Expand Down
2 changes: 1 addition & 1 deletion src/gn/ninja_c_binary_target_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ void NinjaCBinaryTargetWriter::WriteLinkerStuff(
}

// Libraries specified by paths.
const OrderedSet<LibFile>& libs = target_->all_libs();
const UniqueVector<LibFile>& libs = target_->all_libs();
for (size_t i = 0; i < libs.size(); i++) {
if (libs[i].is_source_file()) {
implicit_deps.push_back(
Expand Down
63 changes: 0 additions & 63 deletions src/gn/ordered_set.h

This file was deleted.

20 changes: 10 additions & 10 deletions src/gn/target.cc
Original file line number Diff line number Diff line change
Expand Up @@ -374,13 +374,13 @@ bool Target::OnResolved(Err* err) {
// order (local ones first, then the dependency's).
for (ConfigValuesIterator iter(this); !iter.done(); iter.Next()) {
const ConfigValues& cur = iter.cur();
all_lib_dirs_.append(cur.lib_dirs().begin(), cur.lib_dirs().end());
all_libs_.append(cur.libs().begin(), cur.libs().end());
all_lib_dirs_.Append(cur.lib_dirs().begin(), cur.lib_dirs().end());
all_libs_.Append(cur.libs().begin(), cur.libs().end());

all_framework_dirs_.append(cur.framework_dirs().begin(),
all_framework_dirs_.Append(cur.framework_dirs().begin(),
cur.framework_dirs().end());
all_frameworks_.append(cur.frameworks().begin(), cur.frameworks().end());
all_weak_frameworks_.append(cur.weak_frameworks().begin(),
all_frameworks_.Append(cur.frameworks().begin(), cur.frameworks().end());
all_weak_frameworks_.Append(cur.weak_frameworks().begin(),
cur.weak_frameworks().end());
}

Expand Down Expand Up @@ -722,12 +722,12 @@ void Target::PullDependentTargetLibsFrom(const Target* dep, bool is_public) {

// Library settings are always inherited across static library boundaries.
if (!dep->IsFinal() || dep->output_type() == STATIC_LIBRARY) {
all_lib_dirs_.append(dep->all_lib_dirs());
all_libs_.append(dep->all_libs());
all_lib_dirs_.Append(dep->all_lib_dirs());
all_libs_.Append(dep->all_libs());

all_framework_dirs_.append(dep->all_framework_dirs());
all_frameworks_.append(dep->all_frameworks());
all_weak_frameworks_.append(dep->all_weak_frameworks());
all_framework_dirs_.Append(dep->all_framework_dirs());
all_frameworks_.Append(dep->all_frameworks());
all_weak_frameworks_.Append(dep->all_weak_frameworks());
}
}

Expand Down
21 changes: 10 additions & 11 deletions src/gn/target.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include "gn/label_ptr.h"
#include "gn/lib_file.h"
#include "gn/metadata.h"
#include "gn/ordered_set.h"
#include "gn/output_file.h"
#include "gn/rust_values.h"
#include "gn/source_file.h"
Expand Down Expand Up @@ -286,16 +285,16 @@ class Target : public Item {
RustValues& rust_values() { return rust_values_; }
const RustValues& rust_values() const { return rust_values_; }

const OrderedSet<SourceDir>& all_lib_dirs() const { return all_lib_dirs_; }
const OrderedSet<LibFile>& all_libs() const { return all_libs_; }
const UniqueVector<SourceDir>& all_lib_dirs() const { return all_lib_dirs_; }
const UniqueVector<LibFile>& all_libs() const { return all_libs_; }

const OrderedSet<SourceDir>& all_framework_dirs() const {
const UniqueVector<SourceDir>& all_framework_dirs() const {
return all_framework_dirs_;
}
const OrderedSet<std::string>& all_frameworks() const {
const UniqueVector<std::string>& all_frameworks() const {
return all_frameworks_;
}
const OrderedSet<std::string>& all_weak_frameworks() const {
const UniqueVector<std::string>& all_weak_frameworks() const {
return all_weak_frameworks_;
}

Expand Down Expand Up @@ -460,14 +459,14 @@ class Target : public Item {

// These libs and dirs are inherited from statically linked deps and all
// configs applying to this target.
OrderedSet<SourceDir> all_lib_dirs_;
OrderedSet<LibFile> all_libs_;
UniqueVector<SourceDir> all_lib_dirs_;
UniqueVector<LibFile> all_libs_;

// These frameworks and dirs are inherited from statically linked deps and
// all configs applying to this target.
OrderedSet<SourceDir> all_framework_dirs_;
OrderedSet<std::string> all_frameworks_;
OrderedSet<std::string> all_weak_frameworks_;
UniqueVector<SourceDir> all_framework_dirs_;
UniqueVector<std::string> all_frameworks_;
UniqueVector<std::string> all_weak_frameworks_;

// All hard deps from this target and all dependencies. Filled in when this
// target is marked resolved. This will not include the current target.
Expand Down
4 changes: 4 additions & 0 deletions src/gn/unique_vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ class UniqueVector {
push_back(*i);
}

void Append(const UniqueVector& other) {
Append(other.begin(), other.end());
}

// Returns the index of the item matching the given value in the list, or
// (size_t)(-1) if it's not found.
size_t IndexOf(const T& t) const {
Expand Down

0 comments on commit 170c2db

Please sign in to comment.