Track and remove dynamic implicit dependencies

Due to `deps_loaded_` being reset to false unconditionally
in Edge::Reset(), every incremental build reloaded dependencies
from the deps log or dep files, and added them back to the
build graph. This resulted in massive and un-necessary
duplications. For example, for a large Fuchsia build plan,
every no-op incremental build would add 200 to 300 MiB of
memory to the RSS of the persistent server process!

This patch fixes the situation by doing the following:

- Do not reset `deps_loaded_` in Edge::Reset(), i.e. the
  dependency information recorded in each Edge stays
  there and will not be reloaded by default.

- Add `static_implicit_deps_` and `static_implicit_outs_`
  fields to track the number of implicit dependencies
  that were added to a given Edge, through the deps log,
  depfile or dyndep loading. And implement a new
  Edge::RemoveDynamicImplicitDeps() method to remove
  them when needed.

+ Add a unit-test to verify that this mechanism works
  properly.

This patch has three benefits:

- It fixes the correctness issue from bug 135792
  (verified with a new unit-test).

- It fixes the memory leak from bug 135951
  (verified manually, the RSS is stable after two
  no-op builds, and only increases very
  slightly between the first build and the second
  one).

- It speeds up dependency scanning for the next
  incremental build significantly (2s -> 1.5s)
  because it no longer needs to reload *all*
  recorded dependencies from the log on each
  edge it visits.

+ Add new dyndep-related unit-test to check the behavior of
  the new code.

Fuchsia-Topic: persistent-mode
Original-Change-Id: I09b5035fd7869a6031898707c67bbd0e3cf0d452
Original-Change-Id: Id6142b6834e78ab27eb7c94954849ce1d0872350
Change-Id: Iadcdbb64e28815bcec71edc530c829fcfb3ff20f
Reviewed-on: https://fuchsia-review.googlesource.com/c/third_party/github.com/ninja-build/ninja/+/978773
Reviewed-by: David Fang <fangism@google.com>
Commit-Queue: David Turner <digit@google.com>
diff --git a/src/build.cc b/src/build.cc
index b1012a7..1e8041f 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -863,6 +863,10 @@
       }
     }
   }
+
+  // Ensure that the next dependency scan that touches this edge removes
+  // any recorded deps in it since they are now stale.
+  edge->deps_loaded_ = false;
   return true;
 }
 
diff --git a/src/dyndep.cc b/src/dyndep.cc
index 982e939..7e49a75 100644
--- a/src/dyndep.cc
+++ b/src/dyndep.cc
@@ -87,35 +87,15 @@
   if (dyndeps->restat_)
     edge->SetRestat();
 
-  // Add the dyndep-discovered outputs to the edge.
-  edge->outputs_.insert(edge->outputs_.end(),
-                        dyndeps->implicit_outputs_.begin(),
-                        dyndeps->implicit_outputs_.end());
-  edge->implicit_outs_ += dyndeps->implicit_outputs_.size();
-
-  // Add this edge as incoming to each new output.
-  for (std::vector<Node*>::const_iterator i =
-           dyndeps->implicit_outputs_.begin();
-       i != dyndeps->implicit_outputs_.end(); ++i) {
-    if ((*i)->in_edge()) {
-      // This node already has an edge producing it.
-      *err = "multiple rules generate " + (*i)->path();
-      return false;
-    }
-    (*i)->set_in_edge(edge);
+  if (!edge->UpdateDynamicImplicitOutputs(dyndeps->implicit_outputs_.size(),
+                                          dyndeps->implicit_outputs_.data(),
+                                          err)) {
+    return false;
   }
 
   // Add the dyndep-discovered inputs to the edge.
-  edge->inputs_.insert(edge->inputs_.end() - edge->order_only_deps_,
-                       dyndeps->implicit_inputs_.begin(),
-                       dyndeps->implicit_inputs_.end());
-  edge->implicit_deps_ += dyndeps->implicit_inputs_.size();
-
-  // Add this edge as outgoing from each new input.
-  for (std::vector<Node*>::const_iterator i =
-           dyndeps->implicit_inputs_.begin();
-       i != dyndeps->implicit_inputs_.end(); ++i)
-    (*i)->AddOutEdge(edge);
+  edge->UpdateDynamicImplicitDeps(dyndeps->implicit_inputs_.size(),
+                                  dyndeps->implicit_inputs_.data());
 
   return true;
 }
