Bug 157952 – We should have JSBench in PerformanceTests

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

We should have JSBench in PerformanceTests

[Keith Miller]

Created attachment 279496 [details]
Patch

[WebKit Commit Bot]

Attachment 279496 [details] did not pass style-queue:


ERROR: PerformanceTests/JSBench/harness.py:5:  trailing whitespace  [pep8/W291] [5]
ERROR: PerformanceTests/JSBench/harness.py:8:  trailing whitespace  [pep8/W291] [5]
ERROR: PerformanceTests/JSBench/harness.py:14:  trailing whitespace  [pep8/W291] [5]
ERROR: PerformanceTests/JSBench/harness.py:40:  whitespace before '}'  [pep8/E202] [5]
ERROR: PerformanceTests/JSBench/harness.py:91:  expected 2 blank lines, found 1  [pep8/E302] [5]
ERROR: PerformanceTests/JSBench/harness.py:124:  whitespace before '}'  [pep8/E202] [5]
ERROR: PerformanceTests/JSBench/harness.py:144:  whitespace before ')'  [pep8/E202] [5]
ERROR: PerformanceTests/JSBench/harness.py:182:  trailing whitespace  [pep8/W291] [5]
ERROR: PerformanceTests/JSBench/harness.py:202:  whitespace before '}'  [pep8/E202] [5]
Total errors found: 9 in 59 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

Wait, we already have this. Just run:
run-benchmark --plan jsbench --browser safari --platform osx

[Keith Miller]

(In reply to comment #3)
> Wait, we already have this. Just run:
> run-benchmark --plan jsbench --browser safari --platform OS X

This for run-jsc-benchmarks, which AB tests two different builds for regressions directly. It's easier to have everything managed by the one script rather than have to compute results on our own.

[Saam Barati]

Comment on attachment 279496 [details]
Patch

rs=me

[Yusuke Suzuki]

Hm, it seems that cq does not work well...

[Keith Miller]

(In reply to comment #6)
> Hm, it seems that cq does not work well...

Yeah, I guess I'll have to land this by hand :/

[Keith Miller]

Committed r201339: <http://trac.webkit.org/changeset/201339>

[Csaba Osztrogonác]

(In reply to comment #8)
> Committed r201339: <http://trac.webkit.org/changeset/201339>

These new tests fail on all performace bots, please fix or skip them. Otherwise why didn't you run them before committing?

[Keith Miller]

(In reply to comment #9)
> (In reply to comment #8)
> > Committed r201339: <http://trac.webkit.org/changeset/201339>
> 
> These new tests fail on all performace bots, please fix or skip them.
> Otherwise why didn't you run them before committing?

I didn't know the run-perf-tests script existed. I'll disable the tests for now, although I'll have to figure out how to do that.

[Csaba Osztrogonác]

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Committed r201339: <http://trac.webkit.org/changeset/201339>
> > 
> > These new tests fail on all performace bots, please fix or skip them.
> > Otherwise why didn't you run them before committing?

Log:
ERROR: layer at (0,0) size 800x600

Maybe it is trivial to fix them, the general rule that a performance
tests shouldn't dump any extra output. Otherwise it will fail.
 
> I didn't know the run-perf-tests script existed. I'll disable the tests for
> now, although I'll have to figure out how to do that.

Use PerformanceTests/Skipped file to skip them.

[Keith Miller]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #8)
> > > > Committed r201339: <http://trac.webkit.org/changeset/201339>
> > > 
> > > These new tests fail on all performace bots, please fix or skip them.
> > > Otherwise why didn't you run them before committing?
> 
> Log:
> ERROR: layer at (0,0) size 800x600
> 
> Maybe it is trivial to fix them, the general rule that a performance
> tests shouldn't dump any extra output. Otherwise it will fail.

Interesting, I'll look into that. I'm not sure if the run-jsc-benchmarks test harness uses that information or not however.

>  
> > I didn't know the run-perf-tests script existed. I'll disable the tests for
> > now, although I'll have to figure out how to do that.
> 
> Use PerformanceTests/Skipped file to skip them.

Should be fixed in http://trac.webkit.org/changeset/201372, hopefully.

[Yusuke Suzuki]

(In reply to comment #9)
> (In reply to comment #8)
> > Committed r201339: <http://trac.webkit.org/changeset/201339>
> 
> These new tests fail on all performace bots, please fix or skip them.
> Otherwise why didn't you run them before committing?

FYI, EFL / GTK ports can run this JSBench from run-jsc-benchmark script now :)
http://trac.webkit.org/changeset/201365