Issue metadata
Sign in to add a comment
|
16 KB regression in resource_sizes (MonochromePublic.apk) at 555281:555281 |
||||||||||||||||||||
Issue descriptionCaused by "WebView: Read and write variations seeds" Commit: 5204eb18ab845ca715a8c256adea21463166c7f5 Link to size graph: https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICQ7tb2oQsM It's not clear whether or not this increase was expected. Please have a look and either: Close as “Won't Fix” with a short justification, or Land a revert / fix-up.
,
May 3 2018
Assigning to paulmiller@google.com because this is the only CL in range: WebView: Read and write variations seeds Chrome stores the seed in preferences, but WebView doesn't persist preferences. WebView must save save seeds separately. Serialize with protos, and add aw_variations_seed.proto to mirror the fields in VariationsSeedFetcher.SeedInfo. Since writing might not complete successfully, the file format must make truncation unambiguous. The proto wire format is such that truncation in the middle of a field will be detected, but truncation between fields is permitted. But by requiring in code that all SeedInfo fields are present, any truncated seed file will fail to load. BUG= 733857 Change-Id: I7f8d787afed83019c2d891aef419cdf7d593b71c Reviewed-on: https://chromium-review.googlesource.com/1033823 Commit-Queue: Paul Miller <paulmiller@chromium.org> Reviewed-by: Bo <boliu@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Cr-Commit-Position: refs/heads/master@{#555281}
,
May 3 2018
What is resource_sizes and how is it measured?
,
May 4 2018
Paul, I missed including this link in the template, apologies. https://chromium.googlesource.com/chromium/src/+/lkcr/docs/speed/apk_size_regressions.md#For-Binary-Size-Sheriffs It points out the tools that can be used to pick apart the delta incurred by the change. I clarified with estevenson@ that "resource sizes" is more the name of the script that detected the change, than the actual source of increase. I'll run the diagnose-bloat tool and paste output here.
,
May 4 2018
Tool output:
******************************Resource Sizes Diff******************************
MonochromePublic.apk_Breakdown (+4,095 bytes)
-158 bytes Zip Overhead
-1 bytes Package metadata size
+4,254 bytes Java code size
MonochromePublic.apk_Specifics
+16,215 bytes normalized apk size
+12,120 bytes main dex size
MonochromePublic.apk_Dex
+33 entries fields
+85 entries methods
+8 entries types
+20 entries strings
Eric's supplied more handy explanation, that could find its way into the diagnostic guide doc:
"The 16 kb number here comes from 12 kb (uncompressed dex size increase) + 4kb (compressed dex size increase). That's the way Java code changes affect the normalized_apk_size metric (the metric we use for size alerts).
Technically the dexfile gets optimized and takes up to 4x the uncompressed size on device. We tried alerting on that but basically every java cl fired an alert and they were never actionable, so we stopped that."
@estevenson, it's sounding like this CL simply added a chunk of Java code, and was close to the threshold for alerts anyway.
Is there anything Paul could actually do here?
,
May 4 2018
Don't think so. Generally we like to look at CLs that add protos (the biggest source of bloat in this CL) since they've caused bloat in the past but this looks good to me. Thanks for looking into it! |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, May 3 2018