[Mac] Collect real-time memory pressure stats, in an energy-efficient way

[Mac] Collect real-time memory pressure stats, in an energy-efficient way

macOS sends memory pressure change notifications, but those notifications lag the memory pressure change event by up to 60s. For accurate memory pressure change statistics we want a more timely pressure change signal. The only way to do that is with polling, but using a timer means consuming more CPU.

This cl adds a CFRunLoopObserver that updates the current memory pressure level at the end of every run loop pass (modulo 5s), allowing UMA to accurately track the time spent in the different memory pressure states.

BUG=569166
Review-Url: https://codereview.chromium.org/1587273002
Cr-Commit-Position: refs/heads/master@{#454880}
Committed: https://chromium.googlesource.com/chromium/src/+/c26b5914b28e0a145481fec6d942f0dbba97e227

[shrike, 2017-02-22 20:46:15]

Description was changed from

==========
WIP - Implement a new memory pressure signal on Mac.

BUG=569166
==========

to

==========
[Mac] Collect real-time memory pressure stats, in an energy-efficient way

macOS sends memory pressure change notifications, but those notifications lag
the memory pressure change event by up to 60s. For accurate memory pressure
change statistics we want a more timely pressure change signal. The only way to
do that is with polling, but using a timer means consuming more CPU.

This cl adds a CFRunLoopObserver that updates the current memory pressure level
at the end of every run loop pass (modulo 5s), allowing UMA to accurately track
the time spent in the different memory pressure states.

BUG=569166
==========

[shrike, 2017-02-22 20:46:15]

shrike@chromium.org changed reviewers:
+ lgrey@chromium.org

[shrike, 2017-02-22 20:46:36]

lgrey@ - PTAL

[lgrey, 2017-02-22 21:33:40]

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
File base/memory/memory_pressure_monitor_mac.cc (right):

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
base/memory/memory_pressure_monitor_mac.cc:154: next_run_loop_update_time_ = now
+ kUMATickSize;
Should we account for the error? (`now + kUMATickSize - subtick_seconds_` for
example)

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
File base/memory/memory_pressure_monitor_mac.h (right):

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
base/memory/memory_pressure_monitor_mac.h:52: // some numebr of seconds).
Nit: number

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
File base/memory/memory_pressure_monitor_mac_unittest.cc (right):

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
base/memory/memory_pressure_monitor_mac_unittest.cc:202: EXPECT_EQ(2,
monitor.SubTickSeconds());
I wonder if we should always be rounding up for consistency.

[shrike, 2017-02-22 23:56:25]

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
File base/memory/memory_pressure_monitor_mac.cc (right):

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
base/memory/memory_pressure_monitor_mac.cc:154: next_run_loop_update_time_ = now
+ kUMATickSize;
On 2017/02/22 21:33:40, lgrey wrote:
> Should we account for the error? (`now + kUMATickSize - subtick_seconds_` for
> example)

OK.

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
File base/memory/memory_pressure_monitor_mac.h (right):

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
base/memory/memory_pressure_monitor_mac.h:52: // some numebr of seconds).
On 2017/02/22 21:33:40, lgrey wrote:
> Nit: number

Done.

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
File base/memory/memory_pressure_monitor_mac_unittest.cc (right):

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
base/memory/memory_pressure_monitor_mac_unittest.cc:202: EXPECT_EQ(2,
monitor.SubTickSeconds());
On 2017/02/22 21:33:40, lgrey wrote:
> I wonder if we should always be rounding up for consistency.

I think if we do, and if we get a lot of subtick_seconds_ (they could be
generated, for example, by other code calling GetCurrentPressureLevel() ), we
could end up mis-computing time spent in pressure levels. Currently we show
users in memory pressure something like 5% of the time, which is 3 seconds out
of every minute. It doesn't take many extra seconds per minute to skew that. I
don't know, maybe unrealistic, but I think we should leave it unless you see a
big problem with it.

[lgrey, 2017-02-23 15:49:29]

LGTM

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
File base/memory/memory_pressure_monitor_mac_unittest.cc (right):

https://codereview.chromium.org/1587273002/diff/100001/base/memory/memory_pre...
base/memory/memory_pressure_monitor_mac_unittest.cc:202: EXPECT_EQ(2,
monitor.SubTickSeconds());
On 2017/02/22 23:56:25, shrike wrote:
> On 2017/02/22 21:33:40, lgrey wrote:
> > I wonder if we should always be rounding up for consistency.
> 
> I think if we do, and if we get a lot of subtick_seconds_ (they could be
> generated, for example, by other code calling GetCurrentPressureLevel() ), we
> could end up mis-computing time spent in pressure levels. Currently we show
> users in memory pressure something like 5% of the time, which is 3 seconds out
> of every minute. It doesn't take many extra seconds per minute to skew that. I
> don't know, maybe unrealistic, but I think we should leave it unless you see a
> big problem with it.

Nah, no problem. I guess the inaccuracy is baked in from quantizing to 5 seconds
and it's just a question of whether the old level is overcounted or the new
level.

[shrike, 2017-02-23 17:33:38]

shrike@chromium.org changed reviewers:
+ mark@chromium.org

[shrike, 2017-02-23 17:33:38]

mark@ - PTAL (owner's approval)

[Mark Mentovai, 2017-02-23 18:52:02]

LGTM

[shrike, 2017-03-06 16:47:55]

The CQ bit was checked by shrike@chromium.org

[commit-bot: I haz the power, 2017-03-06 16:48:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-06 17:20:00]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1488818875677250,
"parent_rev": "279edadbd6962b7afa97c166896ae69ceb797da4", "commit_rev":
"c26b5914b28e0a145481fec6d942f0dbba97e227"}

[commit-bot: I haz the power, 2017-03-06 17:20:40]

Description was changed from

==========
[Mac] Collect real-time memory pressure stats, in an energy-efficient way

macOS sends memory pressure change notifications, but those notifications lag
the memory pressure change event by up to 60s. For accurate memory pressure
change statistics we want a more timely pressure change signal. The only way to
do that is with polling, but using a timer means consuming more CPU.

This cl adds a CFRunLoopObserver that updates the current memory pressure level
at the end of every run loop pass (modulo 5s), allowing UMA to accurately track
the time spent in the different memory pressure states.

BUG=569166
==========

to

==========
[Mac] Collect real-time memory pressure stats, in an energy-efficient way

macOS sends memory pressure change notifications, but those notifications lag
the memory pressure change event by up to 60s. For accurate memory pressure
change statistics we want a more timely pressure change signal. The only way to
do that is with polling, but using a timer means consuming more CPU.

This cl adds a CFRunLoopObserver that updates the current memory pressure level
at the end of every run loop pass (modulo 5s), allowing UMA to accurately track
the time spent in the different memory pressure states.

BUG=569166

Review-Url: https://codereview.chromium.org/1587273002
Cr-Commit-Position: refs/heads/master@{#454880}
Committed:
https://chromium.googlesource.com/chromium/src/+/c26b5914b28e0a145481fec6d942...
==========

[commit-bot: I haz the power, 2017-03-06 17:20:42]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/c26b5914b28e0a145481fec6d942...