Bug 97155 - run-perf-tests should record individual value instead of statistics
Summary: run-perf-tests should record individual value instead of statistics
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on: 97205
Blocks: 77037
  Show dependency treegraph
 
Reported: 2012-09-19 17:58 PDT by Ryosuke Niwa
Modified: 2012-09-20 16:59 PDT (History)
8 users (show)

See Also:


Attachments
Patch (16.65 KB, patch)
2012-09-19 18:05 PDT, Ryosuke Niwa
morrita: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2012-09-19 17:58:50 PDT
run-perf-tests should record indivisual value instead of statistics
Comment 1 Ryosuke Niwa 2012-09-19 18:05:34 PDT
Created attachment 164811 [details]
Patch
Comment 2 Ryosuke Niwa 2012-09-19 18:06:54 PDT
I don't have access to perf-o-matic code this week so I'll update it next week.
Comment 3 WebKit Review Bot 2012-09-19 18:57:23 PDT
Comment on attachment 164811 [details]
Patch

Attachment 164811 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13954001

New failing tests:
http/tests/css/link-css-disabled-value-with-slow-loading-sheet.html
Comment 4 Hajime Morrita 2012-09-19 18:59:45 PDT
Comment on attachment 164811 [details]
Patch

The code looks good considering there are some usecases for this value so... what is this for?
Comment 5 Ryosuke Niwa 2012-09-19 19:32:53 PDT
(In reply to comment #4)
> (From update of attachment 164811 [details])
> The code looks good considering there are some usecases for this value so... what is this for?

In the long term, perf-o-matic should be able to store these values and do some statistical analysis on them. e.g. Once we ported Datazilla, it has a built-in student's t-test and other things. For now, we can just report them on results page so that we can use them to analyze test results locally.
Comment 6 Hajime Morrita 2012-09-19 19:43:14 PDT
(In reply to comment #5)
> In the long term, perf-o-matic should be able to store these values and do some statistical analysis on them. e.g. Once we ported Datazilla, it has a built-in student's t-test and other things. For now, we can just report them on results page so that we can use them to analyze test results locally.

Got it. Thanks.
Comment 7 WebKit Review Bot 2012-09-19 21:24:47 PDT
Comment on attachment 164811 [details]
Patch

Rejecting attachment 164811 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

Last 500 characters of output:
ueue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 70, in run_and_handle_errors
    self._run(tool, options, state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 64, in _run
    step(tool, options).run(state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 50, in run
    if changelog_entry.has_valid_reviewer():
AttributeError: 'NoneType' object has no attribute 'has_valid_reviewer'

Full output: http://queues.webkit.org/results/13916045
Comment 8 Ryosuke Niwa 2012-09-19 22:13:44 PDT
Committed r129091: <http://trac.webkit.org/changeset/129091>
Comment 9 WebKit Review Bot 2012-09-20 06:08:07 PDT
Re-opened since this is blocked by 97205
Comment 11 Ryosuke Niwa 2012-09-20 10:41:24 PDT
Could you not roll out these perf-test patches? In practice, nobody is looking at the results of these bots (webkit-perf.appspot.com) and bots don't report whether performance has regressed or not anyway.

It causes too much svn commit churn if we were rolling out every single patch like this.
Comment 12 Ryosuke Niwa 2012-09-20 10:42:55 PDT
Perf. bots are still in development, and rolling out patches just because tests started to fail will slow down the development process than anything else. It's not helpful.
Comment 13 Csaba Osztrogonác 2012-09-20 12:35:14 PDT
Sure. But I don't understand why we should run broken perf bots for days. 
It is wasting the CPU resources ...
Comment 14 Ryosuke Niwa 2012-09-20 13:04:01 PDT
Committed r129158: <http://trac.webkit.org/changeset/129158>
Comment 15 Hajime Morrita 2012-09-20 16:47:58 PDT
(In reply to comment #12)
> Perf. bots are still in development, and rolling out patches just because tests started to fail will slow down the development process than anything else. It's not helpful.

Having red in http://build.webkit.org/console hurts the perception
of the project health, even though current situation is embracing constant redness.
If we expect long-running redness, we probably should hide it or make it clear by givin them a separate group IMO.

It doesn't seems polite behavior for me to blame people
who revert the patch which made bots red.
Comment 16 Ryosuke Niwa 2012-09-20 16:59:25 PDT
(In reply to comment #15)
> (In reply to comment #12)
> > Perf. bots are still in development, and rolling out patches just because tests started to fail will slow down the development process than anything else. It's not helpful.
> 
> If we expect long-running redness, we probably should hide it or make it clear by givin them a separate group IMO.

Yeah, it might make sense for us to add a new category for Perf bots and put them there. We might have added prematurely. It has helped us catching some regressions in the past, but the entire framework is very immature compared to layout tests and other test frameworks we have.