New issue
Advanced search Search tips

Issue 839508 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: May 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

16 KB regression in resource_sizes (MonochromePublic.apk) at 555281:555281

Project Member Reported by cjgrant@google.com, May 3 2018

Issue description

Caused 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.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=839508

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=1bd020b3a624251154513ac5c5dbd8f1f9a90ca3ff4d9af800f76922eb8a9d87


Bot(s) for this bug's original alert(s):

Android Builder Perf
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}
What is resource_sizes and how is it measured?
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.
Cc: estevenson@chromium.org
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?
Status: WontFix (was: Assigned)
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