Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1749263003:
Don't block 3D apis if GPU process shut down was expected (Closed)
|
Created:
4 years, 9 months ago by no sievers Modified:
4 years, 9 months ago CC:
chromium-reviews, darin-cc_chromium.org, jam, piman+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@ctxrestore Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDon't block 3D apis if GPU process shut down was expected
This avoids raising the webgl infobar which forces a reload, when
the GPU process got likely OOM killed, which we unfortunately
can only infer from the fact that we were in the background.
Note that on Android, NORMAL_TERMINATION here implies that the
entire application was in the background.
So that we don't have webgl context restore fight with
the OOM killer, this also requires
https://codereview.chromium.org/1756633002/.
BUG=586905
NOTRY=True
Committed: https://crrev.com/4592c3b9a493db42854e3c706fad7f7a18ad2d18
Cr-Commit-Position: refs/heads/master@{#380468}
Patch Set 1 #
Total comments: 2
Patch Set 2 : remove ifdef #
Depends on Patchset: Messages
Total messages: 26 (15 generated)
Description was changed from ========== Android: Don't block 3D apis if GPU process was killed in background. This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that NORMAL_TERMINATION here implies that the entire application was in the background. BUG=586905 ========== to ========== Android: Don't block 3D apis if GPU process was killed in background. This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that NORMAL_TERMINATION here implies that the entire application was in the background. BUG=586905 ==========
sievers@chromium.org changed reviewers: + kbr@chromium.org
Description was changed from ========== Android: Don't block 3D apis if GPU process was killed in background. This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that NORMAL_TERMINATION here implies that the entire application was in the background. BUG=586905 ========== to ========== Android: Don't block 3D apis if GPU process was killed in background. This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 ==========
Description was changed from ========== Android: Don't block 3D apis if GPU process was killed in background. This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 ========== to ========== Android: Don't block 3D apis if GPU process was killed in background. This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 ==========
ptal
sievers@chromium.org changed reviewers: + piman@chromium.org
https://codereview.chromium.org/1749263003/diff/1/content/browser/gpu/gpu_pro... File content/browser/gpu/gpu_process_host.cc (right): https://codereview.chromium.org/1749263003/diff/1/content/browser/gpu/gpu_pro... content/browser/gpu/gpu_process_host.cc:489: #if defined(OS_ANDROID) I'm wondering if this even needs the ifdef Android. Also, CrOS also has an OOM killer right? +piman
LGTM if this has been tested. Thanks for digging into this. https://codereview.chromium.org/1749263003/diff/1/content/browser/gpu/gpu_pro... File content/browser/gpu/gpu_process_host.cc (right): https://codereview.chromium.org/1749263003/diff/1/content/browser/gpu/gpu_pro... content/browser/gpu/gpu_process_host.cc:489: #if defined(OS_ANDROID) On 2016/03/01 23:04:34, sievers wrote: > I'm wondering if this even needs the ifdef Android. You're right, it's probably not needed. The only time this code path tends to be taken on other platforms, as far as I know, is when about:gpu is visited and that triggers a launch of a temporary GPU process for information gathering purposes. > Also, CrOS also has an OOM killer right? I'm not sure CrOS would ever kill the GPU process though. The entire UI is predicated on the GPU process staying alive.
On Tue, Mar 1, 2016 at 7:48 PM, <kbr@chromium.org> wrote: > LGTM if this has been tested. Thanks for digging into this. > > > > > https://codereview.chromium.org/1749263003/diff/1/content/browser/gpu/gpu_pro... > File content/browser/gpu/gpu_process_host.cc (right): > > > https://codereview.chromium.org/1749263003/diff/1/content/browser/gpu/gpu_pro... > content/browser/gpu/gpu_process_host.cc:489: #if defined(OS_ANDROID) > On 2016/03/01 23:04:34, sievers wrote: > > I'm wondering if this even needs the ifdef Android. > > You're right, it's probably not needed. The only time this code path > tends to be taken on other platforms, as far as I know, is when > about:gpu is visited and that triggers a launch of a temporary GPU > process for information gathering purposes. > > > Also, CrOS also has an OOM killer right? > > I'm not sure CrOS would ever kill the GPU process though. The entire UI > is predicated on the GPU process staying alive. > Chrome OS has an OOM killer in the kernel, though we tune the OOM priority, and I'm pretty sure we try to keep the GPU process over, say, renderers, so by the time we kill the GPU process (extremely rare), we're probably in a bad state overall (and yes, it's needed for the UI). I guess it /could/ happen in theory, but just triggering it would be extremely difficult. I'd rather do without the #ifdef. There's a desire to terminate the GPU process in certain cases (e.g. chrome is in background mode on desktop, when all tabs are closed but maybe extensions are running). > > https://codereview.chromium.org/1749263003/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
I removed the ifdef and updated the comment.
Description was changed from ========== Android: Don't block 3D apis if GPU process was killed in background. This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 ========== to ========== Don't block 3D apis if GPU process shut down was expected This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 ==========
Description was changed from ========== Don't block 3D apis if GPU process shut down was expected This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 ========== to ========== Don't block 3D apis if GPU process shut down was expected This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that on Android, NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 ==========
Description was changed from ========== Don't block 3D apis if GPU process shut down was expected This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that on Android, NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 ========== to ========== Don't block 3D apis if GPU process shut down was expected This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that on Android, NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 ==========
lgtm
The CQ bit was checked by sievers@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org Link to the patchset: https://codereview.chromium.org/1749263003/#ps20001 (title: "remove ifdef")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1749263003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1749263003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Description was changed from ========== Don't block 3D apis if GPU process shut down was expected This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that on Android, NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 ========== to ========== Don't block 3D apis if GPU process shut down was expected This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that on Android, NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 NOTRY=True ==========
The CQ bit was checked by sievers@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1749263003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1749263003/20001
Message was sent while issue was closed.
Description was changed from ========== Don't block 3D apis if GPU process shut down was expected This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that on Android, NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 NOTRY=True ========== to ========== Don't block 3D apis if GPU process shut down was expected This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that on Android, NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 NOTRY=True ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Don't block 3D apis if GPU process shut down was expected This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that on Android, NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 NOTRY=True ========== to ========== Don't block 3D apis if GPU process shut down was expected This avoids raising the webgl infobar which forces a reload, when the GPU process got likely OOM killed, which we unfortunately can only infer from the fact that we were in the background. Note that on Android, NORMAL_TERMINATION here implies that the entire application was in the background. So that we don't have webgl context restore fight with the OOM killer, this also requires https://codereview.chromium.org/1756633002/. BUG=586905 NOTRY=True Committed: https://crrev.com/4592c3b9a493db42854e3c706fad7f7a18ad2d18 Cr-Commit-Position: refs/heads/master@{#380468} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/4592c3b9a493db42854e3c706fad7f7a18ad2d18 Cr-Commit-Position: refs/heads/master@{#380468} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
