aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2017-02-08 10:22:01 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2017-02-13 12:42:42 +0200
commit8eed1ebf9ca2532fac255708a8dc418378c78b0a (patch)
treec21bb8a590e70bdf9acd8c014204e8b5eb7f0c56
parent9aa99bc4e62909c119df72bda26b091245d48274 (diff)
Get rid of count_postponed
Terget execution is postponed with regards to the current thread only thus it doesn't seem right to make postponed a target state.
-rw-r--r--build2/algorithm4
-rw-r--r--build2/algorithm.cxx106
-rw-r--r--build2/operation.cxx4
-rw-r--r--build2/target16
-rw-r--r--build2/target.ixx1
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;
}