Run Target::OnResolve() on background threads
OnResolve() is often trivial, but it can also not be, as there are a lot
of possible checks that it can do.
"gn --tracelog gen" now shows all threads being always busy.
Hyperfine Benchmark from Chromium:
Before:
Time (mean ± σ): 7.349 s ± 0.251 s [User: 66.903 s, System: 50.486 s]
Range (min … max): 6.974 s … 7.655 s 10 runs
After:
Time (mean ± σ): 6.563 s ± 0.313 s [User: 67.873 s, System: 53.023 s]
Range (min … max): 6.186 s … 7.281 s 10 runs
Bug: chromium:398002893
Change-Id: I30bd9b7541a961b1d0cef80c229b0a3f24752d3e
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/19020
Reviewed-by: David Turner <digit@google.com>
Commit-Queue: David Turner <digit@google.com>
Reviewed-by: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/analyzer_unittest.cc b/src/gn/analyzer_unittest.cc
index dfd1486..8718822 100644
--- a/src/gn/analyzer_unittest.cc
+++ b/src/gn/analyzer_unittest.cc
@@ -15,6 +15,7 @@
#include "gn/source_file.h"
#include "gn/substitution_list.h"
#include "gn/target.h"
+#include "gn/test_with_scheduler.h"
#include "gn/tool.h"
#include "gn/toolchain.h"
#include "util/test/test.h"
@@ -43,7 +44,7 @@
~MockLoader() override = default;
};
-class AnalyzerTest : public testing::Test {
+class AnalyzerTest : public TestWithScheduler {
public:
AnalyzerTest()
: loader_(new MockLoader),
diff --git a/src/gn/builder.cc b/src/gn/builder.cc
index 1f3ecd8..bf47b13 100644
--- a/src/gn/builder.cc
+++ b/src/gn/builder.cc
@@ -236,6 +236,9 @@
for (auto* bad_record : bad_records) {
depstring +=
"\n\"" + bad_record->label().GetUserVisibleName(true) + "\"";
+ if (records_resolving_on_worker_.contains(bad_record)) {
+ depstring += " (currently resolving on worker)";
+ }
}
*err = Err(Location(), "", depstring);
} else {
@@ -497,6 +500,7 @@
if (record->type() == BuilderRecord::ITEM_TARGET) {
Target* target = record->item()->AsTarget();
+ // All deps must be resolved before OnResolved can be called.
if (!ResolveDeps(&target->public_deps(), err) ||
!ResolveDeps(&target->private_deps(), err) ||
!ResolveDeps(&target->data_deps(), err) ||
@@ -505,6 +509,10 @@
!ResolveConfigs(&target->public_configs(), err) ||
!ResolvePool(target, err) || !ResolveToolchain(target, err))
return false;
+
+ // Offload Target::OnResolved to a worker thread.
+ ScheduleTargetOnResolve(record);
+ return true;
} else if (record->type() == BuilderRecord::ITEM_CONFIG) {
Config* config = record->item()->AsConfig();
if (!ResolveConfigs(&config->configs(), err))
@@ -516,11 +524,47 @@
if (!ResolvePools(toolchain, err))
return false;
}
-
- record->set_resolved(true);
-
if (!record->item()->OnResolved(err))
return false;
+
+ return CompleteItemResolution(record, err);
+}
+
+void Builder::ScheduleTargetOnResolve(BuilderRecord* record) {
+ DCHECK(g_scheduler);
+ bool already_existed = !records_resolving_on_worker_.add(record);
+ DCHECK(!already_existed);
+
+ g_scheduler->IncrementWorkCount();
+ g_scheduler->ScheduleWork([this, record]() {
+ Err err;
+ bool success = record->item()->AsTarget()->OnResolved(&err);
+ DCHECK(success == !err.has_error());
+
+ g_scheduler->task_runner()->PostTask(
+ [this, record, err]() {
+ CompleteAsyncTargetResolution(record, err);
+ });
+ });
+}
+
+void Builder::CompleteAsyncTargetResolution(
+ BuilderRecord* record, const Err& err) {
+ records_resolving_on_worker_.erase(record);
+ if (err.has_error()) {
+ g_scheduler->FailWithError(err);
+ } else {
+ Err next_err;
+ if (!CompleteItemResolution(record, &next_err)) {
+ g_scheduler->FailWithError(next_err);
+ }
+ }
+ g_scheduler->DecrementWorkCount();
+}
+
+bool Builder::CompleteItemResolution(BuilderRecord* record, Err* err) {
+ record->set_resolved(true);
+
if (record->should_generate() && resolved_and_generated_callback_)
resolved_and_generated_callback_(record);
diff --git a/src/gn/builder.h b/src/gn/builder.h
index a9c01aa..e4d996c 100644
--- a/src/gn/builder.h
+++ b/src/gn/builder.h
@@ -13,6 +13,7 @@
#include "gn/builder_record_map.h"
#include "gn/label.h"
#include "gn/label_ptr.h"
+#include "gn/pointer_set.h"
#include "gn/unique_vector.h"
class ActionValues;
@@ -121,6 +122,9 @@
// This takes a BuilderRecord with resolved dependencies, and fills in the
// target's Label*Vectors with the resolved pointers.
bool ResolveItem(BuilderRecord* record, Err* err);
+ void ScheduleTargetOnResolve(BuilderRecord* record);
+ void CompleteAsyncTargetResolution(BuilderRecord* record, const Err& err);
+ bool CompleteItemResolution(BuilderRecord* record, Err* err);
// Fills in the pointers in the given vector based on the labels. We assume
// that everything should be resolved by this point, so will return an error
@@ -144,6 +148,8 @@
ResolvedGeneratedCallback resolved_and_generated_callback_;
+ PointerSet<const BuilderRecord> records_resolving_on_worker_;
+
Builder(const Builder&) = delete;
Builder& operator=(const Builder&) = delete;
};
diff --git a/src/gn/builder_unittest.cc b/src/gn/builder_unittest.cc
index 59068a6..59b58f8 100644
--- a/src/gn/builder_unittest.cc
+++ b/src/gn/builder_unittest.cc
@@ -166,6 +166,7 @@
b->set_output_type(Target::SHARED_LIBRARY);
b->visibility().SetPublic();
builder_.ItemDefined(std::unique_ptr<Item>(b));
+ scheduler().Run();
// B depends only on the already-loaded C and toolchain so we shouldn't have
// requested anything else.
@@ -253,6 +254,7 @@
a->public_deps().push_back(LabelTargetPair(b_label));
a->set_output_type(Target::EXECUTABLE);
builder_.ItemDefined(std::unique_ptr<Item>(a));
+ scheduler().Run();
// A should have the generate bit set since it's in the default toolchain.
BuilderRecord* a_record = builder_.GetRecord(a_label);
@@ -290,6 +292,7 @@
Target* b = new Target(&settings2, b_label);
b->set_output_type(Target::EXECUTABLE);
+ b->visibility().SetPublic(); // Allow 'a' to depend on 'b'
b->gen_deps().push_back(LabelTargetPair(c_label));
builder_.ItemDefined(std::unique_ptr<Item>(b));
@@ -305,6 +308,7 @@
Target* d = new Target(&settings2, d_label);
d->set_output_type(Target::EXECUTABLE);
builder_.ItemDefined(std::unique_ptr<Item>(d));
+ scheduler().Run();
BuilderRecord* a_record = builder_.GetRecord(a_label);
BuilderRecord* b_record = builder_.GetRecord(b_label);
@@ -335,12 +339,14 @@
Target* a = new Target(&settings_, a_label);
a->gen_deps().push_back(LabelTargetPair(b_label));
a->set_output_type(Target::EXECUTABLE);
+ a->visibility().SetPublic(); // Allow 'b' to depend on 'a'
builder_.ItemDefined(std::unique_ptr<Item>(a));
Target* b = new Target(&settings2, b_label);
b->private_deps().push_back(LabelTargetPair(a_label));
b->set_output_type(Target::EXECUTABLE);
builder_.ItemDefined(std::unique_ptr<Item>(b));
+ scheduler().Run();
Err err;
EXPECT_TRUE(builder_.CheckForBadItems(&err));
diff --git a/src/gn/item.h b/src/gn/item.h
index dd1a606..4a6e193 100644
--- a/src/gn/item.h
+++ b/src/gn/item.h
@@ -69,6 +69,7 @@
// Called when this item is resolved, meaning it and all of its dependents
// have no unresolved deps. Returns true on success. Sets the error and
// returns false on failure.
+ // Can be called from any thread, so must not mutate dependencies.
virtual bool OnResolved(Err* err);
private:
