diff options
Diffstat (limited to 'libbuild2/algorithm.cxx')
| -rw-r--r-- | libbuild2/algorithm.cxx | 72 |
1 files changed, 50 insertions, 22 deletions
diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index 670355c..a2e1b57 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -1270,6 +1270,8 @@ namespace build2 static group_view resolve_members_impl (action a, const target& g, target_lock&& l) { + assert (a.inner ()); + // Note that we will be unlocked if the target is already applied. // group_view r; @@ -1296,27 +1298,6 @@ namespace build2 // Fall through. case target::offset_matched: { - // @@ Doing match without execute messes up our target_count. Does - // not seem like it will be easy to fix (we don't know whether - // someone else will execute this target). - // - // What if we always do match & execute together? After all, - // if a group can be resolved in apply(), then it can be - // resolved in match()! Feels a bit drastic. - // - // But, this won't be a problem if the target returns noop_recipe. - // And perhaps it's correct to fail if it's not noop_recipe but - // nobody executed it? Maybe not. - // - // Another option would be to have a count for such "matched but - // may not be executed" targets and then make sure target_count - // is less than that at the end. Though this definitelt makes it - // less exact (since we can end up executed this target but not - // some other). Maybe we can increment and decrement such targets - // in a separate count (i.e., mark their recipe as special or some - // such). - // - // Apply (locked). // pair<bool, target_state> s (match_impl (l, true /* step */)); @@ -1333,7 +1314,40 @@ namespace build2 if ((r = g.group_members (a)).members != nullptr) { - g.ctx.resolve_count.fetch_add (1, memory_order_relaxed); + // Doing match without execute messes up our target_count. There + // doesn't seem to be a clean way to solve this. Well, just always + // executing if we've done the match would have been clean but quite + // heavy-handed (it would be especially surprising if otherwise + // there is nothing else to do, which can happen, for example, + // during update-for-test when there are no tests to run). + // + // So our solution is as follows: + // + // 1. Keep track both of the targets that ended up in this situation + // (the target::resolve_counted flag) as well as their total + // count (the context::resolve_count member). Only do this if + // set_recipe() (called by match_impl()) would have incremented + // target_count. + // + // 2. If we happen to execute such a target (common case), then + // clear the flag and decrement the count. + // + // 3. When it's time to assert that target_count==0 (i.e., all the + // matched targets have been executed), check if resolve_count is + // 0. If it's not, then find every target with the flag set, + // pretend-execute it, and decrement both counts. See + // perform_execute() for further details on this step. + // + if (s.second != target_state::unchanged) + { + target::opstate& s (l.target->state[a]); // Inner. + + if (!s.recipe_group_action) + { + s.resolve_counted = true; + g.ctx.resolve_count.fetch_add (1, memory_order_relaxed); + } + } break; } @@ -1409,6 +1423,8 @@ namespace build2 void resolve_group_impl (action a, const target& t, target_lock&& l) { + assert (a.inner ()); + pair<bool, target_state> r ( match_impl (l, true /* step */, true /* try_match */)); @@ -2496,7 +2512,17 @@ namespace build2 // postponment logic (see excute_recipe() for details). // if (a.inner () && !s.recipe_group_action) + { + // See resolve_members_impl() for background. + // + if (s.resolve_counted) + { + s.resolve_counted = false; + ctx.resolve_count.fetch_sub (1, memory_order_relaxed); + } + 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. @@ -2516,6 +2542,8 @@ namespace build2 size_t start_count, atomic_count* task_count) { + // NOTE: see also pretend_execute lambda in perform_execute(). + target& t (const_cast<target&> (ct)); // MT-aware. target::opstate& s (t[a]); |