Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(543)

Issue 1161633003: Update sizes expectations since static linking ffmpeg. (Closed)

Created:
5 years, 6 months ago by chcunningham
Modified:
5 years, 6 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Update perf expectations since static linking ffmpeg. This updates bss sizes for the following: - linux-release-64/sizes/chrome-bss/bss - linux-release/sizes/chrome-bss/bss - mac-release/sizes/ChromiumFramework/ChromiumFramework - xp-release/sizes/chrome_child.dll/chrome_child.dll It also updates linux-release chrome-textrel count to take on the textrelocations that were previously part of ffmpegsumo.so. Since the introduction of textrel expectations (https://codereview.chromium.org/9187031), ffmpegsumo has been known to have textrelocations that are not trivial to remove. The good news is, the number of relocations has been steadily falling as we roll from upstream ffmpeg. While chrome/chrome_child have grown significantly, the net change (now that ffmpegsumo.dll is gone) is actually a savings of a few hundred KB due to dead code elimination in static linking. Static linking change: https://codereview.chromium.org/1141703002/ Bugs motivating the original static linking change: BUG=435455, 429131, 441908 Committed: https://crrev.com/f37609eb69066f9531a4cce126e8225819217250 Cr-Commit-Position: refs/heads/master@{#333462}

Patch Set 1 #

Patch Set 2 : Choosing new range for re-landed ffmpeg change #

Patch Set 3 : Updating chrome textrel and broadening range. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -6 lines) Patch
M tools/perf_expectations/perf_expectations.json View 1 2 4 chunks +5 lines, -6 lines 0 comments Download

Messages

Total messages: 30 (9 generated)
chcunningham
5 years, 6 months ago (2015-06-04 21:53:30 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1161633003/1
5 years, 6 months ago (2015-06-04 22:01:30 UTC) #4
commit-bot: I haz the power
Dry run: No LGTM from a valid reviewer yet. Only full committers are accepted. Even ...
5 years, 6 months ago (2015-06-04 22:01:33 UTC) #6
Nicolas Zea
lgtm
5 years, 6 months ago (2015-06-04 22:10:12 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1161633003/1
5 years, 6 months ago (2015-06-04 22:12:29 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 6 months ago (2015-06-04 22:29:09 UTC) #10
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/41c1b31dc7983a1dca627f9e00ee3977b75e5b6f Cr-Commit-Position: refs/heads/master@{#332938}
5 years, 6 months ago (2015-06-04 22:30:08 UTC) #11
Lei Zhang
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/1157993006/ by thestig@chromium.org. ...
5 years, 6 months ago (2015-06-05 03:04:44 UTC) #12
chcunningham
On 2015/06/05 03:04:44, Lei Zhang wrote: > A revert of this CL (patchset #1 id:1) ...
5 years, 6 months ago (2015-06-05 03:32:26 UTC) #13
chcunningham
FYI, the related ffmpeg change is relanded. I'm redoing the sizes change now as build ...
5 years, 6 months ago (2015-06-09 02:32:49 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1161633003/20001
5 years, 6 months ago (2015-06-09 03:14:46 UTC) #17
chcunningham
Heads up. The new range chosen is pretty small - there just aren't a lot ...
5 years, 6 months ago (2015-06-09 03:15:50 UTC) #18
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 6 months ago (2015-06-09 03:25:56 UTC) #19
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/62cc1d7aefd10962b5b7ca4be579cf561e4d5f68 Cr-Commit-Position: refs/heads/master@{#333436}
5 years, 6 months ago (2015-06-09 03:26:51 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1161633003/40001
5 years, 6 months ago (2015-06-09 07:55:06 UTC) #23
chcunningham
All, Please think of this as TBR. I don't yet have committer status so I ...
5 years, 6 months ago (2015-06-09 07:58:32 UTC) #25
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 6 months ago (2015-06-09 08:05:06 UTC) #26
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/f37609eb69066f9531a4cce126e8225819217250 Cr-Commit-Position: refs/heads/master@{#333462}
5 years, 6 months ago (2015-06-09 08:06:19 UTC) #27
chcunningham
On 2015/06/09 08:06:19, commit-bot: I haz the power wrote: > Patchset 3 (id:??) landed as ...
5 years, 6 months ago (2015-06-09 08:29:27 UTC) #28
DaleCurtis
Probably we don't want to tie ffmpeg into the build tools like that. We should ...
5 years, 6 months ago (2015-06-09 16:08:22 UTC) #29
chcunningham
5 years, 6 months ago (2015-06-09 19:55:04 UTC) #30
Message was sent while issue was closed.
On 2015/06/09 16:08:22, DaleCurtis wrote:
> Probably we don't want to tie ffmpeg into the build tools like that.  We
should
> make sure to run the sizes tool ahead of time with the next roll so these can
> updated at the same time.  Can you add some steps to the ffmpeg roll
> documentation about that?

OK, I will update the doc. Also, I checked the perf dashboard cold start tests
for linux release and found that nothing was regressed.

Powered by Google App Engine
This is Rietveld 408576698