Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2307283002:
GN Templates: Use the optimize_max compiler config. (Closed)
|
Created:
4 years, 3 months ago by ehmaldonado_webrtc Modified:
4 years, 3 months ago CC:
webrtc-reviews_webrtc.org, yujie_mao (webrtc), zhuangzesen_agora.io, tlegrand-webrtc, qiang.lu, peah-webrtc, bjornv1, video-team_agora.io, tterriberry_mozilla.com, fengyue_agora.io, sdk-team_agora.io, minyue-webrtc, mflodman, Andrew MacDonald, zhengzhonghou_agora.io, stefan-webrtc, kwiberg-webrtc, danilchap, henrika_webrtc, audio-team_agora.io, hlundin-webrtc, niklas.enbom, the sun, perkj_webrtc, aluebs-webrtc Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionGN 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
Committed: https://crrev.com/59af8b77145b8ff33a610128d339f02f950d3e7a
Cr-Commit-Position: refs/heads/master@{#14067}
Patch Set 1 : Rebase. #
Messages
Total messages: 18 (8 generated)
Patchset #2 (id:20001) has been deleted
Patchset #1 (id:1) has been deleted
Description was changed from ========== 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 ========== to ========== 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 ==========
ehmaldonado@webrtc.org changed reviewers: + kjellander@webrtc.org
Maybe this is a good moment to commit one of the main reasons we introduced templates :) Tested locally, waiting for the bots.
lgtm
The CQ bit was checked by ehmaldonado@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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 Committed: https://crrev.com/59af8b77145b8ff33a610128d339f02f950d3e7a Cr-Commit-Position: refs/heads/master@{#14067} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/59af8b77145b8ff33a610128d339f02f950d3e7a Cr-Commit-Position: refs/heads/master@{#14067}
Message was sent while issue was closed.
thakis@chromium.org changed reviewers: + thakis@chromium.org
Message was sent while issue was closed.
Apparently this increased 64-bit binary size of chrome significantly: https://bugs.chromium.org/p/chromium/issues/detail?id=457078#c76 Was that expected? Did you get a perf win from this? Is it worth it?
Message was sent while issue was closed.
On 2016/09/06 18:14:30, Nico (away until Tuesday) wrote: > Apparently this increased 64-bit binary size of chrome significantly: > https://bugs.chromium.org/p/chromium/issues/detail?id=457078#c76 > > Was that expected? Did you get a perf win from this? Is it worth it? As you can see in the referenced bug we had a 80% regression for NetEq, which is an important part of WebRTC. Although we don't want to break Chrome so we can probably either revert this again or apply the optimization more locally (and mark the regression as expected). However, as we understood it (when comparing GYP and GN compiler flags) it seems like the default optimization changed between GYP and GN - is that not the case?
Message was sent while issue was closed.
On 2016/09/06 20:07:31, kjellander_webrtc wrote: > On 2016/09/06 18:14:30, Nico (away until Tuesday) wrote: > > Apparently this increased 64-bit binary size of chrome significantly: > > https://bugs.chromium.org/p/chromium/issues/detail?id=457078#c76 > > > > Was that expected? Did you get a perf win from this? Is it worth it? > > As you can see in the referenced bug we had a 80% regression for NetEq, which is > an important part of WebRTC. Although we don't want to break Chrome so we can > probably either revert this again or apply the optimization more locally (and > mark the regression as expected). > However, as we understood it (when comparing GYP and GN compiler flags) it seems > like the default optimization changed between GYP and GN - is that not the case? It is true that many flags changed, but the final binary size and perf was roughly the same as far as I understand. Does NetEq test a code path that's hot in chrome as well?
Message was sent while issue was closed.
On 2016/09/06 21:13:13, Nico wrote: > On 2016/09/06 20:07:31, kjellander_webrtc wrote: > > On 2016/09/06 18:14:30, Nico (away until Tuesday) wrote: > > > Apparently this increased 64-bit binary size of chrome significantly: > > > https://bugs.chromium.org/p/chromium/issues/detail?id=457078#c76 > > > > > > Was that expected? Did you get a perf win from this? Is it worth it? > > > > As you can see in the referenced bug we had a 80% regression for NetEq, which > is > > an important part of WebRTC. Although we don't want to break Chrome so we can > > probably either revert this again or apply the optimization more locally (and > > mark the regression as expected). > > However, as we understood it (when comparing GYP and GN compiler flags) it > seems > > like the default optimization changed between GYP and GN - is that not the > case? > > It is true that many flags changed, but the final binary size and perf was > roughly the same as far as I understand. > > Does NetEq test a code path that's hot in chrome as well? I cross-posted your question in https://bugs.chromium.org/p/chromium/issues/detail?id=641966, let's keep the discussion there.
Message was sent while issue was closed.
On 2016/09/07 06:10:49, kjellander_webrtc wrote: > On 2016/09/06 21:13:13, Nico wrote: > > On 2016/09/06 20:07:31, kjellander_webrtc wrote: > > > On 2016/09/06 18:14:30, Nico (away until Tuesday) wrote: > > > > Apparently this increased 64-bit binary size of chrome significantly: > > > > https://bugs.chromium.org/p/chromium/issues/detail?id=457078#c76 > > > > > > > > Was that expected? Did you get a perf win from this? Is it worth it? > > > > > > As you can see in the referenced bug we had a 80% regression for NetEq, > which > > is > > > an important part of WebRTC. Although we don't want to break Chrome so we > can > > > probably either revert this again or apply the optimization more locally > (and > > > mark the regression as expected). > > > However, as we understood it (when comparing GYP and GN compiler flags) it > > seems > > > like the default optimization changed between GYP and GN - is that not the > > case? > > > > It is true that many flags changed, but the final binary size and perf was > > roughly the same as far as I understand. > > > > Does NetEq test a code path that's hot in chrome as well? > > I cross-posted your question in > https://bugs.chromium.org/p/chromium/issues/detail?id=641966, let's keep the > discussion there. I'm reverting back to the default behavior in https://codereview.webrtc.org/2334593002 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
