Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(39)

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

Created:
3 years, 1 month ago by chcunningham
Modified:
3 years, 1 month 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
3 years, 1 month 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
3 years, 1 month 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 ...
3 years, 1 month ago (2015-06-04 22:01:33 UTC) #6
Nicolas Zea
lgtm
3 years, 1 month 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
3 years, 1 month ago (2015-06-04 22:12:29 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
3 years, 1 month 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}
3 years, 1 month 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. ...
3 years, 1 month 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) ...
3 years, 1 month 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 ...
3 years, 1 month 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
3 years, 1 month 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 ...
3 years, 1 month ago (2015-06-09 03:15:50 UTC) #18
commit-bot: I haz the power
Committed patchset #2 (id:20001)
3 years, 1 month 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}
3 years, 1 month 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
3 years, 1 month 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 ...
3 years, 1 month ago (2015-06-09 07:58:32 UTC) #25
commit-bot: I haz the power
Committed patchset #3 (id:40001)
3 years, 1 month 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}
3 years, 1 month 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 ...
3 years, 1 month 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 ...
3 years, 1 month ago (2015-06-09 16:08:22 UTC) #29
chcunningham
3 years, 1 month 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