New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 711040 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

8% Browser java heap regression in range 461545 : 461667

Project Member Reported by ssid@chromium.org, Apr 12 2017

Issue description

Comment 1 by ssid@chromium.org, Apr 12 2017

suspected CLs:
Add an experimental search widget

Adds an experimental search widget to use the omnibox from Android
Launcher.

Review-Url: https://codereview.chromium.org/2777283005
Cr-Commit-Position: refs/heads/master@{#461575}


and


Simplify feature engagement tracker API.

Instead of splitting out preconditions, trigger-events and whether
a feature has been used, they are now all just events. This
simplifies the API surface quite a bit.

Whether a feature has been used will now happen by marking a
particular event as the one that signifies that a feature has been
used. This will be configured per in-product help feature.

The check for whether the in-product help should be trigger is
still a similar method that returns a boolean.

BUG= 706309 

Change-Id: I309f95ebc417d078e9e671b271b2d50e7e8f0fb2
Reviewed-on: https://chromium-review.googlesource.com/466586
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#461587}

Comment 3 by ssid@chromium.org, Apr 12 2017

Blocking: 710909
Labels: Performance-Memory

Comment 4 by yus...@chromium.org, Apr 12 2017

My CL is local builds only. It doesn't run on any public build.
Project Member

Comment 5 by 42576172...@developer.gserviceaccount.com, Apr 12 2017


=== BISECT JOB RESULTS ===
NO Perf regression found

Bisect Details
  Configuration: android_nexus5X_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:chrome:browser_process:reported_by_chrome:java_heap:effective_size_avg/blank_about/blank_about_blank

Revision             Result                    N
chromium@461544      22247700 +- 11704147      21      good
chromium@461667      23547401 +- 12325979      21      bad

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=blank.about.blank system_health.memory_mobile

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8982483371459909952

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5909928155283456


| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection.  Thank you!

Comment 7 by ssid@chromium.org, Apr 12 2017

Blocking: -710909
Labels: OS-Android
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Apr 13 2017

Cc: rlanday@chromium.org
Owner: rlanday@chromium.org

=== Auto-CCing suspected CL author rlanday@chromium.org ===

Hi rlanday@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : rlanday
  Commit : 8054305d4a8aa57e06ef3e5012bf985305bc364d
  Date   : Mon Apr 03 21:27:18 2017
  Subject: Remove unnecessary include from SVGInlineTextBoxPainter.cpp

Bisect Details
  Configuration: android_nexus5X_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:chrome:browser_process:reported_by_os:system_memory:private_dirty_size_avg/load_search/load_search_amazon
  Change       : 1.47% | 32968232.0 -> 33453608.0

Revision             Result                  N
chromium@461544      32968232 +- 425265      6      good
chromium@461545      35775357 +- 204424      6      bad       <--
chromium@461546      35464744 +- 370954      6      bad
chromium@461548      35373949 +- 464256      6      bad
chromium@461552      35123411 +- 618717      6      bad
chromium@461560      34814845 +- 556745      6      bad
chromium@461575      34517885 +- 734622      6      bad
chromium@461606      33975848 +- 775449      6      bad
chromium@461667      33453608 +- 476722      6      bad

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=load.search.amazon system_health.memory_mobile

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8982476088343328192

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5209738414915584


| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection.  Thank you!

Comment 9 by ssid@chromium.org, Apr 13 2017

Owner: ----
That doesn't make much sense.
yusufo@ The bisect does not seem to be working correctly. Can you locally try to find how much extra memory the contextual search uses?
Cc: -yus...@chromium.org
Owner: yus...@chromium.org
Status: Assigned (was: Untriaged)
Sid means search widget, rather than contextual search.

The easiest way is probably to take a Java heap dump (in ddms there's a button to grab a heap dump from a process) and then use go/ahat to view it and look for the new objects we are initializing.
In addition to moving stuff out of ChromeBrowserInitializer, we could maybe also avoid manually loading the TemplateUrlService if it isn't loaded.  It'll likely be loaded somewhere further down the line.
Comparing heap dumps with and without the widget initialization and they are very very close. So I am gonna say this is most likely not us.
FOr reference the numbers I am getting with a release build (not sure that matters) on Nexus 5 for about:blank is:

With widget initialization
app	image	zygote	Total
5,456,036	6,023,099	11,626,556	23,105,691


Without widget initialization
app	image	zygote	Total
5,454,330	6,023,099	11,626,556	23,103,985


Please let me know if anything about these look weird/off and I can retry.
Components: Speed>Metrics>SystemHealthRegressions
Components: -Speed>Metrics>SystemHealthRegressions

Sign in to add a comment