diff --git a/src/graph.cc b/src/graph.cc
index 553c39a..36fb9b1 100644
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -14,10 +14,13 @@
 
 #include "graph.h"
 
-#include <algorithm>
-#include <deque>
 #include <assert.h>
 #include <stdio.h>
+#include <string.h>
+
+#include <algorithm>
+#include <deque>
+#include <set>
 
 #include "build_log.h"
 #include "debug_flags.h"
@@ -594,6 +597,149 @@
   printf("] 0x%p\n", this);
 }
 
+void Edge::UpdateDynamicImplicitDeps(size_t new_count, Node* const* new_deps) {
+  size_t cur_count =
+      static_cast<size_t>(implicit_deps_ - static_implicit_deps_);
+  auto cur_deps = inputs_.end() - order_only_deps_ - cur_count;
+
+  // Most of the time, the content of depfiles will not change between
+  // command invocations, and thus |new_deps| will already be recorded
+  // in this edge. Detect when this is the case and exit immediately.
+  if (cur_count == new_count &&
+      !memcmp(&(*cur_deps), new_deps, cur_count * sizeof(Node*))) {
+    return;
+  }
+
+  // If there are no recorded deps, insert the new ones directly.
+  // This happens the first time this function is called for a given
+  // Edge instance.
+  if (cur_count == 0) {
+    auto it = inputs_.insert(cur_deps, new_count, nullptr);
+    implicit_deps_ += new_count;
+    for (size_t n = 0; n < new_count; ++n) {
+      Node* node = new_deps[n];
+      *it++ = node;
+      node->AddOutEdge(this);
+    }
+    return;
+  }
+
+  // This is the general case where the content of a depfile changed since
+  // the last build invocation. This is rare but can happen, for example
+  // when depfile inputs have content-hash based names that change with
+  // the content of another input.
+  //
+  // Because benchmarking shows that modifying the build graph is slow for
+  // very large build plans, try to minimize changes by detecting the set of
+  // nodes to remove from the current edge, then the set of new ones to add.
+  std::set<Node*> cur_set(cur_deps, cur_deps + cur_count);
+  std::set<Node*> new_set(new_deps, new_deps + new_count);
+
+  // Remove all nodes that are no longer in |new_deps|.
+  {
+    auto it = cur_deps;
+    for (size_t n = 0; n < cur_count;) {
+      Node* node = *it;
+      if (!new_set.count(node)) {
+        node->RemoveOutEdge(this);
+        it = inputs_.erase(it);
+        implicit_deps_ -= 1;
+        cur_count -= 1;
+      } else {
+        ++it;
+        ++n;
+      }
+    }
+    cur_deps = inputs_.end() - order_only_deps_ - cur_count;
+  }
+
+  // Add any edge in |new_deps| that is not in the current set.
+  {
+    auto it = cur_deps + cur_count;
+    for (size_t n = 0; n < new_count; ++n) {
+      Node* node = new_deps[n];
+      if (cur_set.count(node))
+        continue;
+      implicit_deps_ += 1;
+      it = inputs_.insert(it, node) + 1;
+      node->AddOutEdge(this);
+    }
+  }
+}
+
+bool Edge::UpdateDynamicImplicitOutputs(size_t new_count, Node* const* new_outs,
+                                        std::string* err) {
+  size_t cur_count =
+      static_cast<size_t>(implicit_outs_ - static_implicit_outs_);
+  auto cur_outs = outputs_.end() - cur_count;
+
+  // Most of the time, the content of dyndep files will not change.
+  if (cur_count == new_count &&
+      !memcmp(&(*cur_outs), new_outs, cur_count * sizeof(Node*))) {
+    return true;
+  }
+
+  // If there are no recorded outputs, insert the new ones directly.
+  if (cur_count == 0) {
+    auto it = outputs_.insert(cur_outs, new_count, nullptr);
+    implicit_outs_ += new_count;
+    for (size_t n = 0; n < new_count; ++n) {
+      Node* node = new_outs[n];
+      if (node->in_edge()) {
+        // This node already has an edge producing it.
+        *err = "multiple rules generate " + node->path();
+        return false;
+      }
+      *it++ = node;
+      node->set_in_edge(this);
+    }
+    return true;
+  }
+
+  // This is the general case where the content of a dyndep file changed since
+  // the last build invocation.
+  std::set<Node*> cur_set(cur_outs, cur_outs + cur_count);
+  std::set<Node*> new_set(new_outs, new_outs + new_count);
+
+  // Remove all nodes that are no longer in |new_outs|.
+  {
+    auto it = cur_outs;
+    for (size_t n = 0; n < cur_count;) {
+      Node* node = *it;
+      assert(node->in_edge() == this);
+      if (!new_set.count(node)) {
+        node->set_in_edge(nullptr);
+        it = outputs_.erase(it);
+        implicit_outs_ -= 1;
+        cur_count -= 1;
+      } else {
+        ++it;
+        ++n;
+      }
+    }
+    cur_outs = outputs_.end() - cur_count;
+  }
+
+  // Add any edge in |new_outs| that is not in the current set.
+  {
+    auto it = cur_outs + cur_count;
+    for (size_t n = 0; n < new_count; ++n) {
+      Node* node = new_outs[n];
+      if (cur_set.count(node))
+        continue;
+      if (node->in_edge()) {
+        // This node already has an edge producing it.
+        *err = "multiple rules generate " + node->path();
+        return false;
+      }
+      node->set_in_edge(this);
+      implicit_outs_ += 1;
+      it = outputs_.insert(it, node) + 1;
+    }
+  }
+  return true;
+}
+
 bool Edge::is_phony() const {
   return rule_->is_phony();
 }
