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

Issue 651421 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Oct 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

4.6% regression in webrtc_perf_tests at 14345:14347

Project Member Reported by asapersson@chromium.org, Sep 29 2016

Issue description

See graph below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=651421

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgpczE4wgM


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

webrtc-android-tests-nexus72
Cc: asapersson@chromium.org
Owner: ossu@chromium.org
Can you please have a look whether https://codereview.webrtc.org/2342443005 is the cause for this change and verify if it is expected. 

Comment 3 by ossu@chromium.org, Oct 4 2016

I've been unable to reproduce this locally - although I've only tried on my workstation, not on an actual device. I'm going to let the change I made that's related to  issue 651426  land first and see if the numbers improve here as well.

Comment 4 by ossu@chromium.org, Oct 5 2016

There's no clear improvement since 14495 landed (which addresses  issue 651426 ). I'll take a proper look at this one again.

Comment 5 by ossu@chromium.org, Oct 6 2016

I've run through revisions 14343 through 14351 on a Nexus 4 (the closest to a Nexus 7.2 I could get a hold of) and now believe the regression is due to added DCHECKs in the code. The runtime averaged over five runs for each revision is as follows (revisions 14349 and 14350 didn't introduce any changes to webrtc_perf_tests):

        dcheck   dcheck
 rev. |   on   |  off
14343 | 18962  | 16437
14344 | 18969  | 16376
14345 | 18982  | 16338
14346 | 18984  | 16238
14347 | 20014  | 16164 <-- my CL
14348 | 19745  | 15993
14351 | 19776  | 15901

Strangely enough, it looks like release-build performance actually increased with my CL, rather than decreased. I expect the perf tests (like our trybots) build with dcheck_always_on=true, which would corroborate my findings. Who would know if that is the case?

Comment 6 by ossu@chromium.org, Oct 7 2016

Cc: phoglund@chromium.org
+phoglund

Are the binaries built for perf tests using dcheck_always_on=true, and should they? (See previous comment)
Cc: ehmaldonado@chromium.org kjellander@chromium.org
Yes: https://uberchromegw.corp.google.com/i/client.webrtc.perf/builders/Android32%20Builder/builds/224/steps/generate_build_files/logs/stdio

Should they? Not sure. I think we've discussed this at length before, but I don't remember what we came up with.
We've discussed this in the past until recently we've been using the "large tests" bots  (which build Release builds) as a platform for both correctness and performance tests. This is not ideal and Chromium has a clear separation between the two.

Having this, the benefit of having DCHECKs enabled for the non-perf tests was prioritized over disabling them for the perf tests (doing an additional compilation was another option but it wasn't considered). 

Now, however, we've created a separate perf waterfall: client.webrtc.perf and we recently moved the Android perf bots in there when we flipped to Swarming. We're going to do the same for Desktop as soon we get 3 new machines allocated: bug 657572.

So right now, we can remove dcheck_always_on=true for these Android bots, but not for the desktop platforms. I filed https://bugs.chromium.org/p/webrtc/issues/detail?id=6635 to track that work.

Comment 9 by ossu@chromium.org, Oct 31 2016

Status: WontFix (was: Assigned)
Greatness! I'm closing this bug as WontFix, since as far as I can tell, the measurements didn't highlight a proper regression (i.e. only due to DCHECKs).
The future changes to perf bots, turning DCHECKs off, will likely invalidate these measurements anyways, both before and after this regression.

Sign in to add a comment