Bug 108430 - Improve how patch start, processing and finish times are shown on the EWS charts
Summary: Improve how patch start, processing and finish times are shown on the EWS charts
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-30 22:36 PST by Alan Cutter
Modified: 2014-02-05 11:16 PST (History)
3 users (show)

See Also:


Attachments
Patch (27.57 KB, patch)
2013-02-01 07:34 PST, Alan Cutter
no flags Details | Formatted Diff | Diff
Patch (27.60 KB, patch)
2013-02-01 07:47 PST, Alan Cutter
no flags Details | Formatted Diff | Diff
Patch (28.29 KB, patch)
2013-02-01 08:01 PST, Alan Cutter
no flags Details | Formatted Diff | Diff
Patch (28.14 KB, patch)
2013-02-06 01:33 PST, Alan Cutter
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Cutter 2013-01-30 22:36:55 PST
Currently only patches that have completed have their waiting time and process duration shown.
The dots are situated at the time the patch was created.

It would be better if they were situated at the time of their representative events. Their height can indicate the total run time of the patch so they line up horizontally.
Comment 1 Alan Cutter 2013-02-01 07:34:45 PST
Created attachment 186045 [details]
Patch
Comment 2 Alan Cutter 2013-02-01 07:47:17 PST
Created attachment 186046 [details]
Patch
Comment 3 Alan Cutter 2013-02-01 07:50:49 PST
(In reply to comment #2)
> Created an attachment (id=186046) [details]
> Patch

Preview can be seen here: http://108430.webkit-commit-queue.appspot.com/queue-charts/cr-linux-debug-ews
The state of the data it's presenting isn't in the best shape unfortunately.
Comment 4 Alan Cutter 2013-02-01 07:56:51 PST
Looks like this patch is conflicting with another one. Rebasing to resolve...
Comment 5 Alan Cutter 2013-02-01 08:01:37 PST
Created attachment 186049 [details]
Patch
Comment 6 Eric Seidel (no email) 2013-02-01 08:02:55 PST
Comment on attachment 186049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=186049&action=review

> Tools/QueueStatusServer/handlers/queuecharts.py:72
> -    def _get_min_med_max(cls, values, defaults=(0, 0, 0)):
> +    def _get_min_med_max(cls, values, defaults=(None, None, None)):

Um.  You want None here, not a list.  http://stackoverflow.com/questions/1132941/least-astonishment-in-python-the-mutable-default-argument
Comment 7 Alan Cutter 2013-02-01 08:07:08 PST
(In reply to comment #6)
> (From update of attachment 186049 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186049&action=review
> 
> > Tools/QueueStatusServer/handlers/queuecharts.py:72
> > -    def _get_min_med_max(cls, values, defaults=(0, 0, 0)):
> > +    def _get_min_med_max(cls, values, defaults=(None, None, None)):
> 
> Um.  You want None here, not a list.  http://stackoverflow.com/questions/1132941/least-astonishment-in-python-the-mutable-default-argument

Tuples are immutable, the default value cannot change state.
Comment 8 Eric Seidel (no email) 2013-02-01 08:13:13 PST
Oh, you're so right.  :)
Comment 9 Eric Seidel (no email) 2013-02-01 08:14:52 PST
Comment on attachment 186049 [details]
Patch

OK.
Comment 10 Eric Seidel (no email) 2013-02-01 08:15:24 PST
Comment on attachment 186049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=186049&action=review

> Tools/QueueStatusServer/templates/queuecharts.html:348
> +                // data.addRows([
> +                //     {% for patch_datum in patch_data %}
> +                //         [
> +                //             {{ patch_datum.seconds_ago }} / {{ time_unit }},
> +                //             {{ patch_datum.status_update_count }},
> +                //             "Patch {{ patch_datum.attachment_id }}\nStatus updates: " + {{ patch_datum.status_update_count }} + "\n" + secondsToString({{ patch_datum.seconds_ago }}) + " ago",
> +                //             {{ patch_datum.retry_count }},
> +                //             "Patch {{ patch_datum.attachment_id }}\nRetries: " + {{ patch_datum.retry_count }} + "\n" + secondsToString({{ patch_datum.seconds_ago }}) + " ago",
> +                //         ],
> +                //     {% endfor %}
> +                // ]);

Missed this.  We dont' normally commit commented out code, did you mean to leave this?
Comment 11 Alan Cutter 2013-02-06 01:33:43 PST
Created attachment 186784 [details]
Patch
Comment 12 Alan Cutter 2013-02-06 01:41:24 PST
(In reply to comment #10)
> (From update of attachment 186049 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186049&action=review
> 
> > Tools/QueueStatusServer/templates/queuecharts.html:348
> > +                // data.addRows([
> > +                //     {% for patch_datum in patch_data %}
> > +                //         [
> > +                //             {{ patch_datum.seconds_ago }} / {{ time_unit }},
> > +                //             {{ patch_datum.status_update_count }},
> > +                //             "Patch {{ patch_datum.attachment_id }}\nStatus updates: " + {{ patch_datum.status_update_count }} + "\n" + secondsToString({{ patch_datum.seconds_ago }}) + " ago",
> > +                //             {{ patch_datum.retry_count }},
> > +                //             "Patch {{ patch_datum.attachment_id }}\nRetries: " + {{ patch_datum.retry_count }} + "\n" + secondsToString({{ patch_datum.seconds_ago }}) + " ago",
> > +                //         ],
> > +                //     {% endfor %}
> > +                // ]);
> 
> Missed this.  We dont' normally commit commented out code, did you mean to leave this?

Ah, certainly not. That was a mistake, thanks for spotting it.
Comment 13 Build Bot 2013-02-06 07:54:18 PST
Comment on attachment 186784 [details]
Patch

Attachment 186784 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16393251
Comment 14 Alan Cutter 2013-02-18 01:58:33 PST
(In reply to comment #13)
> (From update of attachment 186784 [details])
> Attachment 186784 [details] did not pass win-ews (win):
> Output: http://queues.webkit.org/results/16393251

Build failure for win-ews unrelated to this patch. It is not part of the Webkit build.
Working version can be seen here: http://108430.webkit-commit-queue.appspot.com/queue-charts/cr-linux-debug-ews
Comment 15 Andreas Kling 2014-02-05 11:16:07 PST
Comment on attachment 186784 [details]
Patch

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.