New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 766556 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

98.2% regression in blink_perf.canvas at 502079:502153

Project Member Reported by alexclarke@chromium.org, Sep 19 2017

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Sep 19 2017

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=766556

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=f3891f29fce712f6e65fa3de0128a542bf45da8be1670aa2caaa2eb70914905a


Bot(s) for this bug's original alert(s):

chromium-rel-mac11-air
Project Member

Comment 2 by 42576172...@developer.gserviceaccount.com, Sep 19 2017

๐Ÿ“ Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/14a92fcfb80000

Comment 3 by dtu@chromium.org, Sep 22 2017

Looks like the job just stopped running. Filed https://github.com/catapult-project/catapult/issues/3891 for that.
I'll re-kick the bisect as well.
Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, Sep 22 2017

๐Ÿ“ Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/14af21c0780000
Project Member

Comment 5 by 42576172...@developer.gserviceaccount.com, Sep 22 2017

๐Ÿ“ Pinpoint job completed.
https://pinpoint-dot-chromeperf.appspot.com/job/14af21c0780000

Found significant differences after 1 commit:

gpu: don't allow reentrancy in GpuChannelHost::Send
By piman@chromium.org ยท Fri Sep 15 01:26:06 2017
chromium@b5edb4f2db00acedc8726ba4fbdcbf03a9ae7d49
Owner: piman@chromium.org
Status: Assigned (was: Untriaged)
Assigning to piman per #5
Project Member

Comment 7 by 42576172...@developer.gserviceaccount.com, Sep 22 2017

๐Ÿ“ Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/12c70d88780000

Comment 8 by piman@chromium.org, Sep 22 2017

I will investigate. I suspect something was fishy before my patch and somehow it fixed it.

The historical number was ~2200-2400 runs/s. Recently there was a massive improvement to ~400000 runs/s at https://chromium.googlesource.com/chromium/src/+log/a7d11af324c214dc2c64cc1dbfbb8d3772831a3f%5E..14cb93b47f330a39bf34613f1b921407324bf694?pretty=fuller&n=1000
After my patch the number goes back down to between 2300 and 4600 (seems a bit of a bimodal distribution, but not worse than the historical number).

I just don't believe the ~400000 number :)
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Sep 22 2017

๐Ÿ“ Pinpoint job completed.
https://pinpoint-dot-chromeperf.appspot.com/job/12c70d88780000

Found significant differences after 1 commit:

[Mac] A potential fix for WaitableEvent::WaitMany on macOS 10.11.
By rsesek@chromium.org ยท Mon Aug 28 19:51:25 2017
chromium@5cd8eef498e28e1aacb9109645c001bbbfe1bc26

Comment 10 by piman@chromium.org, Sep 22 2017

Cc: rsesek@chromium.org
What caused the numbers to go crazy is this patch on #9. After that patch a lot of results look like things go infinitely fast, skewing the overall number. That doesn't happen any more after my patch and things go back to normal

Comment 11 by piman@chromium.org, Sep 25 2017

I would like to understand exactly what's happening before closing, because the original regression seems bad, and might affect 62 - my patch was not intended to fix anything specifically in that area.
However I'm running into https://bugs.chromium.org/p/chromium/issues/detail?id=768530 which means I can't get traces from the bot. I will try to repro locally but I don't have the exact same spec.

Comment 12 by piman@chromium.org, Sep 25 2017

Cc: dongseong.hwang@chromium.org junov@chromium.org
Status: WontFix (was: Assigned)
I reproduced some of the effects locally. It looks like the crazy numbers happen when the video is restarting after the loop. Depending on timing, it may or may not happen during the test. If the test gets a little slower, then the video end and restart happen during the test loop, giving a couple (out of 20) out-of-whack measurements (1000x faster), which completely skew the results and gives better numbers (when they should be worse).

Specifically, one thing that seems to largely affect the measurement is GpuChannelHost::Send (for sync messages) which is called every time. Before my patch, it used to call WaitableEvent::WaitMany, which was changed on that config (Mac 10.11) by rsesek's 5cd8eef498e28e1aacb9109645c001bbbfe1bc26 - I assume it made it a tad slower, bringing the video looping behavior into the test loop. It was also changed by my patch, which makes it faster on platforms I looked at (Linux, Mac 10.12), which would kick the looping behavior out of the test loop. I can confirm if I can grab traces after  crbug.com/768530  is fixed.

Locally (MBP on 10.12), I was able to get much more consistent numbers by slowing down the video (adding 'videoElement.playbackRate = 0.1;' to the source), which doesn't really affect what the benchmark measures, but makes sure the video doesn't loop. I got an improvement by about 8% (8737 vs 8099 avg) from my patch.
I filed  crbug.com/768587  about the benchmark issue

Bottom-line: this was a progression rather than a regression, but the test behaves poorly.

Sign in to add a comment