task iteration improvements #16
Conversation
| return { | ||
| itask for itask in itasks | ||
| if self._match_ext_trigger(itask) | ||
| } |
There was a problem hiding this comment.
Saves building and destroying an intermediate list.
There was a problem hiding this comment.
It seems I wasn't aware of the existence of set comphrehensions (until a recent previous review, anyway) .. 🤦
| if any( | ||
| itask for itask in self.pool.get_tasks() | ||
| if itask.state( | ||
| TASK_STATUS_PREPARING, | ||
| TASK_STATUS_SUBMITTED, | ||
| TASK_STATUS_RUNNING | ||
| ) | ||
| or (itask.state(TASK_STATUS_WAITING) | ||
| and not itask.state.is_runahead) | ||
| ): |
There was a problem hiding this comment.
This will stop iterating on the first match.
| TASK_STATUS_EXPIRED | ||
| ) | ||
| or itask.is_manual_submit | ||
| ): |
There was a problem hiding this comment.
Simplification, avoids building and destroying intermediate lists.
| if points or all( | ||
| not itask.state( | ||
| TASK_STATUS_FAILED, | ||
| TASK_STATUS_SUCCEEDED, | ||
| TASK_STATUS_EXPIRED | ||
| ): | ||
| has_unfinished_itasks = True | ||
| break | ||
| if not points and not has_unfinished_itasks: | ||
| # We need to begin with an unfinished cycle point. | ||
| continue | ||
| points.append(point) | ||
| ) | ||
| for itask in itasks | ||
| ): | ||
| points.append(point) |
There was a problem hiding this comment.
Don't need to iterate itasks if not points.
There was a problem hiding this comment.
(Except that should be any() not all())
| for itask in ( | ||
| itask | ||
| for point, itask_id_map in self.main_pool.items() | ||
| for itask in itask_id_map.values() | ||
| if point <= latest_allowed_point | ||
| if itask.state.is_runahead | ||
| ): |
There was a problem hiding this comment.
Saves building and destroying intermediate list.
There was a problem hiding this comment.
Actually this one broke some tests, which did my head in for a bit because I could not see how your code could give a different result. The problem is, the release_runahead_tasks() call inside the loop can sometimes result in the dictionary changing size during iteration (because: parent-less tasks get spawned when their previous instance gets released from runahead-limiting). So the intermediate list is necessary here. I'll insert a comment to explain why, for future reference.
There was a problem hiding this comment.
(The original can still be improved a bit though: use your version to build the list).
| satisfied = { | ||
| itask for itask in itasks | ||
| if itask.state.xtriggers_all_satisfied() | ||
| } |
There was a problem hiding this comment.
Saves building and destroying intermediate list.
No description provided.