Moderate binding management with site isolation |
|
Issue descriptionKind of brain dump here. The idea if binding management is that tabs that a user has used in a session should be lifted to moderate binding. It is currently implemented as a LRU list, where "use" is when a renderer process goes from invisible to visible. It has pretty aggressive policies to listen to memory pressure and prune the list, ie removing moderate binding, in LRU order. The list is also cleared 10 seconds after chrome put into background. Problems I see that affects site isolation: 1) LRU is the wrong order for processes within a tab. Actually it's probably the worst possible order. 2) dskiba@ (before he left) worked on the idea of sustained memory pressure. android's memory pressure signal doesn't tell us when memory pressure goes away, and dskiba@ devised a way to poll android to get the current memory level. Binding manager doesn't really do this, ie processes can still be added even while there is memory pressure, which is probably bad 3) ssid@ mentioned dskiba@'s investigation of memory pressure signal behavior in the past: the signal is sent to a process only if it is in the last 5 to be killed. If this is true, then this kind of breaks memory pressure signals, especially with site isolation that uses more renderer processes. And this would make binding manager not aggressive enough in dropping bindings. I did an unscientific experiment to turn off binding management when site isolation is enabled, and kept it on canary for 2 weeks, and compared it to 2 weeks before. GPU OOM dropped as expected. Visible main renderer oom actually went up though; the speculated cause here is it's due to race when switching tabs; a renderer is killed immediately before user switches to it, then it could be mis-attributed to be a visible oom; and this is more likely to happen without binding management since the process would be dropped to waived. My takeaway is that tweaking binding management can have large impact visible main renderer oom, which causes sad tabs. Ideas on what to do: To solve 1), sort the queue in the order we want, ie by depth. For 2), could with android on the current memory level is. However, it's not clear to me what should happen if there is memory pressure though. If 3) is true, then there isn't really any solution. But might be worth some investigation. I'm also weighing having binding manager prefer invisible main frame over visible sub frames, to address speculation from the experiment. That gets further away from the original LRU idea though.
,
Aug 2
> This causes some memory pressure and chrome makes some background tabs as waived binding and they get moved to "Cached". So.. 3) above is wrong, and browser does receive memory pressure signal even if it's not in the first 5 in the queue to be killed? So I think overall, sounds like chrome is keeping moderate binding on too many processes? https://chromium-review.googlesource.com/1132468 should drop iframes that are not in the viewport down to waived. Which should help a bit. And on this code review, I suggested maybe we should limit moderate binding management to main frames only, and let the subframes sort themselves out.. https://chromium-review.googlesource.com/c/chromium/src/+/1159359 None of this helps much if android keeps a strict limit on number of cached processes though. We can't drop things below waived..
,
Aug 3
I believe 3) above is just wrong. My understanding: * There are a different set of signals you get if you're in the foreground or not. * When in the foreground, the signal is based on the count of background processes * When in the background, it is based on which third of the LRU you are in.
,
Aug 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a62bd679b588a13066d2e117f07ac8df4bf06591 commit a62bd679b588a13066d2e117f07ac8df4bf06591 Author: Bo Liu <boliu@chromium.org> Date: Mon Aug 13 23:05:08 2018 android: Use ChildProcessRanking in BindingManager BindingManager maintains a list of Connections that is separate and in different order compared to ChildProcessRanking. This is a problem with OOPIF because BindingManager's order of processes _within_ a tab is almost certainly the worst possible order: visibility is flipped from root frame to leaf frame, which means a LRU order would remove root frame first. Previous attempt of letting BindingManager use Ranking directly and manipulating the list of moderate binding connections through priority updates deemed to confusing and thus abandoned: https://chromium-review.googlesource.com/c/chromium/src/+/1054642 This is a second attempt: adding connection and removing the number of connections is kept exactly the same. The only difference is connections are removed in the order of ChildProcessRanking. Make ChildProcessRanking an iterable that iterates connections that rank the lowest to rank the highest. Then change BindingManager to hold a set, and on removal, use the order of Ranking. Bug: 868603 Change-Id: I7765d3f6d78369dfec3de928c83b3bec0b2cadfe Reviewed-on: https://chromium-review.googlesource.com/1159359 Commit-Queue: Bo <boliu@chromium.org> Reviewed-by: Siddhartha S <ssid@chromium.org> Cr-Commit-Position: refs/heads/master@{#582734} [modify] https://crrev.com/a62bd679b588a13066d2e117f07ac8df4bf06591/content/public/android/java/src/org/chromium/content/browser/BindingManager.java [modify] https://crrev.com/a62bd679b588a13066d2e117f07ac8df4bf06591/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncherHelperImpl.java [modify] https://crrev.com/a62bd679b588a13066d2e117f07ac8df4bf06591/content/public/android/java/src/org/chromium/content/browser/ChildProcessRanking.java [modify] https://crrev.com/a62bd679b588a13066d2e117f07ac8df4bf06591/content/public/android/junit/src/org/chromium/content/browser/BindingManagerTest.java [modify] https://crrev.com/a62bd679b588a13066d2e117f07ac8df4bf06591/content/public/android/junit/src/org/chromium/content/browser/ChildProcessRankingTest.java |
|
►
Sign in to add a comment |
|
Comment 1 by ssid@chromium.org
, Aug 2