New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 828363 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocked on:
issue 850271

Blocking:
issue 835353



Sign in to add a comment

WebGL flickering when using anti aliasing on Chromium on Samsung S series

Reported by re...@binkies3d.com, Apr 3 2018

Issue description

Steps to reproduce the problem:
1. Go to www.hotspot3d.com
2. Drag the phones around and observe parts of the phone are not drawn completely (you see the white background)

What is the expected behavior?
Phones should be drawn normally.

What went wrong?
NOTE: this only happens on Chrome browser on the Samsung Galaxy S5, S6, S7 and S8 series. Does not happen on other browsers on the same devices and does not happen for example on the Pixel phones (tried with Browserstack).

What happens is that randomly parts of the geometry are not completely drawn (looks like this) and you can see the background instead. Problem seems related to anti aliasing: when this is disabled the problem disappears as well.

Images show a properly drawn frame and a frame that is bad (at the same orientation).

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 65.0.3325.109  Channel: stable
OS Version: 7.0
Flash Version: n.a.
 
Samsung Galaxy S8 Good.jpg
42.5 KB View Download
Samsung Galaxy S8 Bad.jpg
42.6 KB View Download

Comment 1 by kbr@chromium.org, Apr 3 2018

Cc: ericrk@chromium.org enne@chromium.org
Owner: jdarpinian@chromium.org
James, could you please triage this? Hopefully we have one of these devices in the cabinet.

May be related to recent changes Qualcomm made in https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_multisampled_render_to_texture2.txt where they were taking a strict interpretation of the spec and losing the contents of the depth buffer at inopportune times. In a separate email thread, Qualcomm already agreed to revert this behavior, because it was impossible for Chrome to avoid switching contexts and thereby avoid the flush of the tile memory.

The only workaround until a new driver ships is to either disable antialiasing at the application level or add a blacklist entry for WebGL's use of EXT_multisampled_render_to_texture. I'm not sure whether Chrome's compositing or GPU rasterization wants that extension to be present.

Comment 2 by kbr@chromium.org, Apr 3 2018

Labels: -Arch-x86_64 Arch-ARM
I can repro. It's specific to the international versions of the S series, using Mali. Does Mali have the same problem?

I will try another Mali device.
Labels: GPU-ARM
Status: Assigned (was: Unconfirmed)
I can also repro on a Moto M, so it does seem Mali specific. I also noticed that it doesn't happen in 57, but it does in 65.

Comment 6 Deleted

Thanks for the diligent work so far! From a developer point-of-view I would like to point out that anti aliasing really improves the visual quality so is quite important to keep if possible.

Not sure if this is helpful but we are drawing directly to the backbuffer (and not a render target).

Comment 8 by re...@binkies3d.com, Apr 10 2018

Is there any news on this topic?
The cause is Chrome switching GL contexts in the middle of a WebGL frame, at which point the Mali driver resolves the multisampled depth buffer to a single sample depth buffer. This loses information which can't be perfectly restored when Chrome switches back to the WebGL context to finish rendering the frame. 

In general we cannot guarantee that GL contexts will never be switched in the middle of a WebGL frame in all circumstances. However, in this instance the switching is unnecessary. I plan to make a change that will remove the unnecessary switching and fix this case.
I have a WIP change that removes some of the unnecessary context switching. However, there are more sources of unnecessary context switching than I first thought, and I will need to do more debugging to discover them all.

https://chromium-review.googlesource.com/c/chromium/src/+/1013377

I have to put this aside for now, but I plan to come back to it in a few weeks.
Cc: sunn...@chromium.org
Cc: piman@chromium.org
piman: jdarpinian's CL makes WebGL priority equal to UI (on top of other changes in cmd_buffer_helper). We obviously don't want to ship that. We should revisit the scheduling logic, and change it so that we don't deschedule contexts so eagerly e.g. only deschedule other contexts if UI work has been waiting for a long time (say 20ms or so).

Comment 13 by piman@chromium.org, Apr 16 2018

Yeah, that's sort of similar to the original preemption code, where UI would only preempt after being unscheduled for 16ms or so. Probably some tuning to do.
Blocking: 835353
Project Member

