Issue metadata
Sign in to add a comment
|
0.2% regression in sizes at 469566:469566 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
May 5 2017
,
May 8 2017
Hi agrieve, sorry to trouble you again. I downloaded the build files from the perf bot before and after my patch. I then ran `supersize diff` and found that there's indeed a 200KB increase in the .text section. However, the diff file is so large that it's difficult to actually find what the cause of error is. Do you know how I can narrow down the cause of the issue? I can't understand how my patch could cause such a big increase. I've attached the relevant files.
,
May 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a0fc01a3bcb06cac6e10e6eaf2930b1a0fdb73cc commit a0fc01a3bcb06cac6e10e6eaf2930b1a0fdb73cc Author: shend <shend@chromium.org> Date: Mon May 08 07:11:50 2017 Move nested classes in ComputedStyleBase outside. Currently, groups such as StyleSurroundData are stored as nested classes in ComputedStyleBase. It is suspected that this is causing an increase in binary size. In an atttempt to restore the binary size to its original value, we move the nested classes outside of ComputedStyleBase. If this succeeds, we will put these classes in a "detail" namespace to discourage their use externally. Diff of generated files: https://gist.github.com/darrnshn/7d475a02af6f5d57b4a1622721111cba/revisions BUG= 718888 Review-Url: https://codereview.chromium.org/2869613002 Cr-Commit-Position: refs/heads/master@{#469911} [modify] https://crrev.com/a0fc01a3bcb06cac6e10e6eaf2930b1a0fdb73cc/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
,
May 8 2017
Looks like #4 didn't help at all, but your next one completely fixed it: "Generate StyleVisualData in ComputedStyleBase." - 60bc5e0281871949b67b8d7d3a2f0bc3db72f98c https://chromeperf.appspot.com/report?sid=cafe352ff6c97c6e9e7e3c33635cf61c3f7b5dbc15fad921c88d1c9e9c0a87e0&rev=469939 As for looking at the supersize diff, I've taken some examples from your diff and noted them on bug 717550 . There's certainly more that we could do to reduce noise in the linux diffs through more name normalization and clustering, but as of now I don't have any advice for getting more out of the diff :/
,
May 8 2017
Thanks agrieve, yeah I'm surprised that just landing more patches fixes the issue :/ I suspect it's some crazy LTO thing happening (which may explain the noisy diff?). Closing as it is now fixed.
,
May 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/73b8f7ed8d1105af3d39563f30a2771aee24923f commit 73b8f7ed8d1105af3d39563f30a2771aee24923f Author: shend <shend@chromium.org> Date: Mon May 08 23:59:59 2017 Revert of Move nested classes in ComputedStyleBase outside. (patchset #1 id:1 of https://codereview.chromium.org/2869613002/ ) Reason for revert: Did not decrease binary size. Original issue's description: > Move nested classes in ComputedStyleBase outside. > > Currently, groups such as StyleSurroundData are stored as nested classes > in ComputedStyleBase. It is suspected that this is causing an increase > in binary size. In an atttempt to restore the binary size to its > original value, we move the nested classes outside of ComputedStyleBase. > If this succeeds, we will put these classes in a "detail" namespace to > discourage their use externally. > > Diff of generated files: > https://gist.github.com/darrnshn/7d475a02af6f5d57b4a1622721111cba/revisions > > BUG= 718888 > > Review-Url: https://codereview.chromium.org/2869613002 > Cr-Commit-Position: refs/heads/master@{#469911} > Committed: https://chromium.googlesource.com/chromium/src/+/a0fc01a3bcb06cac6e10e6eaf2930b1a0fdb73cc TBR=nainar@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 718888 Review-Url: https://codereview.chromium.org/2871793002 Cr-Commit-Position: refs/heads/master@{#470150} [modify] https://crrev.com/73b8f7ed8d1105af3d39563f30a2771aee24923f/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
,
May 9 2017
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by kraynov@chromium.org
, May 5 2017