Cache "recipe is group_action" information, clear recipes after execution

4 files changed, 25 insertions, 12 deletions

diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx
index 8ef88e4..287ab2e 100644
--- a/libbuild2/algorithm.cxx
+++ b/libbuild2/algorithm.cxx
@@ -1914,14 +1914,20 @@ namespace build2
       ts = s.state = target_state::failed;
     }
 
+    // Clear the recipe to release any associated memory. Note that
+    // s.recipe_group_action may be used further (see, for example,
+    // group_state()) and should retain its value.
+    //
+    //
+    s.recipe = nullptr;
+
     // Decrement the target count (see set_recipe() for details).
     //
-    if (a.inner ())
-    {
-      recipe_function** f (s.recipe.target<recipe_function*> ());
-      if (f == nullptr || *f != &group_action)
-        ctx.target_count.fetch_sub (1, memory_order_relaxed);
-    }
+    // Note that here we cannot rely on s.state being group because of the
+    // postponment logic (see excute_recipe() for details).
+    //
+    if (a.inner () && !s.recipe_group_action)
+      ctx.target_count.fetch_sub (1, memory_order_relaxed);
 
     // Decrement the task count (to count_executed) and wake up any threads
     // that might be waiting for this target.
diff --git a/libbuild2/algorithm.ixx b/libbuild2/algorithm.ixx
index ce1ebee..02c430b 100644
--- a/libbuild2/algorithm.ixx
+++ b/libbuild2/algorithm.ixx
@@ -506,6 +506,7 @@ namespace build2
     target::opstate& s (t[l.action]);
 
     s.recipe = move (r);
+    s.recipe_group_action = false;
 
     // If this is a noop recipe, then mark the target unchanged to allow for
     // some optimizations.
@@ -531,9 +532,11 @@ namespace build2
       // likely. The alternative (trying to "merge" the count keeping track of
       // whether inner and/or outer is noop) gets hairy rather quickly.
       //
-      if (l.action.inner ())
+      if (f != nullptr && *f == &group_action)
+        s.recipe_group_action = true;
+      else
       {
-        if (f == nullptr || *f != &group_action)
+        if (l.action.inner ())
           t.ctx.target_count.fetch_add (1, memory_order_relaxed);
       }
     }
diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx
index 696d5d0..f652347 100644
--- a/libbuild2/target.hxx
+++ b/libbuild2/target.hxx
@@ -626,6 +626,7 @@ namespace build2
       // Applied recipe.
       //
       build2::recipe recipe;
+      bool           recipe_group_action; // Recipe is group_action.
 
       // Target state for this operation. Note that it is undetermined until
       // a rule is matched and recipe applied (see set_recipe()).
diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx
index 79cc07c..bdfa20f 100644
--- a/libbuild2/target.ixx
+++ b/libbuild2/target.ixx
@@ -268,16 +268,19 @@ namespace build2
     // raw state is not group provided the recipe is group_recipe and the
     // state is unknown (see mtime() for a discussion on why we do it).
     //
+    // Note that additionally s.state may not be target_state::group even
+    // after execution due to deferment (see execute_impl() for details).
+    //
+    // @@ Hm, I wonder why not just return s.recipe_group_action now that we
+    //    cache it.
+    //
     const opstate& s (state[a]);
 
     if (s.state == target_state::group)
       return true;
 
     if (s.state == target_state::unknown && group != nullptr)
-    {
-      if (recipe_function* const* f = s.recipe.target<recipe_function*> ())
-        return *f == &group_action;
-    }
+      return s.recipe_group_action;
 
     return false;
   }