Comment 15 by bugdroid1@chromium.org, May 31 2018

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

commit 62ca13ce4a8fda4f316692596f9bf61c74a58e34
Author: James Darpinian <jdarpinian@chromium.org>
Date: Thu May 31 04:12:06 2018

GPU: Make command buffer autoflush more conservative.

This is change #1 of 3 reducing context switching during WebGL
rendering. On tiled rendering GPUs context switches are slow and can
even cause multisampling artifacts. Every time the command buffer is
flushed, a context switch is possible.

Before, the command buffer helper would always autoflush eventually if
no explicit flush was received. Commands would accumulate across
multiple frames until the autoflush threshold was reached. Implicit
flushes caused by ordering barriers were not taken into account.

Now the autoflush decision is based on the last ordering barrier instead
of the last explicit flush. Since there's an ordering barrier at least
once a frame, autoflush will only happen if the threshold is reached
within a single frame.

Bug: 828363,835353
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I168be6892620b1813c598a912e59147f9c45896f
Reviewed-on: https://chromium-review.googlesource.com/1077435
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563149}
[modify] https://crrev.com/62ca13ce4a8fda4f316692596f9bf61c74a58e34/gpu/command_buffer/client/cmd_buffer_helper.cc
[modify] https://crrev.com/62ca13ce4a8fda4f316692596f9bf61c74a58e34/gpu/command_buffer/client/cmd_buffer_helper_test.cc

Project Member

Comment 16 by bugdroid1@chromium.org, May 31 2018

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

commit dcfaac01164977c0e9862880ba33ba6edb5639c2
Author: James Darpinian <jdarpinian@chromium.org>
Date: Thu May 31 22:29:56 2018

GPU: Remove transfer buffer autoflushing.

This is change #2 of 3 reducing context switching during WebGL
rendering. On tiled rendering GPUs context switches are slow and can
even cause multisampling artifacts. Every time the command buffer is
flushed, a context switch is possible.

Before, the transfer buffer would flush the command buffer after an
arbitrary number of bytes were sent. This change removes that threshold.
Any performance benefit of early flushing is killed by the additional
context switching.

Bug: 828363,835353
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I8f71668d4b976332820d290868282d41286ab4fe
Reviewed-on: https://chromium-review.googlesource.com/1077568
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563404}
[modify] https://crrev.com/dcfaac01164977c0e9862880ba33ba6edb5639c2/gpu/command_buffer/client/implementation_base.cc
[modify] https://crrev.com/dcfaac01164977c0e9862880ba33ba6edb5639c2/gpu/command_buffer/client/implementation_base.h
[modify] https://crrev.com/dcfaac01164977c0e9862880ba33ba6edb5639c2/gpu/command_buffer/client/mock_transfer_buffer.cc
[modify] https://crrev.com/dcfaac01164977c0e9862880ba33ba6edb5639c2/gpu/command_buffer/client/mock_transfer_buffer.h
[modify] https://crrev.com/dcfaac01164977c0e9862880ba33ba6edb5639c2/gpu/command_buffer/client/transfer_buffer.cc
[modify] https://crrev.com/dcfaac01164977c0e9862880ba33ba6edb5639c2/gpu/command_buffer/client/transfer_buffer.h
[modify] https://crrev.com/dcfaac01164977c0e9862880ba33ba6edb5639c2/gpu/command_buffer/client/transfer_buffer_unittest.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Jun 6 2018

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

commit b63225bbef75dc91b78029ad8f8b1e516934b268
Author: James Darpinian <jdarpinian@chromium.org>
Date: Wed Jun 06 19:45:58 2018

Revert "GPU: Remove transfer buffer autoflushing."

This reverts commit dcfaac01164977c0e9862880ba33ba6edb5639c2.

Reason for revert: Regressed canvas 2d performance in some MotionMark
tests. Will redo as a WebGL-specific change.

