Most of the time, it's wasteful for EWS bots to keep building or testing once a patch has become invalid (e.g.: obsoleted or r-). It would be much better to drop obsoleted patches quickly, and to move to ones that people are actually waiting for. One step in this direction is for EWS to check if patch is still valid before executing every major step. Also see <rdar://problem/27768813>
Created attachment 285733 [details] Proposed patch This does not stop the processing once the build/command has started, but checks for validation more frequently. For e.g.: if the patch fails to build, before attempting _build_without_patch, we would validate and stop. Note: This fails unit tests because of the change in logic, I will fix it. I want to get feedback on the logic first.
Comment on attachment 285733 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=285733&action=review > Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py:88 > + self.validate() Do you have a list of steps that now validate, and they didn't before? It's not obvious to me if this change provides a noticeable improvement - the real benefit would come from the ability to interrupt builds and tests, which would be a much larger change.
This patch perform validation before any step which executes _run_command(). There are 8 such methods: _build_without_patch, _build_and_test_without_patch*(), _clean(), _update(), _apply(), _build(), _land() and _test(). Earlier we were doing validate before clean() and if validation pass, we run all the next steps. I think the major improvement in this patch would be to skip: _build_without_patch, _build_and_test_without_patch and _test. For e.g.: _build_without_patch might take significant time, we can skip it for obsolete patches.
Comment on attachment 285733 [details] Proposed patch Makes sense, that's a non-trivial improvement. There is some risk of reporting tools being broken by not running all the steps, we should look out for that.
Created attachment 285756 [details] Updated patch Simplified the patch and updated unit-tests. Please review again.
Comment on attachment 285756 [details] Updated patch Clearing flags on attachment: 285756 Committed r204382: <http://trac.webkit.org/changeset/204382>
All reviewed patches have been landed. Closing bug.