New issue
Advanced search Search tips

Issue 771478 link

Starred by 4 users

Issue metadata

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



Sign in to add a comment

Add swap-thrashing signals to Chrome / Improve memory pressure signals

Project Member Reported by sebmarchand@chromium.org, Oct 4 2017

Issue description

Tracking bug for the "swaps-thrashing signals" effort.

High-level goal: Create a high-fidelity thrashing signal that can be used to drive user-visible performance optimizations.

Design doc (WIP, @google only): https://docs.google.com/a/google.com/document/d/1Y1_GwrPF_2XFTfw03zb3lXhbGmXwgY2sh01T55c_X2Y/edit?usp=sharing
 
Cc: semenzato@chromium.org sonnyrao@chromium.org
Cc: gyuyoung...@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38f2e322f9c148e9c5b6ddc3b294ebb2a7f8ab78

commit 38f2e322f9c148e9c5b6ddc3b294ebb2a7f8ab78
Author: Sebastien Marchand <sebmarchand@chromium.org>
Date: Wed Oct 25 16:36:32 2017

Implement a swap thrashing monitor on Windows

The goal of this swap-thrashing monitor is to detect when the system enters into
a high swap-thrashing state and will initially be used to determine the impact
that this can have on the core speed metrics. The systems interested in
observing these signals should query this monitor directly, there's no observer
mechanism in place.

The initial implementation will focus only on Windows and will later be
extended to other platforms.

See the high level comment at the top of
chrome/browser/memory/swap_thrashing_monitor_win.h" for a more detailed
explanation of the different states that the system expose and what can cause a
transition from one given state to the other.

Bug: 771478
Change-Id: I99dd478cd3942ea1e8d2818cc7a9ca8ec89db213
Reviewed-on: https://chromium-review.googlesource.com/727642
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Georges Khalil <georgesak@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511483}
[modify] https://crrev.com/38f2e322f9c148e9c5b6ddc3b294ebb2a7f8ab78/chrome/browser/BUILD.gn
[add] https://crrev.com/38f2e322f9c148e9c5b6ddc3b294ebb2a7f8ab78/chrome/browser/memory/swap_thrashing_monitor_win.cc
[add] https://crrev.com/38f2e322f9c148e9c5b6ddc3b294ebb2a7f8ab78/chrome/browser/memory/swap_thrashing_monitor_win.h
[add] https://crrev.com/38f2e322f9c148e9c5b6ddc3b294ebb2a7f8ab78/chrome/browser/memory/swap_thrashing_monitor_win_unittest.cc
[modify] https://crrev.com/38f2e322f9c148e9c5b6ddc3b294ebb2a7f8ab78/chrome/test/BUILD.gn

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9a0d07b6b5d884e95430ff9dc60519912717fcf

commit f9a0d07b6b5d884e95430ff9dc60519912717fcf
Author: Sebastien Marchand <sebmarchand@chromium.org>
Date: Sat Oct 28 11:34:04 2017

Improve and use the SwapThrashingMonitor for Chrome on Windows

This is a follow up on crrev.com/38f2e322f9c148e9c5b6ddc3b294ebb2a7f8ab78

This CL does several things*:
- It greatly simplify the logic in SwapThrashingMonitorDelegateWin: As
  suggested by wez@ the observation window now looks at how many samples
  are above a certain threshold rather than looking at the average
  page-fault rate over an interval of time. This average rate was really
  affected by the small burst in paging and this was causing a lot of
  unwanted switch to the suspected state. The delta on this CL isn't
  displayed properly, swap_thrashing_monitor_win.h has been renamed to
  swap_thrashing_monitor_delegate_win.h and the SampleWindow subclass
  now replace the PageFaultAverageRate one.
- It introduce the monitor that owns this delegate. The monitor is
  responsible of calling the delegate at regular intervals to sample the
  state of the system.
- It emits some new metrics to record how much time is spent in each
  state and how often the system transition from one step to another.

* Sorry for sending this a one big CL... The timezone difference will
really slow down the release of this if I split it into too many
smaller CLs (and there's only a few days left for me to work on this).

Bug: 771478
TBR: chrisha@chromium.org
Change-Id: I8aa5c8fd2c768a3e1ad0c387a67c7205db5da37a
Reviewed-on: https://chromium-review.googlesource.com/738072
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512398}
[modify] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/chrome/browser/BUILD.gn
[modify] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/chrome/browser/chrome_browser_main_win.cc
[add] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/chrome/browser/memory/swap_thrashing_monitor.cc
[add] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/chrome/browser/memory/swap_thrashing_monitor.h
[add] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/chrome/browser/memory/swap_thrashing_monitor_delegate.cc
[add] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/chrome/browser/memory/swap_thrashing_monitor_delegate.h
[add] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/chrome/browser/memory/swap_thrashing_monitor_delegate_win.cc
[add] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/chrome/browser/memory/swap_thrashing_monitor_delegate_win.h
[add] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/chrome/browser/memory/swap_thrashing_monitor_delegate_win_unittest.cc
[delete] https://crrev.com/23c1566fcff5ea60c0d3fc2de833e4af09ebaaa0/chrome/browser/memory/swap_thrashing_monitor_win.cc
[delete] https://crrev.com/23c1566fcff5ea60c0d3fc2de833e4af09ebaaa0/chrome/browser/memory/swap_thrashing_monitor_win.h
[delete] https://crrev.com/23c1566fcff5ea60c0d3fc2de833e4af09ebaaa0/chrome/browser/memory/swap_thrashing_monitor_win_unittest.cc
[modify] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/chrome/test/BUILD.gn
[modify] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/f9a0d07b6b5d884e95430ff9dc60519912717fcf/tools/metrics/histograms/histograms.xml

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8bb99981f32c5166550c7a08f2a924226204085c