Original change's description:
> GPU: Remove transfer buffer autoflushing.
> 
> This is change #2 of 3 reducing context switching during WebGL
> rendering. On tiled rendering GPUs context switches are slow and can
> even cause multisampling artifacts. Every time the command buffer is
> flushed, a context switch is possible.
> 
> Before, the transfer buffer would flush the command buffer after an
> arbitrary number of bytes were sent. This change removes that threshold.
> Any performance benefit of early flushing is killed by the additional
> context switching.
> 
> Bug: 828363,835353
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: I8f71668d4b976332820d290868282d41286ab4fe
> Reviewed-on: https://chromium-review.googlesource.com/1077568
> Commit-Queue: James Darpinian <jdarpinian@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#563404}

TBR=piman@chromium.org,jdarpinian@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 828363, 835353,  849348 
Change-Id: Iba27bae33f5395333f54db4bea4e79e41cca7e31
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1087370
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565006}
[modify] https://crrev.com/b63225bbef75dc91b78029ad8f8b1e516934b268/gpu/command_buffer/client/implementation_base.cc
[modify] https://crrev.com/b63225bbef75dc91b78029ad8f8b1e516934b268/gpu/command_buffer/client/implementation_base.h
[modify] https://crrev.com/b63225bbef75dc91b78029ad8f8b1e516934b268/gpu/command_buffer/client/mock_transfer_buffer.cc
[modify] https://crrev.com/b63225bbef75dc91b78029ad8f8b1e516934b268/gpu/command_buffer/client/mock_transfer_buffer.h
[modify] https://crrev.com/b63225bbef75dc91b78029ad8f8b1e516934b268/gpu/command_buffer/client/transfer_buffer.cc
[modify] https://crrev.com/b63225bbef75dc91b78029ad8f8b1e516934b268/gpu/command_buffer/client/transfer_buffer.h
[modify] https://crrev.com/b63225bbef75dc91b78029ad8f8b1e516934b268/gpu/command_buffer/client/transfer_buffer_unittest.cc

Comment 18 by kbr@chromium.org, Jun 6 2018

Note: the test case from Issue 831740 demonstrated this regression. That test case first uploads an ImageBitmap to a WebGL texture, and then creates a typed array and does the same. With the transfer buffer auto-flushing removed, the second texture upload takes a very long time.

I think that if we solved this case in some other way than relying on transfer buffer auto-flushing that we could reland the patch without making it WebGL-specific, and avoid the large regressions in the benchmarks. It's pretty small and simple and should be easily diagnosable.

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=850271 for this, if anyone wants to look into it.
Blockedon: 850271
Project Member

Comment 21 by bugdroid1@chromium.org, Jun 15 2018

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

commit 7a59e08137d5c51fa6695fe8ce0557ee257f4a8e
Author: James Darpinian <jdarpinian@chromium.org>
Date: Fri Jun 15 00:19:53 2018

GPU: Don't flush when destroying transfer buffers.

Destroying a transfer buffer now requires only an ordering barrier, not
a full flush. This removes a source of unnecessary flushes and makes
resizing the transfer buffer more efficient.

Bug:  850271 , 835353, 828363
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I9bfe43c46c00f8e9fc29e6450da788f86e74bc52
Reviewed-on: https://chromium-review.googlesource.com/1093580
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567494}
[modify] https://crrev.com/7a59e08137d5c51fa6695fe8ce0557ee257f4a8e/gpu/command_buffer/client/cmd_buffer_helper.cc
[modify] https://crrev.com/7a59e08137d5c51fa6695fe8ce0557ee257f4a8e/gpu/command_buffer/client/cmd_buffer_helper_test.cc
[modify] https://crrev.com/7a59e08137d5c51fa6695fe8ce0557ee257f4a8e/gpu/command_buffer/client/mapped_memory.cc
[modify] https://crrev.com/7a59e08137d5c51fa6695fe8ce0557ee257f4a8e/gpu/command_buffer/client/transfer_buffer.cc
[modify] https://crrev.com/7a59e08137d5c51fa6695fe8ce0557ee257f4a8e/gpu/command_buffer/client/transfer_buffer_unittest.cc
[modify] https://crrev.com/7a59e08137d5c51fa6695fe8ce0557ee257f4a8e/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/7a59e08137d5c51fa6695fe8ce0557ee257f4a8e/gpu/ipc/client/gpu_channel_host.cc
[modify] https://crrev.com/7a59e08137d5c51fa6695fe8ce0557ee257f4a8e/gpu/ipc/client/gpu_channel_host.h
[modify] https://crrev.com/7a59e08137d5c51fa6695fe8ce0557ee257f4a8e/gpu/ipc/common/flush_params.h
[modify] https://crrev.com/7a59e08137d5c51fa6695fe8ce0557ee257f4a8e/gpu/ipc/common/gpu_param_traits_macros.h
[modify] https://crrev.com/7a59e08137d5c51fa6695fe8ce0557ee257f4a8e/gpu/ipc/service/gpu_channel.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Jun 15 2018

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

