Improve process reuse policies |
|||
Issue descriptionI'm filing this to keep track of ideas for improving Chrome's process reuse policy, as some aspects of this have come up in recent bugs and discussions. Some ideas (will need more research and experimentation to see if they're helpful): 1. Currently with site isolation, OOPIFs will aggressively try to reuse an existing process for the same site (see r522537 and issue 512560 ). This is done via the REUSE_PENDING_OR_COMMITTED_SITE process reuse policy. Perhaps we don't have to aggressively reuse when the user only has a couple of processes and is way below the process limit, but we can start progressively reusing more as the user gets closer to the process limit. It's likely that a few sites might account for a lot of the user's workload (e.g., google.com), and this might help split that workload up into more processes and get more performance isolation, when possible. 2. For both OOPIFs, and for all frames when over the process limit, we will attempt to reuse a existing process for the same site (assuming site isolation), but currently this picks a process randomly from the set of eligible processes. Perhaps we could be smarter about the choice and try to better balance the load across processes, e.g. by picking the process that's least loaded (in terms memory, CPU, # of other frames, etc.). 3. OOPIF process reuse favors foreground processes (those that have at least one visible widget) over background processes (see RPHI::FindReusableProcessHostForSiteInstance()). This might've been needed for ServiceWorkers, which use the same process reuse policy, but it's not clear this is always the right choice for OOPIFs (see 2). 4. In certain cases, a frame may be doing a lot of work, in which case putting it into a process with other tabs may adversely affect the performance of those other tabs. A concrete example of this is issue 899418, for web iframes in extension background pages. Maybe we can come up with generic ways to avoid sharing processes in cases like this. 5. OOPIF process reuse can't currently reuse processes which have only frames that are pending deletion, since it relies on tracking which sites have committed in which processes, rather than considering all available processes. An example of this is issue 894253, where navigating from a page with 10 OOPIFs to a same-site page with another 10 OOPIFs, the new OOPIFs can't reuse any of the subframe processes from the first page, even when they're still around and dedicated to the right sites. There were some past discussions on this, and I'm sure there are other ideas, so please feel free to add them.
,
Nov 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9c9e38873afe812dda67da489ca9d5db653c5587 commit 9c9e38873afe812dda67da489ca9d5db653c5587 Author: Alex Moshchuk <alexmos@chromium.org> Date: Fri Nov 02 19:57:03 2018 Stop trying to reuse processes for web iframes on extension pages. There's evidence that some web iframes embedded on extension background pages may be doing a lot of work with the assumption that since this work isn't on a foreground tab, it doesn't need to be optimized for responsiveness. However, OOPIFs currently use an aggressive process reuse policy, which also affects this use case, making it possible for a web iframe on an extension to get into another tab's process and adversely affect its performance. This CL avoids using that reuse policy for web iframes on extensions. Caveats: - When over the process limit, web iframes on extensions will still attempt to reuse an existing process. - OOPIFs from normal tabs may still join processes for web iframes on extensions. This and the previous point imply that there won't be a guarantee of perfect performance isolation for web iframes on extensions; the goal is to be more resilient to problems in the common case. - We will stop potentially useful process reuse between same-site iframes on multiple instances of extensions, even for instances of the same extension. We expect this to be rare in practice though. Bug: 899418, 899838 Change-Id: I35f6ecc1945292f9fab1c21f65d1ac4b7970dbe3 Reviewed-on: https://chromium-review.googlesource.com/c/1306410 Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#605028} [modify] https://crrev.com/9c9e38873afe812dda67da489ca9d5db653c5587/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/9c9e38873afe812dda67da489ca9d5db653c5587/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/9c9e38873afe812dda67da489ca9d5db653c5587/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc [modify] https://crrev.com/9c9e38873afe812dda67da489ca9d5db653c5587/chrome/browser/extensions/chrome_content_browser_client_extensions_part.h [modify] https://crrev.com/9c9e38873afe812dda67da489ca9d5db653c5587/chrome/browser/extensions/process_manager_browsertest.cc [modify] https://crrev.com/9c9e38873afe812dda67da489ca9d5db653c5587/chrome/browser/site_details_browsertest.cc [modify] https://crrev.com/9c9e38873afe812dda67da489ca9d5db653c5587/content/browser/frame_host/render_frame_host_manager.cc [modify] https://crrev.com/9c9e38873afe812dda67da489ca9d5db653c5587/content/public/browser/content_browser_client.cc [modify] https://crrev.com/9c9e38873afe812dda67da489ca9d5db653c5587/content/public/browser/content_browser_client.h
,
Nov 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/366ed8d8c2720dc4bff259a6ea98d8e8888bc28e commit 366ed8d8c2720dc4bff259a6ea98d8e8888bc28e Author: Alex Moshchuk <alexmos@chromium.org> Date: Mon Nov 12 22:31:19 2018 Merge to M71: Stop trying to reuse processes for web iframes on extension pages. There's evidence that some web iframes embedded on extension background pages may be doing a lot of work with the assumption that since this work isn't on a foreground tab, it doesn't need to be optimized for responsiveness. However, OOPIFs currently use an aggressive process reuse policy, which also affects this use case, making it possible for a web iframe on an extension to get into another tab's process and adversely affect its performance. This CL avoids using that reuse policy for web iframes on extensions. Caveats: - When over the process limit, web iframes on extensions will still attempt to reuse an existing process. - OOPIFs from normal tabs may still join processes for web iframes on extensions. This and the previous point imply that there won't be a guarantee of perfect performance isolation for web iframes on extensions; the goal is to be more resilient to problems in the common case. - We will stop potentially useful process reuse between same-site iframes on multiple instances of extensions, even for instances of the same extension. We expect this to be rare in practice though. TBR=alexmos@chromium.org (cherry picked from commit 9c9e38873afe812dda67da489ca9d5db653c5587) Bug: 899418, 899838 Change-Id: I35f6ecc1945292f9fab1c21f65d1ac4b7970dbe3 Reviewed-on: https://chromium-review.googlesource.com/c/1306410 Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#605028} Reviewed-on: https://chromium-review.googlesource.com/c/1332265 Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#643} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/366ed8d8c2720dc4bff259a6ea98d8e8888bc28e/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/366ed8d8c2720dc4bff259a6ea98d8e8888bc28e/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/366ed8d8c2720dc4bff259a6ea98d8e8888bc28e/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc [modify] https://crrev.com/366ed8d8c2720dc4bff259a6ea98d8e8888bc28e/chrome/browser/extensions/chrome_content_browser_client_extensions_part.h [modify] https://crrev.com/366ed8d8c2720dc4bff259a6ea98d8e8888bc28e/chrome/browser/extensions/process_manager_browsertest.cc [modify] https://crrev.com/366ed8d8c2720dc4bff259a6ea98d8e8888bc28e/chrome/browser/site_details_browsertest.cc [modify] https://crrev.com/366ed8d8c2720dc4bff259a6ea98d8e8888bc28e/content/browser/frame_host/render_frame_host_manager.cc [modify] https://crrev.com/366ed8d8c2720dc4bff259a6ea98d8e8888bc28e/content/public/browser/content_browser_client.cc [modify] https://crrev.com/366ed8d8c2720dc4bff259a6ea98d8e8888bc28e/content/public/browser/content_browser_client.h
,
Nov 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/366ed8d8c2720dc4bff259a6ea98d8e8888bc28e Commit: 366ed8d8c2720dc4bff259a6ea98d8e8888bc28e Author: alexmos@chromium.org Commiter: alexmos@chromium.org Date: 2018-11-12 22:31:19 +0000 UTC Merge to M71: Stop trying to reuse processes for web iframes on extension pages. There's evidence that some web iframes embedded on extension background pages may be doing a lot of work with the assumption that since this work isn't on a foreground tab, it doesn't need to be optimized for responsiveness. However, OOPIFs currently use an aggressive process reuse policy, which also affects this use case, making it possible for a web iframe on an extension to get into another tab's process and adversely affect its performance. This CL avoids using that reuse policy for web iframes on extensions. Caveats: - When over the process limit, web iframes on extensions will still attempt to reuse an existing process. - OOPIFs from normal tabs may still join processes for web iframes on extensions. This and the previous point imply that there won't be a guarantee of perfect performance isolation for web iframes on extensions; the goal is to be more resilient to problems in the common case. - We will stop potentially useful process reuse between same-site iframes on multiple instances of extensions, even for instances of the same extension. We expect this to be rare in practice though. TBR=alexmos@chromium.org (cherry picked from commit 9c9e38873afe812dda67da489ca9d5db653c5587) Bug: 899418, 899838 Change-Id: I35f6ecc1945292f9fab1c21f65d1ac4b7970dbe3 Reviewed-on: https://chromium-review.googlesource.com/c/1306410 Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#605028} Reviewed-on: https://chromium-review.googlesource.com/c/1332265 Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#643} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} |
|||
►
Sign in to add a comment |
|||
Comment 1 by lukasza@chromium.org
, Oct 29