commit 8bb99981f32c5166550c7a08f2a924226204085c
Author: Sebastien Marchand <sebmarchand@chromium.org>
Date: Tue Oct 31 05:41:20 2017

Minor fixes to the swap thrashing detector.

- Use the appropriate UMA macros.
- The hard fault counters can sometime overflow, handle this.
- Rename the UMA metrics to Memory.Experimental
- The swapping threshold was too low, it has been guesstimated locally
  with a sampling rate of 1Hz but the monitor runs at 0.5Hz, so the
  value has to be higher.

Bug: 771478
Change-Id: I3038a55f97b78bef1e4a17793d7a1a8196a89230
TBR: chrisha@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/745703
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512753}
[modify] https://crrev.com/8bb99981f32c5166550c7a08f2a924226204085c/chrome/browser/memory/swap_thrashing_monitor.cc
[modify] https://crrev.com/8bb99981f32c5166550c7a08f2a924226204085c/chrome/browser/memory/swap_thrashing_monitor_delegate_win.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f55a147384647745bdde5d3b9f575d6a12a4ab7

commit 8f55a147384647745bdde5d3b9f575d6a12a4ab7
Author: Sebastien Marchand <sebmarchand@chromium.org>
Date: Thu Nov 02 08:42:48 2017

Temporarily disable the SwapThrashingMonitor

The call to NtQuerySystemInformation seems expensive (see crbug.com/780597),
disabling this while I look at better alternatives.

Bug: 780597, 771478
Change-Id: I3355c26695095498d4b459e8e19727be196dc129
TBR: jochen@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/749569
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513427}
[modify] https://crrev.com/8f55a147384647745bdde5d3b9f575d6a12a4ab7/chrome/browser/chrome_browser_main_win.cc
[modify] https://crrev.com/8f55a147384647745bdde5d3b9f575d6a12a4ab7/chrome/browser/memory/swap_thrashing_monitor_delegate_win.cc

This work is currently blocked by the fact that measuring the hard page-fault rate on Windows is really expensive (see crbug.com/780597).

I'm looking at some alternative approach to get this data, there doesn't seem to be another way to get this exact information but we could maybe derive it from other signals (e.g. the I/O rate).

I'll collect more data to see if the hard page-fault rate is correlated with some other signals.
Status: Assigned (was: Untriaged)
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/96f850f7d81dcc52a4f429a54c83b9f39975fb5e

commit 96f850f7d81dcc52a4f429a54c83b9f39975fb5e
Author: Sebastien Marchand <sebmarchand@chromium.org>
Date: Mon Sep 17 16:21:20 2018

Put the swap thrashing monitor behind a feature flag

This'll allow experimenting with this signal locally (this is disabled
by default),

Bug: 771478
Change-Id: Ic41b53831a18a24874873805b87fc5027598a953
Reviewed-on: https://chromium-review.googlesource.com/1226309
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591701}
[modify] https://crrev.com/96f850f7d81dcc52a4f429a54c83b9f39975fb5e/chrome/browser/chrome_browser_main_win.cc
[modify] https://crrev.com/96f850f7d81dcc52a4f429a54c83b9f39975fb5e/chrome/browser/memory/swap_thrashing_monitor.cc
[modify] https://crrev.com/96f850f7d81dcc52a4f429a54c83b9f39975fb5e/chrome/browser/memory/swap_thrashing_monitor.h
[modify] https://crrev.com/96f850f7d81dcc52a4f429a54c83b9f39975fb5e/chrome/browser/memory/swap_thrashing_monitor_delegate_win.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Yesterday (24 hours ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/96ff7c008eedbeba2d794e02a526f671c56d6027

commit 96ff7c008eedbeba2d794e02a526f671c56d6027
Author: Sebastien Marchand <sebmarchand@chromium.org>
Date: Tue Jan 22 18:06:40 2019

Rename PerformanceMonitor to ProcessMonitor

A new SystemMonitor class will be added to this directory, renaming this
class will make it clear that it's tracking some process performance
metrics (the new class will focus on the system metrics).

Bug: 771478
Change-Id: I1d9fa1901f3b36e77696cd17545db7274fca51fd
Reviewed-on: https://chromium-review.googlesource.com/c/1418119
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624831}
[modify] https://crrev.com/96ff7c008eedbeba2d794e02a526f671c56d6027/chrome/browser/BUILD.gn
[modify] https://crrev.com/96ff7c008eedbeba2d794e02a526f671c56d6027/chrome/browser/chrome_browser_main.cc
[rename] https://crrev.com/96ff7c008eedbeba2d794e02a526f671c56d6027/chrome/browser/performance_monitor/process_monitor.cc
[rename] https://crrev.com/96ff7c008eedbeba2d794e02a526f671c56d6027/chrome/browser/performance_monitor/process_monitor.h

Sign in to add a comment