Bug 46254

Summary: Optimize commit-queue performance for green trees
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
work in progress
none
Patch eric: review+

Adam Barth
Reported 2010-09-22 02:21:15 PDT
Optimize commit-queue performance for green trees
Attachments
work in progress (15.92 KB, patch)
2010-09-22 02:26 PDT, Adam Barth
no flags
Patch (31.52 KB, patch)
2010-09-22 17:22 PDT, Adam Barth
eric: review+
Adam Barth
Comment 1 2010-09-22 02:26:07 PDT
Created attachment 68352 [details] work in progress
Adam Barth
Comment 2 2010-09-22 02:27:59 PDT
I need to update existing tests and add new tests. Factoring out the CommitQueueTask object should allow for much better testing of this logic.
Eric Seidel (no email)
Comment 3 2010-09-22 10:03:43 PDT
Comment on attachment 68352 [details] work in progress View in context: https://bugs.webkit.org/attachment.cgi?id=68352&action=review This patch is absolutely fantastic. > WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py:136 > + if not self._patch.is_rollout(): > + if not self._test(): > + if not self._test(): > + if not self._build_and_test_without_patch(): > + return False > + raise self._script_error This block is kinda confusing. I think it may need some comments to annotate what its doing. We could also have a _validate_and_land() command which it called instead, and reverse the logic. But it's kinda nice having the ifs be always for the failure cases.
Adam Barth
Comment 4 2010-09-22 17:22:08 PDT
Eric Seidel (no email)
Comment 5 2010-09-22 17:30:34 PDT
Comment on attachment 68477 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68477&action=review As far as I can tell this is "right" :) > WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py:41 > + # commit-queue is processing. Do one last minute check before landing. this is no longer last minute. we can remove that part
Adam Barth
Comment 6 2010-09-22 17:37:58 PDT
Note You need to log in before you can comment on or make changes to this bug.