Bug 46254 - Optimize commit-queue performance for green trees
Summary: Optimize commit-queue performance for green trees
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-22 02:21 PDT by Adam Barth
Modified: 2010-09-22 17:37 PDT (History)
1 user (show)

See Also:

Attachments
work in progress (15.92 KB, patch)
2010-09-22 02:26 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch (31.52 KB, patch)
2010-09-22 17:22 PDT, Adam Barth 		eric: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-09-22 02:21:15 PDT 
Optimize commit-queue performance for green trees
Comment 1 Adam Barth 2010-09-22 02:26:07 PDT 
Created attachment 68352 [details]
work in progress
Comment 2 Adam Barth 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.
Comment 3 Eric Seidel (no email) 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.
Comment 4 Adam Barth 2010-09-22 17:22:08 PDT 
Created attachment 68477 [details]
Patch
Comment 5 Eric Seidel (no email) 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
Comment 6 Adam Barth 2010-09-22 17:37:58 PDT 
Committed r68102: <http://trac.webkit.org/changeset/68102>