diff options
author | Nico Weber <nicolasweber@gmx.de> | 2015-04-29 21:26:54 (GMT) |
---|---|---|
committer | Nico Weber <nicolasweber@gmx.de> | 2015-04-29 21:26:54 (GMT) |
commit | 3cae29b166c27a93a26cc5f086ba7d89ea81494d (patch) | |
tree | 27320c41e36b5e52c3a136c14ad8a672676c0a5b | |
parent | e5671f83fa842f15f2319ff634bc59c3849c6241 (diff) | |
parent | bf2f4b7356c8ea918718651bae586149ab97ebee (diff) | |
download | Ninja-3cae29b166c27a93a26cc5f086ba7d89ea81494d.zip Ninja-3cae29b166c27a93a26cc5f086ba7d89ea81494d.tar.gz Ninja-3cae29b166c27a93a26cc5f086ba7d89ea81494d.tar.bz2 |
Merge pull request #962 from sgraham/pool-use-fix
Fix pool use count going unbalanced
-rw-r--r-- | src/build.cc | 14 | ||||
-rw-r--r-- | src/build.h | 5 | ||||
-rw-r--r-- | src/build_test.cc | 44 | ||||
-rw-r--r-- | src/state.h | 1 |
4 files changed, 43 insertions, 21 deletions
diff --git a/src/build.cc b/src/build.cc index ccdb37f..cdb8e0a 100644 --- a/src/build.cc +++ b/src/build.cc @@ -374,21 +374,19 @@ void Plan::ScheduleWork(Edge* edge) { } } -void Plan::ResumeDelayedJobs(Edge* edge) { - edge->pool()->EdgeFinished(*edge); - edge->pool()->RetrieveReadyEdges(&ready_); -} - void Plan::EdgeFinished(Edge* edge) { map<Edge*, bool>::iterator e = want_.find(edge); assert(e != want_.end()); - if (e->second) + bool directly_wanted = e->second; + if (directly_wanted) --wanted_edges_; want_.erase(e); edge->outputs_ready_ = true; - // See if this job frees up any delayed jobs - ResumeDelayedJobs(edge); + // See if this job frees up any delayed jobs. + if (directly_wanted) + edge->pool()->EdgeFinished(*edge); + edge->pool()->RetrieveReadyEdges(&ready_); // Check off any nodes we were waiting for with this edge. for (vector<Node*>::iterator o = edge->outputs_.begin(); diff --git a/src/build.h b/src/build.h index 4b48c5f..8106faa 100644 --- a/src/build.h +++ b/src/build.h @@ -78,11 +78,6 @@ private: /// currently-full pool. void ScheduleWork(Edge* edge); - /// Allows jobs blocking on |edge| to potentially resume. - /// For example, if |edge| is a member of a pool, calling this may schedule - /// previously pending jobs in that pool. - void ResumeDelayedJobs(Edge* edge); - /// Keep track of which edges we want to build in this plan. If this map does /// not contain an entry for an edge, we do not want to build the entry or its /// dependents. If an entry maps to false, we do not want to build it, but we diff --git a/src/build_test.cc b/src/build_test.cc index a313693..52a17c9 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -495,8 +495,8 @@ struct BuildTest : public StateTestWithBuiltinRules, public BuildLogUser { /// State of command_runner_ and logs contents (if specified) ARE MODIFIED. /// Handy to check for NOOP builds, and higher-level rebuild tests. void RebuildTarget(const string& target, const char* manifest, - const char* log_path = NULL, - const char* deps_path = NULL); + const char* log_path = NULL, const char* deps_path = NULL, + State* state = NULL); // Mark a path dirty. void Dirty(const string& path); @@ -516,10 +516,13 @@ struct BuildTest : public StateTestWithBuiltinRules, public BuildLogUser { }; void BuildTest::RebuildTarget(const string& target, const char* manifest, - const char* log_path, const char* deps_path) { - State state; - ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); - AssertParse(&state, manifest); + const char* log_path, const char* deps_path, + State* state) { + State local_state, *pstate = &local_state; + if (state) + pstate = state; + ASSERT_NO_FATAL_FAILURE(AddCatRule(pstate)); + AssertParse(pstate, manifest); string err; BuildLog build_log, *pbuild_log = NULL; @@ -532,13 +535,13 @@ void BuildTest::RebuildTarget(const string& target, const char* manifest, DepsLog deps_log, *pdeps_log = NULL; if (deps_path) { - ASSERT_TRUE(deps_log.Load(deps_path, &state, &err)); + ASSERT_TRUE(deps_log.Load(deps_path, pstate, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_path, &err)); ASSERT_EQ("", err); pdeps_log = &deps_log; } - Builder builder(&state, config_, pbuild_log, pdeps_log, &fs_); + Builder builder(pstate, config_, pbuild_log, pdeps_log, &fs_); EXPECT_TRUE(builder.AddTarget(target, &err)); command_runner_.commands_ran_.clear(); @@ -1092,6 +1095,31 @@ TEST_F(BuildTest, SwallowFailuresLimit) { ASSERT_EQ("cannot make progress due to previous errors", err); } +TEST_F(BuildTest, PoolEdgesReadyButNotWanted) { + fs_.Create("x", ""); + + const char* manifest = + "pool some_pool\n" + " depth = 4\n" + "rule touch\n" + " command = touch $out\n" + " pool = some_pool\n" + "rule cc\n" + " command = touch grit\n" + "\n" + "build B.d.stamp: cc | x\n" + "build C.stamp: touch B.d.stamp\n" + "build final.stamp: touch || C.stamp\n"; + + RebuildTarget("final.stamp", manifest); + + fs_.RemoveFile("B.d.stamp"); + + State save_state; + RebuildTarget("final.stamp", manifest, NULL, NULL, &save_state); + EXPECT_GE(save_state.LookupPool("some_pool")->current_use(), 0); +} + struct BuildWithLogTest : public BuildTest { BuildWithLogTest() { builder_.SetBuildLog(&build_log_); diff --git a/src/state.h b/src/state.h index 5000138..d7987ba 100644 --- a/src/state.h +++ b/src/state.h @@ -44,6 +44,7 @@ struct Pool { bool is_valid() const { return depth_ >= 0; } int depth() const { return depth_; } const string& name() const { return name_; } + int current_use() const { return current_use_; } /// true if the Pool might delay this edge bool ShouldDelayEdge() const { return depth_ != 0; } |