commit 5a49da30d841e14aaabe40748de574908c3f6c58
Author: Francois Doray <fdoray@chromium.org>
Date: Fri Jun 15 14:04:36 2018

Revert "GPU: Don't flush when destroying transfer buffers."

This reverts commit 7a59e08137d5c51fa6695fe8ce0557ee257f4a8e.

Reason for revert:  https://crbug.com/853194 

Original change's description:
> GPU: Don't flush when destroying transfer buffers.
>
> Destroying a transfer buffer now requires only an ordering barrier, not
> a full flush. This removes a source of unnecessary flushes and makes
> resizing the transfer buffer more efficient.
>
> Bug:  850271 , 835353, 828363
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: I9bfe43c46c00f8e9fc29e6450da788f86e74bc52
> Reviewed-on: https://chromium-review.googlesource.com/1093580
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
> Commit-Queue: James Darpinian <jdarpinian@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#567494}

TBR=sunnyps@chromium.org,rsesek@chromium.org,piman@chromium.org,jdarpinian@chromium.org

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  850271 , 835353, 828363
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I7cd21d6cee9beca02ae28da8614a58e2f53dc848
Reviewed-on: https://chromium-review.googlesource.com/1102346
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567631}
[modify] https://crrev.com/5a49da30d841e14aaabe40748de574908c3f6c58/gpu/command_buffer/client/cmd_buffer_helper.cc
[modify] https://crrev.com/5a49da30d841e14aaabe40748de574908c3f6c58/gpu/command_buffer/client/cmd_buffer_helper_test.cc
[modify] https://crrev.com/5a49da30d841e14aaabe40748de574908c3f6c58/gpu/command_buffer/client/mapped_memory.cc
[modify] https://crrev.com/5a49da30d841e14aaabe40748de574908c3f6c58/gpu/command_buffer/client/transfer_buffer.cc
[modify] https://crrev.com/5a49da30d841e14aaabe40748de574908c3f6c58/gpu/command_buffer/client/transfer_buffer_unittest.cc
[modify] https://crrev.com/5a49da30d841e14aaabe40748de574908c3f6c58/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/5a49da30d841e14aaabe40748de574908c3f6c58/gpu/ipc/client/gpu_channel_host.cc
[modify] https://crrev.com/5a49da30d841e14aaabe40748de574908c3f6c58/gpu/ipc/client/gpu_channel_host.h
[modify] https://crrev.com/5a49da30d841e14aaabe40748de574908c3f6c58/gpu/ipc/common/flush_params.h
[modify] https://crrev.com/5a49da30d841e14aaabe40748de574908c3f6c58/gpu/ipc/common/gpu_param_traits_macros.h
[modify] https://crrev.com/5a49da30d841e14aaabe40748de574908c3f6c58/gpu/ipc/service/gpu_channel.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Jun 19 2018

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

commit d1b4ae2892b05cf622b3f649cb38b995ed2d1733
Author: James Darpinian <jdarpinian@chromium.org>
Date: Tue Jun 19 19:49:08 2018

GPU: Don't flush when destroying transfer buffers.

This reverts commit 5a49da30d841e14aaabe40748de574908c3f6c58.

2nd try due to  http://crbug.com/853194 
1st try was here: https://chromium-review.googlesource.com/c/chromium/src/+/1093580

Destroying a transfer buffer now requires only an ordering barrier, not
a full flush. This removes a source of unnecessary flushes and makes
resizing the transfer buffer more efficient.