@@ -625,6 +771,14 @@
   return result;
 }
 
+void Node::RemoveOutEdge(Edge* edge) {
+  auto it = std::find(out_edges_.begin(), out_edges_.end(), edge);
+  assert(it != out_edges_.end());
+  // order is not important here, avoid O(n) deletion cost.
+  *it = out_edges_.back();
+  out_edges_.pop_back();
+}
+
 void Node::Dump(const char* prefix) const {
   printf("%s <%s 0x%p> mtime: %" PRId64 "%s, (:%s), ",
          prefix, path().c_str(), this,
@@ -738,20 +892,14 @@
 
 bool ImplicitDepLoader::ProcessDepfileDeps(
     Edge* edge, std::vector<StringPiece>* depfile_ins, std::string* err) {
-  // Preallocate space in edge->inputs_ to be filled in below.
-  vector<Node*>::iterator implicit_dep =
-      PreallocateSpace(edge, depfile_ins->size());
-
-  // Add all its in-edges.
-  for (std::vector<StringPiece>::iterator i = depfile_ins->begin();
-       i != depfile_ins->end(); ++i, ++implicit_dep) {
+  std::vector<Node*> new_deps;
+  new_deps.reserve(depfile_ins->size());
+  for (StringPiece dep : *depfile_ins) {
     uint64_t slash_bits;
-    CanonicalizePath(const_cast<char*>(i->str_), &i->len_, &slash_bits);
-    Node* node = state_->GetNode(*i, slash_bits);
-    *implicit_dep = node;
-    node->AddOutEdge(edge);
+    CanonicalizePath(const_cast<char*>(dep.str_), &dep.len_, &slash_bits);
+    new_deps.push_back(state_->GetNode(dep, slash_bits));
   }
-
+  edge->UpdateDynamicImplicitDeps(new_deps.size(), new_deps.data());
   return true;
 }
 
@@ -771,13 +919,8 @@
     return false;
   }
 
-  vector<Node*>::iterator implicit_dep =
-      PreallocateSpace(edge, deps->node_count);
-  for (int i = 0; i < deps->node_count; ++i, ++implicit_dep) {
-    Node* node = deps->nodes[i];
-    *implicit_dep = node;
-    node->AddOutEdge(edge);
-  }
+  edge->UpdateDynamicImplicitDeps(static_cast<size_t>(deps->node_count),
+                                  deps->nodes);
   return true;
 }
 
diff --git a/src/graph.h b/src/graph.h
index 9cd0479..2cda42b 100644
--- a/src/graph.h
+++ b/src/graph.h
@@ -119,6 +119,7 @@
   const std::vector<Edge*>& out_edges() const { return out_edges_; }
   const std::vector<Edge*>& validation_out_edges() const { return validation_out_edges_; }
   void AddOutEdge(Edge* edge) { out_edges_.push_back(edge); }
+  void RemoveOutEdge(Edge* edge);
   void AddValidationOutEdge(Edge* edge) { validation_out_edges_.push_back(edge); }
 
   void Dump(const char* prefix="") const;
@@ -187,9 +188,9 @@
   Edge()
       : rule_(NULL), pool_(NULL), dyndep_(NULL), env_(NULL), mark_(VisitNone),
         id_(0), outputs_ready_(false), deps_loaded_(false),
-        deps_missing_(false), generated_by_dep_loader_(false),
-        command_start_time_(0), implicit_deps_(0), order_only_deps_(0),
-        implicit_outs_(0) {}
+        deps_missing_(false), has_restat_(-1), is_generator_(-1),
+        command_start_time_(0), static_implicit_deps_(0), implicit_deps_(0),
+        order_only_deps_(0), static_implicit_outs_(0), implicit_outs_(0) {}
 
   /// Return true if all inputs' in-edges are ready.
   bool AllInputsReady() const;
@@ -218,7 +219,6 @@
   /// Reset state of edge for next build / dependency scan.
   void ResetState() {
     outputs_ready_ = false;
-    deps_loaded_ = false;
     deps_missing_ = false;
     mark_ = VisitNone;
   }
@@ -233,8 +233,16 @@
   VisitMark mark_;
   size_t id_;
   bool outputs_ready_;
+
+  /// Set to true to indicate that this edge contains extra dependencies that
+  /// were loaded from depfiles, the deps log, or dyndep files.
   bool deps_loaded_;
+
   bool deps_missing_;
+
+  /// True for special phony edges that are created when loading extra
+  /// dependencies from depfiles or the deps log that do not have a
+  /// generating edge.
   bool generated_by_dep_loader_;
 
   /// Return the command hash for this edge.
@@ -261,14 +269,26 @@
 
   void SetRestat();
 
-  // There are three types of inputs.
+  // There are four types of inputs.
   // 1) explicit deps, which show up as $in on the command line;
-  // 2) implicit deps, which the target depends on implicitly (e.g. C headers),
-  //                   and changes in them cause the target to rebuild;
-  // 3) order-only deps, which are needed before the target builds but which
+  // 2) static implicit deps, which the target depends on implicitly (e.g. C
+  //    headers), as they appear in the build plan, and changes in them cause
+  //    the target to be rebuild.
+  // 3) dynamic implicit deps, which come from depfiles, the deps log or
+  //    dyndep files. They never appear in the build plan, and are inserted
+  //    into the build plan during incremental builds. They are otherwise
+  //    considered implicit dependencies.
+  // 4) order-only deps, which are needed before the target builds but which
   //                     don't cause the target to rebuild.
   // These are stored in inputs_ in that order, and we keep counts of
-  // #2 and #3 when we need to access the various subsets.
+  // #2, #3 and #4 when we need to access the various subsets.
+  //
+  //                static_implicit_deps_
+  //                    |
+  //  inputs_ [...|<----*----->|            |<--order_only_deps_-->]
+  //              |<------implicit_deps_--->|
+  //
+  int static_implicit_deps_;
   int implicit_deps_;
   int order_only_deps_;
   bool is_implicit(size_t index) {
@@ -279,16 +299,38 @@
     return index >= inputs_.size() - order_only_deps_;
   }
 
-  // There are two types of outputs.
+  // There are three types of outputs.
   // 1) explicit outs, which show up as $out on the command line;
-  // 2) implicit outs, which the target generates but are not part of $out.
+  // 2) static implicit outs, that the target generates in the build plan, and
+  //    which are not listed as part of $out.
+  // 3) dynamic implicit outs. which do not appear in the build plan, but
+  //    inserted into the build graph through dyndep files only.
   // These are stored in outputs_ in that order, and we keep a count of
-  // #2 to use when we need to access the various subsets.
+  // #2 and #3 to use when we need to access the various subsets.
+  //
+  //               static_implicit_outs_
+  //                        |
+  //  outputs_ [.....|<-----*------>|                 ]
+  //                 |<--------implicit_outs_-------->|
+  //
+  int static_implicit_outs_;
   int implicit_outs_;
   bool is_implicit_out(size_t index) const {
     return index >= outputs_.size() - implicit_outs_;
   }
 
+  /// Update the set of dynamic implicit inputs for this edge. These can
+  /// come from the deps log, a depfile, or a dyndep file. This method tries
+  /// to minimize the changes to the build graph during incremental builds.
+  void UpdateDynamicImplicitDeps(size_t new_count, Node* const* new_deps);
+
+  /// Update the set of dynamic implicit outputs for this edge. These only
+  /// come from dyndep files. On success, return true. On failure, which means
+  /// that one of the new outputs already has a producing edge, set |*err|
+  /// then return false.
+  bool UpdateDynamicImplicitOutputs(size_t new_count, Node* const* new_outs,
+                                    std::string* err);
+
   bool is_phony() const;
   bool use_console() const;
   bool maybe_phonycycle_diagnostic() const;
diff --git a/src/graph_test.cc b/src/graph_test.cc
index 9dba8af..8a738ec 100644
--- a/src/graph_test.cc
+++ b/src/graph_test.cc
@@ -298,13 +298,71 @@
   ASSERT_EQ("", err);
   EXPECT_FALSE(GetNode("out.o")->dirty());
 
-  state_.Reset();
   fs_.RemoveFile("out.o.d");
+
+  // Note that State::Reset() does not remove the recorded deps from the edge
+  // so a new dependency scan will ingore the depfile being removed.
+  state_.Reset();
+  EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), NULL, &err));
+  ASSERT_EQ("", err);
+  EXPECT_FALSE(GetNode("out.o")->dirty());
+
+  // Set the edge's |deps_are_invalid_| flag to ensure the next dependency
+  // scan will try to reload the depfile.
+  state_.Reset();
+  GetNode("out.o")->in_edge()->deps_loaded_ = false;
   EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), NULL, &err));
   ASSERT_EQ("", err);
   EXPECT_TRUE(GetNode("out.o")->dirty());
 }
 
