New issue
Advanced search Search tips

Issue 864575 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

add hysteresis to tab discard?

Project Member Reported by semenzato@chromium.org, Jul 17

Issue description

From my recollection, there are numerous cases in which chrome sees a low-memory signal, does a bunch of computation to figure out which tabs to discard, then checks available memory, finds it's above the threshold, and aborts the discard.

I am wondering how common this is, how expensive, and whether it's worth reducing such occurrences by adding some hysteresis (i.e. add a fixed quantity to the target-to-free amount).  We could add an UMA stat to find out, or wait for data from memd. 

 
Yeah I think this is a good idea -- though, it's complicated by the fact that we're still polling for tab discard signals.  Probably we should fix that first?

When I was trying to fix the polling I found that we discarded way more and it wasn't clear that that was better than before, so I'm thinking that if we do get rid of the polling, hysteresis would be even more beneficial.
Cc: cylee@chromium.org cywang@chromium.org
I think it's a good idea. I also see a lot of aborted discard. I don't think of any drawback to add this hysteresis.

It can be done by increasing the return value of TabManagerDelegate::MemoryStat::TargetMemoryToFreeKB() in tab_manager_delegate_chromeos.cc .
Components: -OS>Performance OS>Performance>Memory
Cc: chinglinyu@chromium.org
Thanks for cywang's help, the following are the reports that low-memory-notify is triggered but no tab is discarded.
These are reported on 2018/07/31.

https://listnr.corp.google.com/product/208/report/85578331680  peppy-release/R64-10176.76.0
https://listnr.corp.google.com/product/208/report/85578353410  veyron_jerry-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85578716603  edgar-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85578760029  kip-release/R64-10176.76.0
https://listnr.corp.google.com/product/208/report/85578926105  leon-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85578972931  peppy-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85579036148  celes-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85579337375  caroline-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85579459283  veyron_speedy-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85579483592  banon-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85579574895  nyan_blaze-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85579788163  veyron_jerry-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85579855553  daisy-release/R64-10176.72.0
https://listnr.corp.google.com/product/208/report/85579925336  caroline-release/R69-10895.10.0
https://listnr.corp.google.com/product/208/report/85579932507  veyron_jerry-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85580012326  banon-release/R67-10575.58.0
https://listnr.corp.google.com/product/208/report/85580014221  veyron_minnie-release/R67-10575.58.0

For example, in 85579932507 (veyron_jerry-release/R67-10575.58.0):

2018-08-01 18:25:46.273 4 kernel  :  [ 2711.073306] chrome invoked oom-killer: gfp_mask=0x200da, order=0, oom_score_adj=300
[18:26:13.549] Memory: tab_manager_delegate_chromeos.cc:573 Target memory to free: -481280 KB
[18:26:13.682] Memory: tab_manager_delegate_chromeos.cc:573 Target memory to free: -481280 KB
[18:26:13.773] Memory: tab_manager_delegate_chromeos.cc:573 Target memory to free: -479232 KB
[18:26:13.918] Memory: tab_manager_delegate_chromeos.cc:573 Target memory to free: -479232 KB
[18:26:14.036] Memory: tab_manager_delegate_chromeos.cc:573 Target memory to free: -478208 KB
[18:26:14.151] Memory: tab_manager_delegate_chromeos.cc:573 Target memory to free: -479232 KB
[18:26:14.247] Memory: tab_manager_delegate_chromeos.cc:573 Target memory to free: -479232 KB
[18:26:14.373] Memory: tab_manager_delegate_chromeos.cc:573 Target memory to free: -483328 KB
[18:26:14.476] Memory: tab_manager_delegate_chromeos.cc:573 Target memory to free: -481280 KB

Tab discarder is invoked after oom-killer is invoked. And there are a lot of queued discarding events.

The fixes in  https://crbug.com/850545  should reduce this problem. And the discarding event should be skipped ASAP when available memory is above margin.

P.S.: the datetime of the kernel log is calculated by Android Bug Tool.
Hysteresis is not so helpful for the cases in #5. Before adding hysteresis, we may need to find some feedback reports to support adding it.
re #5,6,7 -- how did you pick those reports? 

I think for cases where the OOM killer was invoked, yes we're going to see what looks like false positive low-memory signals because the OOM killer activated faster than the discarder. 

I think as things stand now, we kind of get some of the effect of hysteresis because we're polling and so there's lag between when the signal could have woken up the discarder and when it actually does. 

This is one of the reports from #5 where there's a single activation of the discarder but nothing was discarded
https://listnr.corp.google.com/product/208/report/85579337375

I believe In this type of case, if we stopped polling and activated the discarder sooner, we would probably do a discard.  However it was possibly unnecessary because the system would eventually catch up.

When I was looking at getting rid of polling I saw something similar where the number of discards actually went up quite a bit compared to polling, and I think hysteresis would help to limit that effect.

I think memd reports would also give us more data about whether we're excessively discarding or not. 
I pick the reports in #5 by searching all CrOS user feedback reports in 2018/07/31 that contain "Target memory to free: \d* KB" but don't contain "Killed tab .*, estimated \d* KB freed"

Sign in to add a comment