Issue metadata
Sign in to add a comment
|
OOP-D Scheduling.Renderer.DrawInterval2 on Mac |
||||||||||||||||||||||
Issue descriptionScheduling.Renderer.DrawInterval2 has regressed with OOP-D enabled on Mac. https://uma.googleplex.com/p/chrome/variations/?sid=f151fabd8279f3c7a59e4e96d3488341 On most platforms the metric improves slightly in the 75th+ percentile range with OOP-D enabled. That was previously the case for mac as well but something changed. This graph shows the switch happened sometime after 70.0.3538.67 beta was released: https://uma.googleplex.com/p/chrome/timeline_v2/?sid=6a4a930c03c5885b4c43b8e297d18ed3 I think this might be related to https://crrev.com/c/1229189. It was merged back into 70.0.3538.67. That CL originally landed in https://crrev.com/c/1229189 which was in the 71.0.3559.6 dev release. I see a similar uptick at 71.0.3559.6 this graph: https://uma.googleplex.com/p/chrome/timeline_v2/?sid=1eeef1b2aaefad68ea2cde71158dc9a2
,
Nov 8
,
Nov 14
I see that this caused the spike in https://crbug.com/901559. It makes sense that switching from a synthetic 60fps vsync signal to the real signal could cause a change in Scheduling.Renderer.DrawInterval2. I don't understood why there is a regression with OOP-D compared to non OOP-D though. Both should be using the real vsync timing now.
,
Nov 15
I think there is a bug in InProcessCommandBuffer. We're updating vsync parameters when we shouldn't be.
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d2b1aadb7232344d8a8fc90569ab3df3e3bc7351 commit d2b1aadb7232344d8a8fc90569ab3df3e3bc7351 Author: kylechar <kylechar@chromium.org> Date: Thu Nov 15 20:35:12 2018 Fix InProcessCommandBuffer handling of buffer presented. There was some incorrect logic in InProcessCommandBuffer. Presentation feedback was getting sent to the GpuControlClient even if it didn't request it. Worse yet, vsync timing parameters were getting updated if presentation feedback was requested, even if the presentation feedback didn't contain accurate vsync information. On mac with OOP-D this was causing the real vsync timebase provided by the browser to get overridden with junk. Mac doesn't have accurate presentation feedback, instead it's just base::TimeTicks::Now() for when swap buffers completed. Share the existing logic found in GLES2CommandBufferStub and CommandBufferProxyImpl with InProcessCommandBuffer to avoid future problems. Bug: 900564 Change-Id: Ib1bdb822b2357a905a923a537d4da7bf01ad0a25 Reviewed-on: https://chromium-review.googlesource.com/c/1337638 Commit-Queue: kylechar <kylechar@chromium.org> Reviewed-by: Jonathan Backer <backer@chromium.org> Cr-Commit-Position: refs/heads/master@{#608498} [modify] https://crrev.com/d2b1aadb7232344d8a8fc90569ab3df3e3bc7351/gpu/command_buffer/common/BUILD.gn [add] https://crrev.com/d2b1aadb7232344d8a8fc90569ab3df3e3bc7351/gpu/command_buffer/common/presentation_feedback_utils.cc [add] https://crrev.com/d2b1aadb7232344d8a8fc90569ab3df3e3bc7351/gpu/command_buffer/common/presentation_feedback_utils.h [modify] https://crrev.com/d2b1aadb7232344d8a8fc90569ab3df3e3bc7351/gpu/ipc/client/command_buffer_proxy_impl.cc [modify] https://crrev.com/d2b1aadb7232344d8a8fc90569ab3df3e3bc7351/gpu/ipc/in_process_command_buffer.cc [modify] https://crrev.com/d2b1aadb7232344d8a8fc90569ab3df3e3bc7351/gpu/ipc/service/gles2_command_buffer_stub.cc
,
Nov 15
This needs to get merged into M71 for the next beta assuming it doesn't cause any issues. Will check on Monday.
,
Nov 19
The NextAction date has arrived: 2018-11-19
,
Nov 19
Merge request #5 to M71 (hopefully in time for the beta release tomorrow?). The logic change is in InProcessCommandBuffer code which is only used with the VizCompositorThread experiment enabled, so if this causes issues we can turn off the experiment.
,
Nov 19
This bug requires manual review: M71 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), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 19
Approving merge to M71 branch 3578 based on comment #8. Pls merge ASAP so we can pick it up for tomororw's besta release, cutting RC soon,Thank you.
,
Nov 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7dc1c2a787fc714b61c0ba096316ec7d11472c03 commit 7dc1c2a787fc714b61c0ba096316ec7d11472c03 Author: kylechar <kylechar@chromium.org> Date: Mon Nov 19 16:10:51 2018 Fix InProcessCommandBuffer handling of buffer presented. There was some incorrect logic in InProcessCommandBuffer. Presentation feedback was getting sent to the GpuControlClient even if it didn't request it. Worse yet, vsync timing parameters were getting updated if presentation feedback was requested, even if the presentation feedback didn't contain accurate vsync information. On mac with OOP-D this was causing the real vsync timebase provided by the browser to get overridden with junk. Mac doesn't have accurate presentation feedback, instead it's just base::TimeTicks::Now() for when swap buffers completed. Share the existing logic found in GLES2CommandBufferStub and CommandBufferProxyImpl with InProcessCommandBuffer to avoid future problems. Bug: 900564 Change-Id: Ib1bdb822b2357a905a923a537d4da7bf01ad0a25 Reviewed-on: https://chromium-review.googlesource.com/c/1337638 Commit-Queue: kylechar <kylechar@chromium.org> Reviewed-by: Jonathan Backer <backer@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#608498}(cherry picked from commit d2b1aadb7232344d8a8fc90569ab3df3e3bc7351) Reviewed-on: https://chromium-review.googlesource.com/c/1341589 Reviewed-by: kylechar <kylechar@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#750} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/7dc1c2a787fc714b61c0ba096316ec7d11472c03/gpu/command_buffer/common/BUILD.gn [add] https://crrev.com/7dc1c2a787fc714b61c0ba096316ec7d11472c03/gpu/command_buffer/common/presentation_feedback_utils.cc [add] https://crrev.com/7dc1c2a787fc714b61c0ba096316ec7d11472c03/gpu/command_buffer/common/presentation_feedback_utils.h [modify] https://crrev.com/7dc1c2a787fc714b61c0ba096316ec7d11472c03/gpu/ipc/client/command_buffer_proxy_impl.cc [modify] https://crrev.com/7dc1c2a787fc714b61c0ba096316ec7d11472c03/gpu/ipc/in_process_command_buffer.cc [modify] https://crrev.com/7dc1c2a787fc714b61c0ba096316ec7d11472c03/gpu/ipc/service/gles2_command_buffer_stub.cc
,
Nov 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7dc1c2a787fc714b61c0ba096316ec7d11472c03 Commit: 7dc1c2a787fc714b61c0ba096316ec7d11472c03 Author: kylechar@chromium.org Commiter: kylechar@chromium.org Date: 2018-11-19 16:10:51 +0000 UTC Fix InProcessCommandBuffer handling of buffer presented. There was some incorrect logic in InProcessCommandBuffer. Presentation feedback was getting sent to the GpuControlClient even if it didn't request it. Worse yet, vsync timing parameters were getting updated if presentation feedback was requested, even if the presentation feedback didn't contain accurate vsync information. On mac with OOP-D this was causing the real vsync timebase provided by the browser to get overridden with junk. Mac doesn't have accurate presentation feedback, instead it's just base::TimeTicks::Now() for when swap buffers completed. Share the existing logic found in GLES2CommandBufferStub and CommandBufferProxyImpl with InProcessCommandBuffer to avoid future problems. Bug: 900564 Change-Id: Ib1bdb822b2357a905a923a537d4da7bf01ad0a25 Reviewed-on: https://chromium-review.googlesource.com/c/1337638 Commit-Queue: kylechar <kylechar@chromium.org> Reviewed-by: Jonathan Backer <backer@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#608498}(cherry picked from commit d2b1aadb7232344d8a8fc90569ab3df3e3bc7351) Reviewed-on: https://chromium-review.googlesource.com/c/1341589 Reviewed-by: kylechar <kylechar@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#750} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 28
Looks fixed to me: https://uma.googleplex.com/p/chrome/variations/?sid=2a144e6e06484d1429bdfa2ec2d1b739 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by kylec...@chromium.org
, Nov 8Cc: jonr...@chromium.org