New issue
Advanced search Search tips

Issue 880293 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

A zero-to-nonzero to 20.1% regression in system_health.memory_desktop at 587769:588034

Project Member Reported by maxlg@chromium.org, Sep 4

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=880293

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


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

Android Nexus5 Perf
Win 7 Nvidia GPU Perf
Win 7 Perf
android-nexus5x-perf
mac-10_12_laptop_low_end-perf
mac-10_13_laptop_high_end-perf

system_health.memory_mobile - Benchmark documentation link:
  https://bit.ly/system-health-benchmarks

system_health.memory_desktop - Benchmark documentation link:
  https://bit.ly/system-health-benchmarks

memory.top_10_mobile - Benchmark documentation link:
  None
Cc: hajimehoshi@chromium.org
Owner: hajimehoshi@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/10b56183640000

Remove RenderThreadImpl::IdleHandler by hajimehoshi@chromium.org
https://chromium.googlesource.com/chromium/src/+/99c1351b89e5593e16f8005f4128808ee89a83e9
0 → 6.08e+05 (+6.08e+05)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  https://bit.ly/system-health-benchmarks
Cc: sullivan@chromium.org
 Issue 881405  has been merged into this issue.
Issue 881406 has been merged into this issue.
Cc: keishi@chromium.org haraken@chromium.org tasak@google.com
The observable changes by my CL: https://chromeperf.appspot.com/group_report?rev=587951

My CL disables idle handling, which deletes allocations that can be always deleted like cache.

For memory usage change at 'browse_social_tumblr_infinite_scroll' in at most 100-MB-ish (e.g. https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICQ29SNggsM), it looks like the similar bump happened before and disappeared. I guess the test is a little flaky my CL changed the lifetime of the cache and triggered this potential memory usage.

For 'multitab_misc/multitab_misc_typical24' in 5-MB-ish change (e.g. 'https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICQq5GKkwgM') I think this should be the same thing (there were similar bumps before)

Other changes are under 1MB on Android.

Thus, I'd like to keep my CL committed.
Cc: eirage@chromium.org
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/16f6daad640000

Enable use-zoom-for-dsf on Android on M70 by eirage@chromium.org
https://chromium.googlesource.com/chromium/src/+/d0b8bbd2dbf9c06dcbbd01a4b1285cf7a1bc2ba0
1.184e+08 → 6.376e+06 (-1.12e+08)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  https://bit.ly/system-health-benchmarks
I think, this is not a regression. This shows the change of memory management policy.
When running browse_social_tumblr_infinite_scroll, IdleHandler (probably) ran multiple times. The handler cleared cc cache... so memory used for invisible images was freed.
I think, cc caches have some upper limit. We have and will improve MemoryPressureMonitor. So it is ok to keep such images if we get no memory pressure signals.

Issue 880288 has been merged into this issue.
Issue 881525 has been merged into this issue.
Cc: npm@chromium.org
 Issue 884301  has been merged into this issue.
 Issue 884325  has been merged into this issue.
Just to double-check, re comment #9: The extra memory, which is expected to be freed under memory pressure, is not released when forcing GCs?

Is there something that could be done on benchmarks to force-free that memory?
+tasak?

The benchmark: LongRunningGmailDesktopBackgroundStory's code is:

class _LongRunningStory(system_health_story.SystemHealthStory):
...
  def RunPageInteractions(self, action_runner):
    super(_LongRunningStory, self).RunPageInteractions(action_runner)
    if self.BACKGROUND:
      action_runner.tab.browser.tabs.New()
    if self._take_memory_measurement:
      action_runner.MeasureMemory()
    for _ in xrange(STEPS):
      action_runner.Wait(SAMPLING_INTERVAL_IN_SECONDS)
      if self._take_memory_measurement:
        action_runner.MeasureMemory()

action_runner.MeasureMemory() records memory usage, but its determinstic_mode is False.

 def MeasureMemory(self, deterministic_mode=False):
    if not self.tab.browser.platform.tracing_controller.is_tracing_running:
      logging.warning('Tracing is off. No memory dumps are being recorded.')
      return None
    if deterministic_mode:
      self.Wait(_MEMORY_DUMP_WAIT_TIME)
      self.ForceGarbageCollection()
    dump_id = self.tab.browser.DumpMemory()
    if not dump_id:
      raise exceptions.StoryActionError('Unable to obtain memory dump')
    return dump_id

So no memory pressure signals are sent. Measured memory usage contains free-list memory usage, v8 unused objects usage, ... and so on.
I'm not sure, but probably it is not good to use MeasureMemory(deterministic_mode=True) for long_running_stories.

Just talking about background tabs, we have already had "background page freezing" feature and "purge before background page freezing"(this will be done 5 minutes after pages are hidden).
If we are not able to freeze pages, memory pressure signals will be sent. V8 GC, BlinkGC, and so on will run.
I think, background tab's memory is correctly managed.

Sign in to add a comment