Pak file entries in supersize causing some noise |
||||
Issue descriptionLooking at supersize output for 766fbc88bed0c117536f00aab036aef7851a3c03, there are a few issues: 1) IDS_DEFAULT_AVATAR_* are showing as changed, when they haven't. 2) APK zip overhead and ELF file overhead show up prominently, but they are not actionable. We'll need to investigate & fix why adding a pak symbol is causing a bunch of other entries to shift around. Maybe an issue with .pak.info? For 2), I think we should make the "overhead" symbols be "padding-only symbols", by setting sym.padding = sym.size. 17 symbols added (+), 1169 changed (~), 0 removed (-), 660396 unchanged (not shown) Of changed symbols, 1158 grew, 28 shrank Number of unique symbols 497775 -> 497790 (+15) 0 paths added, 0 removed, 161 changed Showing 1,186 symbols (1,169 -> 1,186 unique) with total pss: 65535 bytes Histogram of symbols based on PSS: (-2048,-1024]: 1 (-128,-64]: 2 (-8,-4]: 4 [8,16): 175 [128,256): 4 [32768,65536): 1 (-1024,-512]: 1 (-64,-32]: 6 [1,2): 152 [16,32): 49 [256,512): 5 (-512,-256]: 3 (-32,-16]: 3 [2,4): 420 [32,64): 13 [512,1024): 1 (-256,-128]: 3 (-16,-8]: 5 [4,8): 329 [64,128): 8 [4096,8192): 1 .text=472 bytes .rodata=448 bytes .data.rel.ro=0 bytes .data=0 bytes .bss=48 bytes .pak.translations=57.5kb .pak.nontranslated=7.86kb .other=-2.29kb total=64.0kb Number of unique paths: 165 Section Legend: t=.text, r=.rodata, R=.data.rel.ro, d=.data, b=.bss, p=.pak.translations, P=.pak.nontranslated, o=.other Index | Running Total | Section@Address | Δ PSS (Δ size_without_padding) | Path ------------------------------------------------------------ + 0) 52400 (79.9%) p@0x3e07 52400 (0->52400) chrome/browser/ui/webui/about_ui.cc ../../components/security_interstitials_strings.grdp: IDS_CONNECTION_HELP_HTML + 1) 59843 (91.3%) P@0x3f3c 7443 (0->7443) chrome/browser/ui/webui/about_ui.cc ../../components/resources/security_interstitials_resources.grdp: IDR_SECURITY_INTERSTITIAL_COMMON_CSS ~ 2) 58448 (89.2%) o@0x0 -1395 (421304->419909) {{no path}} APK zip overhead ~ 3) 57527 (87.8%) o@0x0 -921 (7178->6257) {{no path}} ELF file overhead + 4) 58131 (88.7%) P@0x3f3d 604 (0->604) chrome/browser/ui/webui/about_ui.cc ../../components/resources/security_interstitials_resources.grdp: IDR_SECURITY_INTERSTITIAL_CORE_CSS ~ 5) 58537 (89.3%) p@0x1618 406 (222->628) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_1 ~ 6) 58905 (89.9%) p@0x161e 368 (174->542) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_7 ~ 7) 58541 (89.3%) p@0x1620 -364 (539->175) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_9 ~ 8) 58216 (88.8%) p@0x161f -325 (539->214) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_8 + 9) 58531 (89.3%) p@0x3e06 315 (0->315) chrome/browser/ui/webui/about_ui.cc ../../components/security_interstitials_strings.grdp: IDS_CONNECTION_HELP_TITLE ~ 10) 58843 (89.8%) p@0x161c 312 (246->558) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_5 ~ 11) 58536 (89.3%) p@0x1622 -307 (554->247) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_11 ~ 12) 58832 (89.8%) t@0x25e2988 296 (836->1132) chrome/browser/ui/webui/about_ui.cc AboutUIHTMLSource::StartDataRequest ~ 13) 59066 (90.1%) p@0x1625 234 (109->343) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_14 ~ 14) 58833 (89.8%) p@0x162b -233 (456->223) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_20 ~ 15) 59028 (90.0%) p@0x1630 195 (158->353) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_25 + 16) 59188 (90.3%) r@0x5124ef 160 (0->160) chrome/browser/flag_descriptions.cc flag_descriptions::kBundledConnectionHelpDescription ~ 17) 59031 (90.1%) p@0x1623 -157 (267->110) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_12 ~ 18) 58878 (89.8%) p@0x1629 -153 (346->193) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_18 ~ 19) 59023 (90.0%) p@0x3e3b 145 (19382->19527) components/ssl_errors/error_info.cc ../../components/ssl_errors_strings.grdp: IDS_CERT_ERROR_EXPIRED_DETAILS ~ 20) 59146 (90.2%) p@0x162e 123 (225->348) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_23 ~ 21) 59026 (90.0%) p@0x162d -120 (279->159) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_22 ~ 22) 59143 (90.2%) p@0x1626 117 (341->458) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_15 ~ 23) 59241 (90.4%) p@0x162f 98 (170->268) chrome/browser/profiles/profile_avatar_icon_util.cc ../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_24 + 24) 59328 (90.5%) r@0x0 87 (0->0) {{no path}} ** aggregate padding of diff'ed symbols + 25) 59415 (90.6%) r@0x264cab 87 (0->87) chrome/browser/ui/webui/about_ui.cc string literal ~ 26) 59501 (90.8%) p@0x3e3d 86 (11673->11759) components/ssl_errors/error_info.cc ../../components/ssl_errors_strings.grdp: IDS_CERT_ERROR_NOT_YET_VALID_DETAILS
,
Feb 21 2018
Native symbol addresses change all the time though. I think there's some other issue here going on... Maybe ID mappings became off from .info files being stale? (although IDS_CONNECTION_HELP_HTML looks correct)
,
Feb 21 2018
I think I found the culprit, it's that the actual sizes did change for all those 1000+ symbols, will see if I can change the way pak symbols are allocated to make this not happen in the future:
original:
660428) 62214793 (97.9%) p@0x1617 526 chrome/browser/profiles/profile_avatar_icon_util.cc
../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_0
after commit:
660439) 62222386 (97.8%) p@0x1617 530 chrome/browser/profiles/profile_avatar_icon_util.cc
../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_0
diff:
~ 495) 63958 (97.6%) p@0x1617 +4 (526->530) chrome/browser/profiles/profile_avatar_icon_util.cc
../../chrome/app/generated_resources.grd: IDS_DEFAULT_AVATAR_LABEL_0
,
Feb 21 2018
Basically, since each symbol is tracking approximate compressed sizes, they will change depending on how new data affects compression, which is unpredictable. We have a couple of options, here are two: 1) Keep code as is, and just accept that sizes will change due to compression artifacts. 2) Track uncompressed sizes instead. I'm leaning towards 2) even though it means our sizes won't match exactly. :(
,
Feb 22 2018
How about a 2b) Track len(zlib.compress(value)) for each entry. That way it'll still approximate compression (to an extent), but entries won't affect each other?
,
Feb 22 2018
Sounds good, I like that better than either 1) or 2). Will test it out.
,
Feb 22 2018
Here's the compression ratio per language: assets/am.pak: 0.291449933122 assets/ar.pak: 0.289466347606 assets/bg.pak: 0.273507759009 assets/ca.pak: 0.359096697239 assets/cs.pak: 0.36639394875 assets/da.pak: 0.371431043433 assets/de.pak: 0.361756609692 assets/el.pak: 0.262354810171 assets/en-GB.pak: 0.37779564951 assets/en-US.pak: 0.372087332957 assets/es-419.pak: 0.364451976275 assets/es.pak: 0.353262265605 assets/fa.pak: 0.288705195233 assets/fi.pak: 0.377025928424 assets/fil.pak: 0.344916143378 assets/fr.pak: 0.34708358023 assets/he.pak: 0.315037389531 assets/hi.pak: 0.222769768886 assets/hr.pak: 0.372131472786 assets/hu.pak: 0.367055283941 assets/id.pak: 0.367465765733 assets/it.pak: 0.354582451571 assets/ja.pak: 0.307290755715 assets/ko.pak: 0.341059813799 assets/lt.pak: 0.358684194208 assets/lv.pak: 0.355124465639 assets/nb.pak: 0.37509423377 assets/nl.pak: 0.365501236576 assets/pl.pak: 0.370858391692 assets/pt-BR.pak: 0.362012316938 assets/pt-PT.pak: 0.35929474516 assets/ro.pak: 0.358706404161 assets/ru.pak: 0.274451539483 assets/sk.pak: 0.369331052656 assets/sl.pak: 0.366719669524 assets/sr.pak: 0.279164349656 assets/sv.pak: 0.375048355899 assets/sw.pak: 0.371124889283 assets/th.pak: 0.219054596135 assets/tr.pak: 0.366778354833 assets/uk.pak: 0.270691719148 assets/vi.pak: 0.315990487858 assets/zh-CN.pak: 0.412930718652 assets/zh-TW.pak: 0.40857461882
,
Feb 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6d411ef0a77d65ece6166fd5c411983a8548f467 commit 6d411ef0a77d65ece6166fd5c411983a8548f467 Author: Peter Wen <wnwen@chromium.org> Date: Mon Feb 26 20:23:11 2018 Supersize: Fix pak spurious deltas Previously we recorded the exact pak compression ratios in .size files. This results in minor changes to 1k+ unrelated symbols due to changes in compression. Use a mostly accurate static constant instead to make .size files comparable across runs. Bug: 814078 Change-Id: I63c78379898ef02885ae86e0b95df95d51965918 Reviewed-on: https://chromium-review.googlesource.com/938122 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Peter Wen <wnwen@chromium.org> Cr-Commit-Position: refs/heads/master@{#539252} [modify] https://crrev.com/6d411ef0a77d65ece6166fd5c411983a8548f467/tools/binary_size/libsupersize/archive.py
,
Feb 26 2018
,
Feb 26 2018
Forgot about the padding part of this bug.
,
Mar 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9b70a4435f46b08b4e5b19a0793a87a99f28045d commit 9b70a4435f46b08b4e5b19a0793a87a99f28045d Author: Peter Wen <wnwen@chromium.org> Date: Thu Mar 01 01:26:14 2018 Revert "Supersize: Fix pak spurious deltas" This reverts commit 6d411ef0a77d65ece6166fd5c411983a8548f467. Reason for revert: Failed perf builders with division by zero Original change's description: > Supersize: Fix pak spurious deltas > > Previously we recorded the exact pak compression ratios in .size files. > This results in minor changes to 1k+ unrelated symbols due to changes in > compression. Use a mostly accurate static constant instead to make .size > files comparable across runs. > > Bug: 814078 > Change-Id: I63c78379898ef02885ae86e0b95df95d51965918 > Reviewed-on: https://chromium-review.googlesource.com/938122 > Reviewed-by: agrieve <agrieve@chromium.org> > Commit-Queue: Peter Wen <wnwen@chromium.org> > Cr-Commit-Position: refs/heads/master@{#539252} TBR=wnwen@chromium.org,agrieve@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 814078 , 817534 Change-Id: I5cd4a6375e1f318c3559801da95a850f5ffcf4fa Reviewed-on: https://chromium-review.googlesource.com/942061 Reviewed-by: Peter Wen <wnwen@chromium.org> Commit-Queue: Peter Wen <wnwen@chromium.org> Cr-Commit-Position: refs/heads/master@{#539971} [modify] https://crrev.com/9b70a4435f46b08b4e5b19a0793a87a99f28045d/tools/binary_size/libsupersize/archive.py
,
Mar 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d6e318dcf7f158e1f019462aa26151a25869ac93 commit d6e318dcf7f158e1f019462aa26151a25869ac93 Author: Peter Wen <wnwen@chromium.org> Date: Thu Mar 01 15:27:57 2018 Reland "Supersize: Fix pak spurious deltas" Original CL: https://crrev.com/c/938122 Fix: - Division by zero when there is no uncompressed pak files. TBR=agrieve@chromium.org Bug: 814078 , 817534 Change-Id: Id3198a84dce95074d13ca20afe4caec45631b0cc Reviewed-on: https://chromium-review.googlesource.com/943221 Reviewed-by: Peter Wen <wnwen@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Peter Wen <wnwen@chromium.org> Cr-Commit-Position: refs/heads/master@{#540161} [modify] https://crrev.com/d6e318dcf7f158e1f019462aa26151a25869ac93/tools/binary_size/libsupersize/archive.py
,
Mar 1 2018
,
Mar 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/780a9e000760b3f484201153d50ba73827392844 commit 780a9e000760b3f484201153d50ba73827392844 Author: Peter Wen <wnwen@chromium.org> Date: Thu Mar 01 15:38:43 2018 Supersize: Switch overhead symbols to padding-only Overhead should be tracked but these symbols are not actionable. Making them padding-only removes them from size diffs. Bug: 814078 Change-Id: I68991f54d4ee990c7bd675c4dd0f22e5337eb9f4 Reviewed-on: https://chromium-review.googlesource.com/941587 Commit-Queue: Peter Wen <wnwen@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Cr-Commit-Position: refs/heads/master@{#540164} [modify] https://crrev.com/780a9e000760b3f484201153d50ba73827392844/tools/binary_size/libsupersize/archive.py [modify] https://crrev.com/780a9e000760b3f484201153d50ba73827392844/tools/binary_size/libsupersize/testdata/Archive_Elf.golden [modify] https://crrev.com/780a9e000760b3f484201153d50ba73827392844/tools/binary_size/libsupersize/testdata/Archive_Pak.golden [modify] https://crrev.com/780a9e000760b3f484201153d50ba73827392844/tools/binary_size/libsupersize/testdata/Console.golden [modify] https://crrev.com/780a9e000760b3f484201153d50ba73827392844/tools/binary_size/libsupersize/testdata/Csv.golden [modify] https://crrev.com/780a9e000760b3f484201153d50ba73827392844/tools/binary_size/libsupersize/testdata/FullDescription.golden [modify] https://crrev.com/780a9e000760b3f484201153d50ba73827392844/tools/binary_size/libsupersize/testdata/SymbolGroupMethods.golden |
||||
►
Sign in to add a comment |
||||
Comment 1 by wnwen@chromium.org
, Feb 21 2018