From ecc26d8cd608de282c675c25a0a6aed1318dc360 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sat, 8 Jan 2011 11:17:50 -0800 Subject: remove bottom-up dirtying --- src/build_test.cc | 73 +++++++++++++++++++++++++++++++------------------------ src/graph.cc | 32 ------------------------ src/graph.h | 5 +--- src/ninja_test.cc | 14 +++++++---- src/test.h | 3 +++ 5 files changed, 54 insertions(+), 73 deletions(-) diff --git a/src/build_test.cc b/src/build_test.cc index a430801..01c1b4a 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -12,7 +12,8 @@ TEST_F(PlanTest, Basic) { AssertParse(&state_, "build out: cat mid\n" "build mid: cat in\n"); - GetNode("in")->MarkDependentsDirty(); + GetNode("mid")->dirty_ = true; + GetNode("out")->dirty_ = true; string err; EXPECT_TRUE(plan_.AddTarget(GetNode("out"), &err)); ASSERT_EQ("", err); @@ -46,7 +47,10 @@ TEST_F(PlanTest, DoubleOutputDirect) { AssertParse(&state_, "build out: cat mid1 mid2\n" "build mid1 mid2: cat in\n"); - GetNode("in")->MarkDependentsDirty(); + GetNode("mid1")->dirty_ = true; + GetNode("mid2")->dirty_ = true; + GetNode("out")->dirty_ = true; + string err; EXPECT_TRUE(plan_.AddTarget(GetNode("out"), &err)); ASSERT_EQ("", err); @@ -75,7 +79,11 @@ TEST_F(PlanTest, DoubleOutputIndirect) { "build b1: cat a1\n" "build b2: cat a2\n" "build a1 a2: cat in\n"); - GetNode("in")->MarkDependentsDirty(); + GetNode("a1")->dirty_ = true; + GetNode("a2")->dirty_ = true; + GetNode("b1")->dirty_ = true; + GetNode("b2")->dirty_ = true; + GetNode("out")->dirty_ = true; string err; EXPECT_TRUE(plan_.AddTarget(GetNode("out"), &err)); ASSERT_EQ("", err); @@ -114,7 +122,11 @@ TEST_F(PlanTest, DoubleDependent) { "build a1: cat mid\n" "build a2: cat mid\n" "build mid: cat in\n"); - GetNode("in")->MarkDependentsDirty(); + GetNode("mid")->dirty_ = true; + GetNode("a1")->dirty_ = true; + GetNode("a2")->dirty_ = true; + GetNode("out")->dirty_ = true; + string err; EXPECT_TRUE(plan_.AddTarget(GetNode("out"), &err)); ASSERT_EQ("", err); @@ -151,7 +163,8 @@ TEST_F(PlanTest, DependencyCycle) { "build mid: cat in\n" "build in: cat pre\n" "build pre: cat out\n"); - GetNode("pre")->MarkDependentsDirty(); + ResetDirty(); + string err; EXPECT_FALSE(plan_.AddTarget(GetNode("out"), &err)); ASSERT_EQ("dependency cycle: out -> mid -> in -> pre -> out", err); @@ -172,8 +185,6 @@ struct BuildTest : public StateTestWithBuiltinRules, // Mark a path dirty. void Dirty(const string& path); - // Mark dependents of a path dirty. - void Touch(const string& path); // CommandRunner impl virtual bool CanRunMore(); @@ -215,7 +226,7 @@ struct BuildTest : public StateTestWithBuiltinRules, void BuildTest::Dirty(const string& path) { Node* node = GetNode(path); - node->MarkDirty(); + node->dirty_ = true; // If it's an input file, mark that we've already stat()ed it and // it's missing. @@ -223,12 +234,6 @@ void BuildTest::Dirty(const string& path) { node->file_->mtime_ = 0; } -void BuildTest::Touch(const string& path) { - Node* node = GetNode(path); - assert(node); - node->MarkDependentsDirty(); -} - bool BuildTest::CanRunMore() { // Only run one at a time. return last_command_ == NULL; @@ -240,7 +245,8 @@ bool BuildTest::StartCommand(Edge* edge) { if (edge->rule_->name_ == "cat" || edge->rule_->name_ == "cc") { for (vector::iterator out = edge->outputs_.begin(); out != edge->outputs_.end(); ++out) { - (*out)->file_->Touch(now_); + (*out)->file_->mtime_ = now_; + (*out)->dirty_ = false; } last_command_ = edge; return true; @@ -299,9 +305,8 @@ TEST_F(BuildTest, OneStep2) { } TEST_F(BuildTest, TwoStep) { - // Modifying in1 requires rebuilding both intermediate files - // and the final file. - Touch("in1"); + ResetDirty(); + string err; EXPECT_TRUE(builder_.AddTarget("cat12", &err)); ASSERT_EQ("", err); @@ -314,10 +319,12 @@ TEST_F(BuildTest, TwoStep) { // Modifying in2 requires rebuilding one intermediate file // and the final file. - Touch("in2"); - ASSERT_TRUE(builder_.AddTarget("cat12", &err)); + GetNode("cat2")->dirty_ = true; + GetNode("cat12")->dirty_ = true; + EXPECT_TRUE(builder_.AddTarget("cat12", &err)); + ASSERT_EQ("", err); EXPECT_TRUE(builder_.Build(&err)); - EXPECT_EQ("", err); + ASSERT_EQ("", err); ASSERT_EQ(5, commands_ran_.size()); EXPECT_EQ("cat in1 in2 > cat2", commands_ran_[3]); EXPECT_EQ("cat cat1 cat2 > cat12", commands_ran_[4]); @@ -330,7 +337,8 @@ TEST_F(BuildTest, Chain) { "build c4: cat c3\n" "build c5: cat c4\n")); - Touch("c1"); // Will recursively dirty all the way to c5. + ResetDirty(); + string err; EXPECT_TRUE(builder_.AddTarget("c5", &err)); ASSERT_EQ("", err); @@ -345,7 +353,8 @@ TEST_F(BuildTest, Chain) { EXPECT_TRUE(builder_.Build(&err)); ASSERT_EQ(0, commands_ran_.size()); - Touch("c3"); // Will recursively dirty through to c5. + GetNode("c4")->dirty_ = true; + GetNode("c5")->dirty_ = true; err.clear(); commands_ran_.clear(); EXPECT_TRUE(builder_.AddTarget("c5", &err)); @@ -375,7 +384,7 @@ TEST_F(BuildTest, MakeDirs) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "build subdir/dir2/file: cat in1\n")); - Touch("in1"); + ResetDirty(); EXPECT_TRUE(builder_.AddTarget("subdir/dir2/file", &err)); EXPECT_EQ("", err); now_ = 0; // Make all stat()s return file not found. @@ -391,7 +400,7 @@ TEST_F(BuildTest, DepFileMissing) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule cc\n command = cc $in\n depfile = $out.d\n" "build foo.o: cc foo.c\n")); - Touch("foo.c"); + ResetDirty(); EXPECT_TRUE(builder_.AddTarget("foo.o", &err)); ASSERT_EQ("", err); ASSERT_EQ(1, files_read_.size()); @@ -404,8 +413,8 @@ TEST_F(BuildTest, DepFileOK) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule cc\n command = cc $in\n depfile = $out.d\n" "build foo.o: cc foo.c\n")); - Touch("foo.c"); - Dirty("bar.h"); // Mark bar.h as missing. + ResetDirty(); + GetNode("bar.h")->dirty_ = true; // Mark bar.h as missing. file_contents_["foo.o.d"] = "foo.o: blah.h bar.h\n"; EXPECT_TRUE(builder_.AddTarget("foo.o", &err)); ASSERT_EQ("", err); @@ -426,7 +435,7 @@ TEST_F(BuildTest, DepFileParseError) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule cc\n command = cc $in\n depfile = $out.d\n" "build foo.o: cc foo.c\n")); - Touch("foo.c"); + ResetDirty(); file_contents_["foo.o.d"] = "foo.o blah.h bar.h\n"; EXPECT_FALSE(builder_.AddTarget("foo.o", &err)); EXPECT_EQ("line 1, col 7: expected ':', got 'blah.h'", err); @@ -437,7 +446,7 @@ TEST_F(BuildTest, OrderOnlyDeps) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule cc\n command = cc $in\n depfile = $out.d\n" "build foo.o: cc foo.c | otherfile\n")); - Touch("foo.c"); + ResetDirty(); file_contents_["foo.o.d"] = "foo.o: blah.h bar.h\n"; EXPECT_TRUE(builder_.AddTarget("foo.o", &err)); @@ -463,7 +472,7 @@ TEST_F(BuildTest, OrderOnlyDeps) { // implicit dep dirty, expect a rebuild. commands_ran_.clear(); - Touch("blah.h"); + GetNode("blah.h")->dirty_ = true; EXPECT_TRUE(builder_.AddTarget("foo.o", &err)); EXPECT_TRUE(builder_.Build(&err)); ASSERT_EQ("", err); @@ -471,7 +480,7 @@ TEST_F(BuildTest, OrderOnlyDeps) { // order only dep dirty, no rebuild. commands_ran_.clear(); - Touch("otherfile"); + GetNode("otherfile")->dirty_ = true; // We should fail to even add the depenency on foo.o, because // there's nothing to do. EXPECT_FALSE(builder_.AddTarget("foo.o", &err)); @@ -482,7 +491,7 @@ TEST_F(BuildTest, Phony) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "build out: cat bar.cc\n" "build all: phony out\n")); - Touch("bar.cc"); + ResetDirty(); EXPECT_TRUE(builder_.AddTarget("all", &err)); diff --git a/src/graph.cc b/src/graph.cc index 22283a5..45fcea8 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -6,30 +6,11 @@ #include "ninja.h" #include "parsers.h" -void FileStat::Touch(int mtime) { - mtime_ = mtime; - if (node_) - node_->MarkDirty(); -} - bool FileStat::Stat(DiskInterface* disk_interface) { mtime_ = disk_interface->Stat(path_); return mtime_ > 0; } -void Node::MarkDirty() { - if (dirty_) - return; // We already know. - - dirty_ = true; - MarkDependentsDirty(); -} - -void Node::MarkDependentsDirty() { - for (vector::iterator i = out_edges_.begin(); i != out_edges_.end(); ++i) - (*i)->MarkDirty(this); -} - bool Edge::RecomputeDirty(State* state, DiskInterface* disk_interface, string* err) { bool dirty = false; @@ -96,19 +77,6 @@ bool Edge::RecomputeDirty(State* state, DiskInterface* disk_interface, return true; } -void Edge::MarkDirty(Node* node) { - if (rule_ == &State::kPhonyRule) - return; - - vector::iterator i = find(inputs_.begin(), inputs_.end(), node); - if (i == inputs_.end()) - return; - if (i - inputs_.begin() >= ((int)inputs_.size()) - order_only_deps_) - return; // Order-only deps don't cause us to become dirty. - for (i = outputs_.begin(); i != outputs_.end(); ++i) - (*i)->MarkDirty(); -} - struct EdgeEnv : public Env { EdgeEnv(Edge* edge) : edge_(edge) {} virtual string LookupVariable(const string& var) { diff --git a/src/graph.h b/src/graph.h index 3f46db4..2ebf2e4 100644 --- a/src/graph.h +++ b/src/graph.h @@ -12,7 +12,7 @@ struct DiskInterface; struct Node; struct FileStat { FileStat(const string& path) : path_(path), mtime_(-1), node_(NULL) {} - void Touch(int mtime); + // Return true if the file exists (mtime_ got a value). bool Stat(DiskInterface* disk_interface); @@ -46,8 +46,6 @@ struct Node { Node(FileStat* file) : file_(file), dirty_(false), in_edge_(NULL) {} bool dirty() const { return dirty_; } - void MarkDirty(); - void MarkDependentsDirty(); FileStat* file_; bool dirty_; @@ -71,7 +69,6 @@ struct State; struct Edge { Edge() : rule_(NULL), env_(NULL), implicit_deps_(0), order_only_deps_(0) {} - void MarkDirty(Node* node); bool RecomputeDirty(State* state, DiskInterface* disk_interface, string* err); string EvaluateCommand(); // XXX move to env, take env ptr string GetDescription(); diff --git a/src/ninja_test.cc b/src/ninja_test.cc index 0c992e9..b0c19dd 100644 --- a/src/ninja_test.cc +++ b/src/ninja_test.cc @@ -23,6 +23,15 @@ Node* StateTestWithBuiltinRules::GetNode(const string& path) { return state_.GetNode(path); } +void StateTestWithBuiltinRules::ResetDirty() { + for (StatCache::Paths::iterator i = state_.stat_cache_.paths_.begin(); + i != state_.stat_cache_.paths_.end(); ++i) { + Node* node = i->second->node_; + // Mark node dirty if we have a way to rebuild it. + node->dirty_ = node->in_edge_ != NULL; + } +} + TEST(State, Basic) { State state; Rule* rule = new Rule("cat"); @@ -40,11 +49,6 @@ TEST(State, Basic) { EXPECT_FALSE(state.GetNode("in1")->dirty()); EXPECT_FALSE(state.GetNode("in2")->dirty()); EXPECT_FALSE(state.GetNode("out")->dirty()); - - state.stat_cache()->GetFile("in1")->Touch(1); - EXPECT_TRUE(state.GetNode("in1")->dirty()); - EXPECT_FALSE(state.GetNode("in2")->dirty()); - EXPECT_TRUE(state.GetNode("out")->dirty()); } struct TestEnv : public Env { diff --git a/src/test.h b/src/test.h index 405f039..25d4149 100644 --- a/src/test.h +++ b/src/test.h @@ -7,6 +7,9 @@ struct StateTestWithBuiltinRules : public testing::Test { StateTestWithBuiltinRules(); Node* GetNode(const string& path); + // Mark every non-leaf node dirty. + void ResetDirty(); + State state_; }; -- cgit v0.12