Bug:  850271 , 835353, 828363,  853194 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ibc5e72fcf4538a3f10022a613e9d0f15e4e7a95a
Reviewed-on: https://chromium-review.googlesource.com/1105466
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568575}
[modify] https://crrev.com/d1b4ae2892b05cf622b3f649cb38b995ed2d1733/gpu/command_buffer/client/cmd_buffer_helper.cc
[modify] https://crrev.com/d1b4ae2892b05cf622b3f649cb38b995ed2d1733/gpu/command_buffer/client/cmd_buffer_helper_test.cc
[modify] https://crrev.com/d1b4ae2892b05cf622b3f649cb38b995ed2d1733/gpu/command_buffer/client/mapped_memory.cc
[modify] https://crrev.com/d1b4ae2892b05cf622b3f649cb38b995ed2d1733/gpu/command_buffer/client/transfer_buffer.cc
[modify] https://crrev.com/d1b4ae2892b05cf622b3f649cb38b995ed2d1733/gpu/command_buffer/client/transfer_buffer_unittest.cc
[modify] https://crrev.com/d1b4ae2892b05cf622b3f649cb38b995ed2d1733/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/d1b4ae2892b05cf622b3f649cb38b995ed2d1733/gpu/ipc/client/gpu_channel_host.cc
[modify] https://crrev.com/d1b4ae2892b05cf622b3f649cb38b995ed2d1733/gpu/ipc/client/gpu_channel_host.h
[modify] https://crrev.com/d1b4ae2892b05cf622b3f649cb38b995ed2d1733/gpu/ipc/common/flush_params.h
[modify] https://crrev.com/d1b4ae2892b05cf622b3f649cb38b995ed2d1733/gpu/ipc/common/gpu_param_traits_macros.h
[modify] https://crrev.com/d1b4ae2892b05cf622b3f649cb38b995ed2d1733/gpu/ipc/service/gpu_channel.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Jul 3

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

commit c9afd79a9b8dddba6169dd805f60d8f4b14b82c7
Author: James Darpinian <jdarpinian@chromium.org>
Date: Tue Jul 03 01:31:40 2018

GPU: Fix transfer buffers not being freed promptly

A previous change removed the command buffer flush when destroying a
transfer buffer:
https://chromium-review.googlesource.com/c/chromium/src/+/1105466

If we want to free a transfer buffer promptly, we now need to flush
separately, and we do. Unfortunately if no commands were issued between
the call to DestroyTransferBuffer and the flush, the flush might not
execute because the CommandBufferProxy would pass an outdated flush ID
to GpuChannelHost::EnsureFlush. The fix is to inform the
CommandBufferProxy about the new flush ID when it calls
DestroyTransferBuffer.

This should fix the memory regressions on the perf waterfall noted in
bug 855402.

Bug: 855402,  850271 , 835353, 828363
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Id125c5124b26763e692c1fe8067df28761fa6dab
Reviewed-on: https://chromium-review.googlesource.com/1121590
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572074}
[modify] https://crrev.com/c9afd79a9b8dddba6169dd805f60d8f4b14b82c7/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/c9afd79a9b8dddba6169dd805f60d8f4b14b82c7/gpu/ipc/client/gpu_channel_host.cc
[modify] https://crrev.com/c9afd79a9b8dddba6169dd805f60d8f4b14b82c7/gpu/ipc/client/gpu_channel_host.h

Project Member

Comment 25 by bugdroid1@chromium.org, Jul 3

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

commit 65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783
Author: James Darpinian <jdarpinian@chromium.org>
Date: Tue Jul 03 07:23:17 2018

GPU: Grow and shrink transfer buffer

Automatically grow the transfer buffer when it is full and shrink it
when the full capacity is not being used. Allow choosing larger and
smaller sizes than before.

Before we would only grow it when a single transfer request was larger
than the whole buffer; requests smaller than the whole buffer would just
block if the buffer was full. Also we would not ever shrink the buffer
until someone called Free() manually.

