Issue metadata
Sign in to add a comment
|
82.5% regression in webrtc_perf_tests at 13946:13946 due to changed optimizations. |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Aug 29 2016
kjellander: this is caused by "MB: Flip Windows bots to GN by default" (https://codereview.webrtc.org/2277253002). Is this expected?
,
Aug 29 2016
Certainly not expected. Edward and I were just about to start auditing the differences between the GYP and GN build. Seems like we'll start by looking at this target tomorrow!
,
Aug 30 2016
This regression might be caused because GYP is compiled with /O2 while GN is compiled with /O1 It seems like the default in GYP is O2: https://cs.chromium.org/chromium/src/build/common.gypi?l=2482 But it's O1 in GN: https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?l=1345 Although there is also a "optimize_max" compile config in GN: https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?l=1398 The differences in the compile flags are: In gyp, but not in GN: /GF /GS /O2 /fp:precise /we4389 In GN, but not in gyp: /O1 /Oi
,
Aug 31 2016
I'm going to let you own this, since you're already looking at it.
,
Aug 31 2016
Hmm, it doesn't seem like Edwards CL https://codereview.webrtc.org/2291253003 helped. The points in the graph with ID 14001 is unchanged. I verified in https://build.chromium.org/p/client.webrtc/builders/Win32%20Release%20%5Blarge%20tests%5D/builds/8658/steps/compile/logs/stdio that the relevant object are actually recompiled. I'll force a clobber build on the bot just to be sure.
,
Sep 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/59af8b77145b8ff33a610128d339f02f950d3e7a commit 59af8b77145b8ff33a610128d339f02f950d3e7a Author: ehmaldonado <ehmaldonado@webrtc.org> Date: Mon Sep 05 09:48:54 2016 GN Templates: Use the optimize_max compiler config. Add "//build/config/compiler:optimize_max" to rtc_add_configs and "//build/config/compiler:default_optimization" to rtc_remove_configs. This is the default optimization in GYP, and might help explain a 82.5% regression in webrtc_perf_tests at 13946:13946 BUG= chromium:641966 NOTRY=True Review-Url: https://codereview.webrtc.org/2307283002 Cr-Commit-Position: refs/heads/master@{#14067} [modify] https://crrev.com/59af8b77145b8ff33a610128d339f02f950d3e7a/webrtc/BUILD.gn [modify] https://crrev.com/59af8b77145b8ff33a610128d339f02f950d3e7a/webrtc/build/webrtc.gni [modify] https://crrev.com/59af8b77145b8ff33a610128d339f02f950d3e7a/webrtc/modules/audio_coding/BUILD.gn
,
Sep 5 2016
Wohoo, graphs are restored with the commit in #8 above!
,
Sep 6 2016
Nico asked in https://codereview.webrtc.org/2307283002/ if it was necessary as they saw a size regression in Chrome due to our fix. +hlundin regarding if this regression is serious or something we can just discard referencing the changed optimization level.
,
Sep 7 2016
Another question in https://codereview.webrtc.org/2307283002/: Does NetEq test a code path that's hot in chrome as well? hlundin: can you provide a detailed answer?
,
Sep 7 2016
This test runs NetEq stand-alone at maximum speed (faster than real-time). NetEq usually accounts for just a percent or so of the total WebRTC load. Video encoding, decoding and processing are usually much larger. So even an 80% regression for NetEq probably only translates to a few percents in total for an HD Video WebRTC call. But the question is how many other components are affected by this. This test looks only at NetEq.
,
Sep 9 2016
So to me it sounds like increasing chrome's size quite a bit for this isn't worth it. Is that incorrect? (It looks like that patch is still in, so I guess someone disagrees? Why?)
,
Sep 12 2016
I'm reverting the optimization change for Win Release in https://codereview.webrtc.org/2334593002/ so we're going to see this test regress again. I'll inform our perf sheriffs so they can connect the new alerts to this bug.
,
Sep 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/11d57668190edb1f4b8208500da3ab33f638dee3 commit 11d57668190edb1f4b8208500da3ab33f638dee3 Author: kjellander <kjellander@webrtc.org> Date: Mon Sep 12 06:29:36 2016 GN: Revert to default compiler optimizations for Win Release. Revert back to the GN/Chromium default optimization levels what was changed in https://codereview.webrtc.org/2307283002/ and https://codereview.webrtc.org/2305403002/ due to a performance regression. Those changes caused a size regression in Chromium, which is why this is changed back. BUG= chromium:641966 NOTRY=True TBR=ehmaldonado@webrtc.org, henrik.lundin@webrtc.org Review-Url: https://codereview.webrtc.org/2334593002 Cr-Commit-Position: refs/heads/master@{#14178} [modify] https://crrev.com/11d57668190edb1f4b8208500da3ab33f638dee3/webrtc/build/webrtc.gni
,
Sep 13 2016
Issue 646346 has been merged into this issue. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by hlundin@chromium.org
, Aug 29 2016