Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2590073002:
Disable suspending renderer when memory coordinator is enabled (Closed)
DescriptionDisable suspending renderer when memory coordinator is enabled
Just calling SuspendRenderer() in OnMemoryStateChange() breaks
many things. We need a reliable and consistent way to suspend
renderers. Until we come up with a solution, disable renderer
suspending when memory coordinator is enabled. In follow-up
CLs I'll add some tests to prevent this kind of regressions.
BUG=675735, 675811
Committed: https://crrev.com/b1536fd7e1fa4dd7559e2c12e290e79cf949e9fe
Cr-Commit-Position: refs/heads/master@{#439708}
Patch Set 1 #
Messages
Total messages: 19 (9 generated)
The CQ bit was checked by bashi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
bashi@chromium.org changed reviewers: + haraken@chromium.org, tasak@google.com
PTAL
LGTM I'm a bit concerned if SuspendRenderer would be causing the same issue though.
On 2016/12/20 02:34:48, haraken wrote: > LGTM > > I'm a bit concerned if SuspendRenderer would be causing the same issue though. Yes, I have the same concern. tasak@: Would you mind checking it (esp. tracing) ?
bashi@chromium.org changed reviewers: + avi@chromium.org
+avi@ for OWNERS review. (IIUC concern raised in comment #5 is about purge+suspend experiment and this CL is for memory coordinator. They are separate things at this point)
lgtm stamp Good luck.
Thank you for the quick review!
The CQ bit was unchecked by bashi@chromium.org
The CQ bit was checked by bashi@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1482207079572250, "parent_rev":
"30e9fe3de06342b853de0c7f1aa658b844c797c1", "commit_rev":
"e3072a46580726d6b75d8905490f0f96c48434bb"}
Message was sent while issue was closed.
Description was changed from ========== Disable suspending renderer when memory coordinator is enabled Just calling SuspendRenderer() in OnMemoryStateChange() breaks many things. We need a reliable and consistent way to suspend renderers. Until we come up with a solution, disable renderer suspending when memory coordinator is enabled. In follow-up CLs I'll add some tests to prevent this kind of regressions. BUG=675735,675811 ========== to ========== Disable suspending renderer when memory coordinator is enabled Just calling SuspendRenderer() in OnMemoryStateChange() breaks many things. We need a reliable and consistent way to suspend renderers. Until we come up with a solution, disable renderer suspending when memory coordinator is enabled. In follow-up CLs I'll add some tests to prevent this kind of regressions. BUG=675735,675811 Review-Url: https://codereview.chromium.org/2590073002 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Disable suspending renderer when memory coordinator is enabled Just calling SuspendRenderer() in OnMemoryStateChange() breaks many things. We need a reliable and consistent way to suspend renderers. Until we come up with a solution, disable renderer suspending when memory coordinator is enabled. In follow-up CLs I'll add some tests to prevent this kind of regressions. BUG=675735,675811 Review-Url: https://codereview.chromium.org/2590073002 ========== to ========== Disable suspending renderer when memory coordinator is enabled Just calling SuspendRenderer() in OnMemoryStateChange() breaks many things. We need a reliable and consistent way to suspend renderers. Until we come up with a solution, disable renderer suspending when memory coordinator is enabled. In follow-up CLs I'll add some tests to prevent this kind of regressions. BUG=675735,675811 Committed: https://crrev.com/b1536fd7e1fa4dd7559e2c12e290e79cf949e9fe Cr-Commit-Position: refs/heads/master@{#439708} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/b1536fd7e1fa4dd7559e2c12e290e79cf949e9fe Cr-Commit-Position: refs/heads/master@{#439708}
Message was sent while issue was closed.
On 2016/12/20 02:38:00, bashi1 wrote: > On 2016/12/20 02:34:48, haraken wrote: > > LGTM > > > > I'm a bit concerned if SuspendRenderer would be causing the same issue though. > > Yes, I have the same concern. > > tasak@: Would you mind checking it (esp. tracing) ? I'm sorry, bashi-san. I was on vacation. I think, this change breaks PurgeAndSuspend finch experiment, because CC, and so on is not purged when suspended. So I created the patch: https://codereview.chromium.org/2595813002/. I think, the patch will fix this issue. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
