New issue
Advanced search Search tips

Issue 887223 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 1
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

OomIntervention triggered right after loading / reloading

Project Member Reported by yuzus@chromium.org, Sep 20

Issue description

Oom intervention feature gets triggered right after page navigation starts, because monitoring starts when garbage collection has not yet run on the page and memory usage is still high.
Monitoring should start after the loading finishes and gc runs.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a3be34098f115e8a6f9a91821de47e5fe8e8163e

commit a3be34098f115e8a6f9a91821de47e5fe8e8163e
Author: Yuzu Saijo <yuzus@chromium.org>
Date: Tue Sep 25 06:12:45 2018

Run page navigation gc when oom intervention is triggered on the previous page

When intervention runs and user navigates away from the page, it is likely that intervention runs
right after the navigation on the new page. This is a bug because it is not the new page's fault
that the memory usage is high, but the previous page's.

This CL makes page navigation gc run when intervention is triggered on the previous page, so that
the memory usage of two pages would not overlap in near-oom situation.

Bug:  887223 
Change-Id: I435331410e516d1c87756d84dbd6017d8fff0335
Reviewed-on: https://chromium-review.googlesource.com/1235854
Commit-Queue: Yuzu Saijo <yuzus@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593834}
[modify] https://crrev.com/a3be34098f115e8a6f9a91821de47e5fe8e8163e/third_party/blink/renderer/bindings/core/v8/v8_gc_for_context_dispose.cc
[modify] https://crrev.com/a3be34098f115e8a6f9a91821de47e5fe8e8163e/third_party/blink/renderer/bindings/core/v8/v8_gc_for_context_dispose.h
[modify] https://crrev.com/a3be34098f115e8a6f9a91821de47e5fe8e8163e/third_party/blink/renderer/controller/oom_intervention_impl.cc

Labels: Merge-Request-70 Merge-Request-71
We would like to request a merge of this change to beta.

This fix is critical in that currently about 20-30% of the times where OomIntervention is triggered, it gets re-triggered because of this bug.

But at the same time it would have a low risk of regression because the intervention feature is only triggered for 0.03% of the entire pageloads.

Currently finch experiment of OomIntervention is running for 50% beta and thus we would like to land this fix as soon as possible.
Thanks in advance and please let me know if there is any other rationale needed.
Project Member

Comment 3 by sheriffbot@chromium.org, Sep 25

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: We don't branch M71 until 2018-10-11.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 4 by sheriffbot@chromium.org, Sep 25

Labels: -Merge-Request-70 Merge-Review-70
This bug requires manual review: M70 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: benmason@chromium.org
Please let me know what steps we need to follow in order to merge this change. Thanks!
Please cherry pick the change into refs/branch-heads/3538 and then submit. Thanks!
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 27

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5f6d116fd93b0cc2d556b2bbd9c1c8f3f1d4928c

commit 5f6d116fd93b0cc2d556b2bbd9c1c8f3f1d4928c
Author: Yuzu Saijo <yuzus@chromium.org>
Date: Thu Sep 27 01:24:37 2018

Run page navigation gc when oom intervention is triggered on the previous page

When intervention runs and user navigates away from the page, it is likely that intervention runs
right after the navigation on the new page. This is a bug because it is not the new page's fault
that the memory usage is high, but the previous page's.

This CL makes page navigation gc run when intervention is triggered on the previous page, so that
the memory usage of two pages would not overlap in near-oom situation.

Bug:  887223 
Change-Id: I435331410e516d1c87756d84dbd6017d8fff0335
Reviewed-on: https://chromium-review.googlesource.com/1235854
Commit-Queue: Yuzu Saijo <yuzus@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#593834}(cherry picked from commit a3be34098f115e8a6f9a91821de47e5fe8e8163e)
Reviewed-on: https://chromium-review.googlesource.com/1247704
Reviewed-by: Yuzu Saijo <yuzus@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#696}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/5f6d116fd93b0cc2d556b2bbd9c1c8f3f1d4928c/third_party/blink/renderer/bindings/core/v8/v8_gc_for_context_dispose.cc
[modify] https://crrev.com/5f6d116fd93b0cc2d556b2bbd9c1c8f3f1d4928c/third_party/blink/renderer/bindings/core/v8/v8_gc_for_context_dispose.h
[modify] https://crrev.com/5f6d116fd93b0cc2d556b2bbd9c1c8f3f1d4928c/third_party/blink/renderer/controller/oom_intervention_impl.cc

Labels: CommitLog-Audit-Violation Merge-Without-Approval M-70
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 5f6d116fd93b0cc2d556b2bbd9c1c8f3f1d4928c was merged to refs/branch-heads/3538 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 
Labels: Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/5f6d116fd93b0cc2d556b2bbd9c1c8f3f1d4928c

Commit: 5f6d116fd93b0cc2d556b2bbd9c1c8f3f1d4928c
Author: yuzus@chromium.org
Commiter: yuzus@chromium.org
Date: 2018-09-27 01:24:37 +0000 UTC

Run page navigation gc when oom intervention is triggered on the previous page

When intervention runs and user navigates away from the page, it is likely that intervention runs
right after the navigation on the new page. This is a bug because it is not the new page's fault
that the memory usage is high, but the previous page's.

This CL makes page navigation gc run when intervention is triggered on the previous page, so that
the memory usage of two pages would not overlap in near-oom situation.

Bug:  887223 
Change-Id: I435331410e516d1c87756d84dbd6017d8fff0335
Reviewed-on: https://chromium-review.googlesource.com/1235854
Commit-Queue: Yuzu Saijo <yuzus@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#593834}(cherry picked from commit a3be34098f115e8a6f9a91821de47e5fe8e8163e)
Reviewed-on: https://chromium-review.googlesource.com/1247704
Reviewed-by: Yuzu Saijo <yuzus@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#696}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
Labels: -CommitLog-Audit-Violation -Merge-Without-Approval
I thought I'd added the merge approved labels on this when discussing. Looks like I didn't.
Status: Fixed (was: Assigned)
Labels: -Hotlist-Merge-Review -Merge-Review-70 -Merge-Review-71

Sign in to add a comment