New issue
Advanced search Search tips

Issue 764920 link

Starred by 1 user
Status: Assigned
Owner:
chiniforooshan@chromium.org
Cc:
estevenson@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

20 KB regression in resource_sizes (MonochromePublic.apk) at 501668:501668

Project Member Reported by estevenson@chromium.org, Sep 13 2017 
Caused by "move memory_infra and tracing to the GRC service"

Commit 60345b63eb89379e689347ff01d7c8b52cfaa18a

Link to size graph: https://chromeperf.appspot.com/report?sid=a097e74b1aa288511afb4cb616efe0f95ba4d347ad61d5e835072f23450938ba&num_points=10&rev=501668

Debugging size regressions is documented at: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/apk_size_regressions.md#Debugging-Apk-Size-Increase

It's not clear to me whether or not this increase was expected.Please have a look and either:

  1. Close as “Won't Fix” with a short justification, or
  2. Land a revert / fix-up.
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Sep 13 2017 

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=764920

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


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

Android Builder

Comment 2 by estevenson@chromium.org, Sep 13 2017

Owner: chiniforooshan@chromium.org
Status: Assigned (was: Untriaged)
Growth was due solely to native code size. Didn't see anything obvious in the diff (attached) though.

Ehsan - can you take a look please?
diff_results.txt
147 KB View Download

Comment 3 by chiniforooshan@chromium.org, Sep 14 2017 

If I'm understanding the diff correctly, the main increase is because my CL adds the tracing coordinator. Now we are in a temporary intermediate state that we have the new tracing coordinator and the old tracing controller. I think the size will go down again when I land my next CL that deletes the old tracing controller (crrev/c/575716). estevenson@ Is it OK to wait until then?

Comment 4 by estevenson@chromium.org, Sep 14 2017 

Sounds good to me.

Let's leave this open for now, and could you reply back here once those CLs have landed? We likely won't see the entire regression reversed but I'm curious to see how much we get back (normally we just close these as "WontFix").

Thanks!

Comment 5 by estevenson@chromium.org, Nov 22 2017 

Ehsan, any chance you've submitted any other CLs that may have addressed this size regression? crrev/c/575716 affect the size of MonochromePublic.apk.

Comment 6 by estevenson@chromium.org, Nov 22 2017 

*did not affect

Comment 7 by agrieve@chromium.org, Jan 10 2018 

ping chiniforooshan

Comment 8 by chiniforooshan@chromium.org, Jan 11 2018 

Sorry for the late reply. I think it's because crrev/c/575716 did not completely delete the old tracing controller. I plan to clean up the tracing code a little bit more sometime this quarter. Hopefully that may have some effect; if not I'm not sure what else can be done. How bad is a 20 KB increase?

Comment 9 by agrieve@chromium.org, Jan 11 2018 

20kb is not that bad, but in the range of "should do something if something can be done". If nothing can be done, then we can close & move on.
►

Sign in to add a comment