+// Regression test for https://fxbug.dev/135792
+TEST_F(GraphTest, DepfileOutputChanged) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+                                      "rule catdep\n"
+                                      "  depfile = $out.d\n"
+                                      "  command = echo OUT > $out\n"
+                                      "build ./out: catdep\n"));
+  // Create three files as if 'out' had been built by a command that generated
+  // a depfile pointing to blobs/1, all three files have the same timestamp
+  // so the corresponding Node should not be dirty.
+  fs_.Create("out.d", "out: blobs/1\n");
+  fs_.Create("out", "");
+  fs_.Create("blobs/1", "1");
+
+  Node* out_node = GetNode("out");
+  Edge* out_edge = out_node->in_edge();
+  EXPECT_TRUE(out_edge->inputs_.empty());
+
+  // Perform a dependency scan, then verify that the node is not dirty, and
+  // that blobs/1 was injected into the edge's inputs.
+  string err;
+  EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), NULL, &err));
+  ASSERT_EQ("", err);
+  EXPECT_FALSE(out_node->dirty());
+  ASSERT_EQ(1u, out_edge->inputs_.size());
+  EXPECT_EQ(out_edge->inputs_[0]->path(), "blobs/1");
+
+  // Simulate a command that builds out while generating a depfile
+  // pointing to a different file. All three files have the same timestamp.
+  fs_.Tick();
+  fs_.Create("blobs/2", "2");
+  fs_.WriteFile("out.d", "out: blobs/2");
+  fs_.WriteFile("out", "");
+
+  state_.Reset();
+  out_edge->deps_loaded_ = false;
+
+  GetNode("out")->in_edge()->deps_loaded_ = false;
+  EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), NULL, &err));
+  ASSERT_EQ("", err);
+  EXPECT_FALSE(out_node->dirty());
+
+  // Verify that blobs/1 was removed from the list of inputs.
+  EXPECT_EQ(1u, out_edge->inputs_.size());
+  EXPECT_EQ(out_edge->inputs_[0]->path(), "blobs/2");
+}
+
 // Check that rule-level variables are in scope for eval.
 TEST_F(GraphTest, RuleVariablesInScope) {
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
@@ -923,6 +981,135 @@
   EXPECT_EQ(1u, edge->order_only_deps_);
 }
 