Bug:  850271 , 835353, 828363,  856347 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: If2f2235a1d5297c63663398b37e1e30791347a3e
Reviewed-on: https://chromium-review.googlesource.com/1105505
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572135}
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/client_test_helper.cc
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/client_test_helper.h
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/gles2_implementation.cc
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/implementation_base.cc
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/implementation_base.h
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/mock_transfer_buffer.cc
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/mock_transfer_buffer.h
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/ring_buffer.h
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/shared_memory_limits.h
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/transfer_buffer.cc
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/transfer_buffer.h
[modify] https://crrev.com/65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783/gpu/command_buffer/client/transfer_buffer_unittest.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Jul 3

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

commit 6df2efa5313f48302a51d510fa9b5983811c2328
Author: Brian Sheedy <bsheedy@chromium.org>
Date: Tue Jul 03 17:42:32 2018

Revert "GPU: Grow and shrink transfer buffer"

This reverts commit 65c38f54b0ae666e8b27bfa5aef6d3ab92d4a783.

Reason for revert: Causing DCHECK failures in VR instrumentation tests

Original change's description:
> GPU: Grow and shrink transfer buffer
> 
> Automatically grow the transfer buffer when it is full and shrink it
> when the full capacity is not being used. Allow choosing larger and
> smaller sizes than before.
> 
> Before we would only grow it when a single transfer request was larger
> than the whole buffer; requests smaller than the whole buffer would just
> block if the buffer was full. Also we would not ever shrink the buffer
> until someone called Free() manually.
> 
> Bug:  850271 , 835353, 828363,  856347 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: If2f2235a1d5297c63663398b37e1e30791347a3e
> Reviewed-on: https://chromium-review.googlesource.com/1105505
> Commit-Queue: James Darpinian <jdarpinian@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#572135}

TBR=jdarpinian@chromium.org,sunnyps@chromium.org,piman@chromium.org

Change-Id: I031b30234cdcfbd8a4e48c7ebb6fd23312ec10ed
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  850271 , 835353, 828363,  856347 ,  859952 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1124822
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572277}
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/client_test_helper.cc
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/client_test_helper.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/gles2_implementation.cc
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/implementation_base.cc
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/implementation_base.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/mock_transfer_buffer.cc
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/mock_transfer_buffer.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/ring_buffer.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/shared_memory_limits.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/transfer_buffer.cc
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/transfer_buffer.h
[modify] https://crrev.com/6df2efa5313f48302a51d510fa9b5983811c2328/gpu/command_buffer/client/transfer_buffer_unittest.cc

Cc: jdarpinian@chromium.org
Owner: kbr@chromium.org
Taking over from James while he's OOO.

Cc: kbr@chromium.org
Owner: jdarpinian@chromium.org
James isn't OOO yet.

Project Member

Comment 29 by bugdroid1@chromium.org, Jul 13

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

commit 1c3333c5acf70c841a66feba15273b1dc7d76da0
Author: James Darpinian <jdarpinian@chromium.org>
Date: Fri Jul 13 01:56:36 2018

GPU: Grow and shrink transfer buffer

Second try after revert 6df2efa5313f48302a51d510fa9b5983811c2328.
Original at https://crrev.com/c/1105505

Automatically grow the transfer buffer when it is full and shrink it
when the full capacity is not being used. Start at a smaller size.

Before we would only grow it when a single transfer request was larger
than the whole buffer; requests smaller than the whole buffer would just
block if the buffer was full. Also we would not ever shrink the buffer
until someone called Free() manually.

Bug:  850271 , 835353, 828363,  856347 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Id1223267d0ab444afe18f62f45ae5f365131e18c
Reviewed-on: https://chromium-review.googlesource.com/1125503
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574816}
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/client_test_helper.cc
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/client_test_helper.h
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/gles2_implementation.cc
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/implementation_base.cc
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/implementation_base.h
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/mock_transfer_buffer.cc
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/mock_transfer_buffer.h
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/ring_buffer.h
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/shared_memory_limits.h
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/transfer_buffer.cc
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/transfer_buffer.h
[modify] https://crrev.com/1c3333c5acf70c841a66feba15273b1dc7d76da0/gpu/command_buffer/client/transfer_buffer_unittest.cc

Sign in to add a comment