|
|
Created:
4 years, 6 months ago by halliwell Modified:
4 years, 6 months ago CC:
chromium-reviews, alokp+watch_chromium.org, lcwu+watch_chromium.org, halliwell+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Chromecast] Add free memory UMA histograms
MeminfoMemFree is currently recorded by sysmon; the problem with this is
we don't get the data for all devices. MemFreeDerived is newly relevant
for new memory pressure heuristic.
BUG=internal b/26029747
Committed: https://crrev.com/f6070f3599da694dad1b86bac46ffe41e74619b0
Cr-Commit-Position: refs/heads/master@{#398019}
Patch Set 1 #
Total comments: 1
Messages
Total messages: 15 (5 generated)
Description was changed from ========== [Chromecast] Add free memory UMA histograms MeminfoMemFree is currently recorded by sysmon; the problem with this is we don't get the data for all devices. MemFreeDerived is newly relevant for new memory pressure heuristic. BUG=internal b/26029747 ========== to ========== [Chromecast] Add free memory UMA histograms MeminfoMemFree is currently recorded by sysmon; the problem with this is we don't get the data for all devices. MemFreeDerived is newly relevant for new memory pressure heuristic. BUG=internal b/26029747 ==========
halliwell@chromium.org changed reviewers: + gfhuang@chromium.org, wzhong@chromium.org
lgtm
https://codereview.chromium.org/2028233002/diff/1/chromecast/browser/cast_mem... File chromecast/browser/cast_memory_pressure_monitor.cc (right): https://codereview.chromium.org/2028233002/diff/1/chromecast/browser/cast_mem... chromecast/browser/cast_memory_pressure_monitor.cc:97: info.total, 100); The info.total varies by platform though, for example can be 2G on TV product.
On 2016/06/01 20:24:25, wzhong wrote: > https://codereview.chromium.org/2028233002/diff/1/chromecast/browser/cast_mem... > File chromecast/browser/cast_memory_pressure_monitor.cc (right): > > https://codereview.chromium.org/2028233002/diff/1/chromecast/browser/cast_mem... > chromecast/browser/cast_memory_pressure_monitor.cc:97: info.total, 100); > The info.total varies by platform though, for example can be 2G on TV product. Indeed. I was following here how ChromeOS collects the same stat ... presumably they must have hw with different memory totals ... do they have an ability to segment by hardware that we don't have? We can just go with % if that's easier. Although eventually we will want to segment by hw in order for this to be meaningful.
On 2016/06/01 21:05:23, halliwell wrote: > On 2016/06/01 20:24:25, wzhong wrote: > > > https://codereview.chromium.org/2028233002/diff/1/chromecast/browser/cast_mem... > > File chromecast/browser/cast_memory_pressure_monitor.cc (right): > > > > > https://codereview.chromium.org/2028233002/diff/1/chromecast/browser/cast_mem... > > chromecast/browser/cast_memory_pressure_monitor.cc:97: info.total, 100); > > The info.total varies by platform though, for example can be 2G on TV product. > > Indeed. I was following here how ChromeOS collects the same stat ... presumably > they must have hw with different memory totals ... do they have an ability to > segment by hardware that we don't have? > > We can just go with % if that's easier. Although eventually we will want to > segment by hw in order for this to be meaningful. Percentage should suffice in our case, but need a distinct name if CrOS already uses it for absolute values.
On 2016/06/02 00:39:45, wzhong wrote: > On 2016/06/01 21:05:23, halliwell wrote: > > On 2016/06/01 20:24:25, wzhong wrote: > > > > > > https://codereview.chromium.org/2028233002/diff/1/chromecast/browser/cast_mem... > > > File chromecast/browser/cast_memory_pressure_monitor.cc (right): > > > > > > > > > https://codereview.chromium.org/2028233002/diff/1/chromecast/browser/cast_mem... > > > chromecast/browser/cast_memory_pressure_monitor.cc:97: info.total, 100); > > > The info.total varies by platform though, for example can be 2G on TV > product. > > > > Indeed. I was following here how ChromeOS collects the same stat ... > presumably > > they must have hw with different memory totals ... do they have an ability to > > segment by hardware that we don't have? > > > > We can just go with % if that's easier. Although eventually we will want to > > segment by hw in order for this to be meaningful. > > Percentage should suffice in our case, but need a distinct name if CrOS already > uses it for absolute values. I spoke with the CrOS owner of this metric, they're fine with us using it too (they already have filters so our data won't affect them). I think we should just do the same as them: record kbytes and filter based on total RAM when looking at the data.
lgtm
The CQ bit was checked by halliwell@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2028233002/1
Message was sent while issue was closed.
Description was changed from ========== [Chromecast] Add free memory UMA histograms MeminfoMemFree is currently recorded by sysmon; the problem with this is we don't get the data for all devices. MemFreeDerived is newly relevant for new memory pressure heuristic. BUG=internal b/26029747 ========== to ========== [Chromecast] Add free memory UMA histograms MeminfoMemFree is currently recorded by sysmon; the problem with this is we don't get the data for all devices. MemFreeDerived is newly relevant for new memory pressure heuristic. BUG=internal b/26029747 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== [Chromecast] Add free memory UMA histograms MeminfoMemFree is currently recorded by sysmon; the problem with this is we don't get the data for all devices. MemFreeDerived is newly relevant for new memory pressure heuristic. BUG=internal b/26029747 ========== to ========== [Chromecast] Add free memory UMA histograms MeminfoMemFree is currently recorded by sysmon; the problem with this is we don't get the data for all devices. MemFreeDerived is newly relevant for new memory pressure heuristic. BUG=internal b/26029747 Committed: https://crrev.com/f6070f3599da694dad1b86bac46ffe41e74619b0 Cr-Commit-Position: refs/heads/master@{#398019} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/f6070f3599da694dad1b86bac46ffe41e74619b0 Cr-Commit-Position: refs/heads/master@{#398019} |