+TEST_F(GraphTest, DyndepLoadIncremental) {
+  AssertParse(&state_,
+              "rule r\n"
+              "  command = unused\n"
+              "build out: r in || dd\n"
+              "  dyndep = dd\n");
+
+  // First version of the dyndep file adds one implicit output, and
+  // one implicit input.
+  fs_.Create("dd",
+             "ninja_dyndep_version = 1\n"
+             "build out | implicit_out: dyndep | implicit1\n");
+
+  string err;
+  ASSERT_TRUE(GetNode("dd")->dyndep_pending());
+  EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), NULL, &err));
+
+  EXPECT_EQ("", err);
+  Node* out_node = GetNode("out");
+  Edge* edge = out_node->in_edge();
+
+  // Check that implicit_out was inserted as a dynamic implicit output.
+  ASSERT_EQ(2u, edge->outputs_.size());
+  EXPECT_EQ(0, edge->static_implicit_outs_);
+  EXPECT_EQ(1, edge->implicit_outs_);
+  EXPECT_EQ("out", edge->outputs_[0]->path());
+  EXPECT_EQ("implicit_out", edge->outputs_[1]->path());
+
+  // Check that implicit1 was inserted as a dynamic implicit input,
+  // i.e. before order-only deps in the inputs_ array.
+  ASSERT_EQ(3u, edge->inputs_.size());
+  EXPECT_EQ("in", edge->inputs_[0]->path());
+  EXPECT_EQ("implicit1", edge->inputs_[1]->path());
+  EXPECT_EQ("dd", edge->inputs_[2]->path());
+  EXPECT_EQ(0u, edge->static_implicit_deps_);
+  EXPECT_EQ(1u, edge->implicit_deps_);
+  EXPECT_EQ(1u, edge->order_only_deps_);
+  EXPECT_FALSE(edge->GetBindingBool("restat"));
+
+  // Perform a second scan, forcing a reload of the dyndep information.
+  // and verify that nothing changed.
+  state_.Reset();
+  edge->deps_loaded_ = false;
+  edge->dyndep_->set_dyndep_pending(true);
+  EXPECT_TRUE(scan_.RecomputeDirty(out_node, NULL, &err));
+
+  EXPECT_EQ("", err);
+  ASSERT_EQ(2u, edge->outputs_.size());
+  EXPECT_EQ(0, edge->static_implicit_outs_);
+  EXPECT_EQ(1, edge->implicit_outs_);
+  EXPECT_EQ("out", edge->outputs_[0]->path());
+  EXPECT_EQ("implicit_out", edge->outputs_[1]->path());
+
+  ASSERT_EQ(3u, edge->inputs_.size());
+  EXPECT_EQ("in", edge->inputs_[0]->path());
+  EXPECT_EQ("implicit1", edge->inputs_[1]->path());
+  EXPECT_EQ("dd", edge->inputs_[2]->path());
+  EXPECT_EQ(0u, edge->static_implicit_deps_);
+  EXPECT_EQ(1u, edge->implicit_deps_);
+  EXPECT_EQ(1u, edge->order_only_deps_);
+  EXPECT_FALSE(edge->GetBindingBool("restat"));
+
+  // Now modify the dyndep file content to add a new implicit output,
+  // and two new implicit inputs. Note that some of the new files are listed
+  // _before_ the current ones.
+  fs_.Tick();
+  fs_.WriteFile("dd",
+                "ninja_dyndep_version = 1\n"
+                "build out | implicit_out2 implicit_out: dyndep | implicit2 "
+                "implicit1 implicit3\n");
+
+  state_.Reset();
+  edge->deps_loaded_ = false;
+  edge->dyndep_->set_dyndep_pending(true);
+  EXPECT_TRUE(scan_.RecomputeDirty(out_node, NULL, &err));
+
+  EXPECT_EQ("", err);
+  // Verify that the new implicit output was inserted _after_ the first one
+  // even though it appeared before it in the dyndep file.
+  ASSERT_EQ(3u, edge->outputs_.size());
+  EXPECT_EQ(0, edge->static_implicit_outs_);
+  EXPECT_EQ(2, edge->implicit_outs_);
+  EXPECT_EQ("out", edge->outputs_[0]->path());
+  EXPECT_EQ("implicit_out", edge->outputs_[1]->path());
+  EXPECT_EQ("implicit_out2", edge->outputs_[2]->path());
+
+  // Verify that the new implicits input were inserted _after_ the first one
+  // even though some appeared before it in the dyndep file.
+  ASSERT_EQ(5u, edge->inputs_.size());
+  EXPECT_EQ("in", edge->inputs_[0]->path());
+  EXPECT_EQ("implicit1", edge->inputs_[1]->path());
+  EXPECT_EQ("implicit2", edge->inputs_[2]->path());
+  EXPECT_EQ("implicit3", edge->inputs_[3]->path());
+  EXPECT_EQ("dd", edge->inputs_[4]->path());
+  EXPECT_EQ(0u, edge->static_implicit_deps_);
+  EXPECT_EQ(3u, edge->implicit_deps_);
+  EXPECT_EQ(1u, edge->order_only_deps_);
+  EXPECT_FALSE(edge->GetBindingBool("restat"));
+
+  // Modify the dyndep file again to remove all implicit outputs, and the
+  // two implicit inputs.
+  fs_.Tick();
+  fs_.WriteFile("dd",
+                "ninja_dyndep_version = 1\n"
+                "build out: dyndep | implicit2\n");
+
+  state_.Reset();
+  edge->deps_loaded_ = false;
+  edge->dyndep_->set_dyndep_pending(true);
+  EXPECT_TRUE(scan_.RecomputeDirty(out_node, NULL, &err));
+
+  EXPECT_EQ("", err);
+  // Verify that all dynamic implicit outputs were removed.
+  ASSERT_EQ(1u, edge->outputs_.size());
+  EXPECT_EQ(0, edge->static_implicit_outs_);
+  EXPECT_EQ(0, edge->implicit_outs_);
+  EXPECT_EQ("out", edge->outputs_[0]->path());
+
+  // Verify that only one dynamic implicit input remains.
+  ASSERT_EQ(3u, edge->inputs_.size());
+  EXPECT_EQ("in", edge->inputs_[0]->path());
+  EXPECT_EQ("implicit2", edge->inputs_[1]->path());
+  EXPECT_EQ("dd", edge->inputs_[2]->path());
+  EXPECT_EQ(0u, edge->static_implicit_deps_);
+  EXPECT_EQ(1u, edge->implicit_deps_);
+  EXPECT_EQ(1u, edge->order_only_deps_);
+  EXPECT_FALSE(edge->GetBindingBool("restat"));
+}
+
 TEST_F(GraphTest, Validation) {
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
 "build out: cat in |@ validate\n"
diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index 67a9dee..d8c20fc 100644
--- a/src/manifest_parser.cc
+++ b/src/manifest_parser.cc
@@ -349,7 +349,7 @@
     delete edge;
     return true;
   }
-  edge->implicit_outs_ = implicit_outs;
+  edge->static_implicit_outs_ = edge->implicit_outs_ = implicit_outs;
 
   edge->inputs_.reserve(ins.size());
   for (vector<EvalString>::iterator i = ins.begin(); i != ins.end(); ++i) {
@@ -360,7 +360,7 @@
     CanonicalizePath(&path, &slash_bits);
     state_->AddIn(edge, path, slash_bits);
   }
-  edge->implicit_deps_ = implicit;
+  edge->static_implicit_deps_ = edge->implicit_deps_ = implicit;
   edge->order_only_deps_ = order_only;
 
   edge->validations_.reserve(validations.size());