diff options
| -rw-r--r-- | build2/algorithm | 4 | ||||
| -rw-r--r-- | build2/algorithm.cxx | 106 | ||||
| -rw-r--r-- | build2/operation.cxx | 4 | ||||
| -rw-r--r-- | build2/target | 16 | ||||
| -rw-r--r-- | build2/target.ixx | 1 |
5 files changed, 61 insertions, 70 deletions
diff --git a/build2/algorithm b/build2/algorithm index e9098bd..5ee6c64 100644 --- a/build2/algorithm +++ b/build2/algorithm @@ -172,9 +172,9 @@ namespace build2 execute_delegate (const recipe&, action, const target&); // A special version of the above that should be used for "direct" and "now" - // execution, that is, side-stepping the normal target- prerequisite + // execution, that is, side-stepping the normal target-prerequisite // relationship (so no dependents count is decremented) and execution order - // (so this function will never return postponed target state). It will also + // (so this function never returns the postponed target state). It will also // wait for the completion if the target is busy. // target_state diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx index 872b10e..3887c64 100644 --- a/build2/algorithm.cxx +++ b/build2/algorithm.cxx @@ -405,15 +405,23 @@ namespace build2 })); target_state ts (t.recipe (a) (a, t)); - assert (ts != target_state::unknown && ts != target_state::failed); - t.state_ = ts; - if (ts == target_state::group) - ts = t.group->state_; + // The the recipe documentation for details on what's going on here. + // + switch (t.state_ = ts) + { + case target_state::changed: + case target_state::unchanged: break; + case target_state::postponed: t.state_ = target_state::unknown; break; + case target_state::group: ts = t.group->state_; break; + default: assert (false); + } // Decrement the task count (to count_executed) and wake up any threads // that might be waiting for this target. // + // @@ MT: not happenning in case of an exception! + // size_t tc (t.task_count--); assert (tc == target::count_executing); sched.resume (t.task_count); @@ -445,65 +453,50 @@ namespace build2 // Handle the "last" execution mode. // - // This gets interesting when we consider interaction with - // groups. It seem to make sense to treat group members as - // dependents of the group, so, for example, if we try to - // clean the group via three of its members, only the last - // attempt will actually execute the clean. This means that - // when we match a group member, inside we should also match - // the group in order to increment the dependents count. This - // seems to be a natural requirement: if we are delegating to - // the group, we need to find a recipe for it, just like we - // would for a prerequisite. - // - // Note that below we are going to change the group state - // to postponed. This is not a mistake: until we execute - // the recipe, we want to keep returning postponed. And - // once the recipe is executed, it will reset the state - // to group (see group_action()). To put it another way, - // the execution of this member is postponed, not of the - // group. - // - // One important invariant to keep in mind: the return - // value from execute() should always be the same as what - // would get returned by a subsequent call to state(). + // This gets interesting when we consider interaction with groups. It seem + // to make sense to treat group members as dependents of the group, so, + // for example, if we try to clean the group via three of its members, + // only the last attempt will actually execute the clean. This means that + // when we match a group member, inside we should also match the group in + // order to increment the dependents count. This seems to be a natural + // requirement: if we are delegating to the group, we need to find a + // recipe for it, just like we would for a prerequisite. + // + // Note that below we are going to treat the group state to postponed. + // This is not a mistake: until we execute the recipe, we want to keep + // returning postponed. And once the recipe is executed, it will reset the + // state to group (see group_action()). To put it another way, the + // execution of this member is postponed, not of the group. + // + // Note also that the target execution is postponed with regards to this + // thread. For other threads the state will still be unknown (until they + // try to execute it). // - size_t tc (target::count_unexecuted); if (current_mode == execution_mode::last && d != 0) + return target_state::postponed; + + // Try to atomically change unexecuted to executing. + // + size_t tc (target::count_unexecuted); + if (t.task_count.compare_exchange_strong (tc, target::count_executing)) { - // Try to atomically change unexecuted to postponed. - // - if (t.task_count.compare_exchange_strong (tc, target::count_postponed)) - tc = target::count_postponed; - } - else - { - // Try to atomically change unexecuted to executing. But it can also be - // postponed to executing. - // - if (t.task_count.compare_exchange_strong (tc, target::count_executing) || - (tc == target::count_postponed && - t.task_count.compare_exchange_strong (tc, target::count_executing))) - { - if (task_count == nullptr) - return execute_impl (a, t); - - sched.async (start_count, - *task_count, - [a] (target& t) - { - execute_impl (a, t); // @@ MT exception handling. - }, - ref (t)); - - return target_state::unknown; - } + if (task_count == nullptr) + return execute_impl (a, t); + + sched.async (start_count, + *task_count, + [a] (target& t) + { + execute_impl (a, t); // @@ MT exception handling. + }, + ref (t)); + + return target_state::unknown; } switch (tc) { case target::count_unexecuted: assert (false); - case target::count_postponed: return target_state::postponed; case target::count_executed: return t.synchronized_state (); default: return target_state::busy; } @@ -522,8 +515,7 @@ namespace build2 // switch (tc) { - case target::count_unexecuted: - case target::count_postponed: assert (false); + case target::count_unexecuted: assert (false); case target::count_executed: break; default: sched.wait (target::count_executed, t.task_count); diff --git a/build2/operation.cxx b/build2/operation.cxx index f37d642..ea9a462 100644 --- a/build2/operation.cxx +++ b/build2/operation.cxx @@ -168,10 +168,10 @@ namespace build2 } case target_state::changed: break; - case target_state::postponed: - assert (false); case target_state::failed: //@@ MT: This could probably happen in a parallel build. + // Or does state() throw? Do we want to print? + break; default: assert (false); } diff --git a/build2/target b/build2/target index 6cc492a..e709395 100644 --- a/build2/target +++ b/build2/target @@ -63,13 +63,15 @@ namespace build2 // Recipe. // - // The returned target state should be changed, unchanged, or postponed, - // though you shouldn't be returning postponed directly. If there is an - // error, then the recipe should throw rather than returning failed. + // The returned target state is normally changed or unchanged. If there is + // an error, then the recipe should throw rather than returning failed. // // The return value of the recipe is used to update the target state. If it // is target_state::group then the target's state is the group's state. // + // The recipe can also return postponed. In this case the target state is + // set to unknown but the postponed state is propagated to the caller. + // // Note that max size for the "small capture optimization" in std::function // ranges (in pointer sizes) from 0 (GCC prior to 5) to 2 (GCC 5) to 6 (VC // 14u2). With the size ranging (in bytes for 64-bit target) from 32 (GCC) @@ -410,8 +412,7 @@ namespace build2 // subset of the target's state as well as the number of its sub-tasks // (execution of prerequisites). // - // The count starts unexecuted and can then transition to postponed or - // executing. Postponed can transition to executing. And executing + // The count starts unexecuted then transitions executing. Executing // transitions (via a decrement) to executed. Once it is executed, then // state_ becomes immutable. // @@ -422,9 +423,8 @@ namespace build2 // case) its state, mtime, etc. // static const size_t count_unexecuted = 0; - static const size_t count_postponed = 1; - static const size_t count_executed = 2; - static const size_t count_executing = 3; + static const size_t count_executed = 1; + static const size_t count_executing = 2; mutable atomic_count task_count; diff --git a/build2/target.ixx b/build2/target.ixx index 68a683a..a097e39 100644 --- a/build2/target.ixx +++ b/build2/target.ixx @@ -31,7 +31,6 @@ namespace build2 switch (task_count) { case target::count_unexecuted: return target_state::unknown; - case target::count_postponed: return target_state::postponed; case target::count_executed: return synchronized_state (); default: return target_state::busy; } |