📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/13eb717ae40000

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/13eb717ae40000

Use SharedImage for HUD layer and one copy raster by backer@chromium.org
https://chromium.googlesource.com/chromium/src/+/e29d023ec416e0931584cb4c6daaa478cd36b478
main_thread_scroll_latency_tbmv2: 15.71 → 88.73 (+73.02)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  https://bit.ly/rendering-benchmarks

+tdresser +sadrul

I'm not sure how important these metrics are. I was debugging something else, noticed these regressions, and ran a pinpoint to investigate.

Over to Navid.

@nzolghadr: I could use some context here to prioritize. Right now this is a P3.

cc'ing bokan@ as I'm away.

Marking as untriaged so I remember to take a look today during bug triage

Took a look, we occasionally see super long latency (the max was 1.3 _seconds_) on main thread scrolls. Digging into the traces, it looks like the main difference is that in the "Good" CL we're sending BeginMainFrame (and thus resolving the scroll latency) quite regularly. With the blamed CL applied, we sometimes have stretches of 400+ms without a BeginMainFrame so you see the ScrollUpdateLatency pyramid (see attached screenshot), despite the main thread appearing to be fairly idle.

Probably also related, the GPU main thread appears to be quite busy during the time when the scroll latencies pile up. This looks like a real regression; related to scroll only because it's delaying BMF/Swap, so it should probably be investigated in the short term.

Deleted: Screenshot from 2018-11-01 14-46-28.png 494 KB

	Screenshot from 2018-11-01 14-46-28.png 494 KB View Download

Thanks for pointing this out bokan@. Knowing that it was tied to swap really helped. I have a small CL out for review: https://chromium-review.googlesource.com/c/chromium/src/+/1320591/

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

commit ce6ad323cd9aa0c49aa7245fe942dc5b9a91e421
Author: Jonathan Backer <backer@chromium.org>
Date: Wed Nov 07 19:11:43 2018

Avoid unnecessary call to MakeCurrent

SharedImage backend implementation is careful to restore GL state. The
reason we need to MakeCurrent is to create the texture in the correct
GPU process side share group.

With virtual contexts, we are often current with a different surface. By
skipping the unnecessary MakeCurrent, we avoid stalling GPU main thread
waiting for vblank on Nvidia GPUs (where use_virtualized_gl_contexts is
on by default).

Bug:  894100 
Change-Id: Ibaf66ffffbe228d5413a50873e1809d115b86e59
Reviewed-on: https://chromium-review.googlesource.com/c/1320591
Commit-Queue: Jonathan Backer <backer@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606103}
[modify] https://crrev.com/ce6ad323cd9aa0c49aa7245fe942dc5b9a91e421/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/ce6ad323cd9aa0c49aa7245fe942dc5b9a91e421/gpu/ipc/service/shared_image_stub.cc
[modify] https://crrev.com/ce6ad323cd9aa0c49aa7245fe942dc5b9a91e421/gpu/ipc/service/shared_image_stub.h

This is a low risk change that should help with performance on M71. It has the potential to impact many platforms (e.g. anything using context virtualization), because SharedImage is used for GPU rasterization as well (e.g. android, OSX, linux).

Pls apply appropriate OSs label.

Also pls update bug with canary result tomorrow.

Added OSes Linux, Mac, and Android.

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

Pls update bug with canary result tomorrow also provide more details if possible on merge safety as M71 is already in beta and going to stable soon. Thank you.

The NextAction date has arrived: 2018-11-08

FWIW, the graphs in https://chromeperf.appspot.com/report?sid=314035b098313721cdd182f3d83955c628984d138fa0695774fc0d19d7d1dad0&start_rev=595317&end_rev=606399 DO show improvement after the CL in c#12.

Approving merge to M71 branch 3578 based on comments #13 & #19. Please merge ASAP. Thank you.

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

commit 3fceb095e0dc78212cf2eef38c8eb79c34da8caa
Author: Jonathan Backer <backer@chromium.org>
Date: Thu Nov 08 16:14:25 2018

Avoid unnecessary call to MakeCurrent

SharedImage backend implementation is careful to restore GL state. The
reason we need to MakeCurrent is to create the texture in the correct
GPU process side share group.

With virtual contexts, we are often current with a different surface. By
skipping the unnecessary MakeCurrent, we avoid stalling GPU main thread
waiting for vblank on Nvidia GPUs (where use_virtualized_gl_contexts is
on by default).

Bug:  894100 
Change-Id: Ibaf66ffffbe228d5413a50873e1809d115b86e59
Reviewed-on: https://chromium-review.googlesource.com/c/1320591
Commit-Queue: Jonathan Backer <backer@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#606103}(cherry picked from commit ce6ad323cd9aa0c49aa7245fe942dc5b9a91e421)
Reviewed-on: https://chromium-review.googlesource.com/c/1326801
Reviewed-by: Jonathan Backer <backer@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#580}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/3fceb095e0dc78212cf2eef38c8eb79c34da8caa/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/3fceb095e0dc78212cf2eef38c8eb79c34da8caa/gpu/ipc/service/shared_image_stub.cc
[modify] https://crrev.com/3fceb095e0dc78212cf2eef38c8eb79c34da8caa/gpu/ipc/service/shared_image_stub.h

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

Commit: 3fceb095e0dc78212cf2eef38c8eb79c34da8caa
Author: backer@chromium.org
Commiter: backer@chromium.org
Date: 2018-11-08 16:14:25 +0000 UTC

Avoid unnecessary call to MakeCurrent

SharedImage backend implementation is careful to restore GL state. The
reason we need to MakeCurrent is to create the texture in the correct
GPU process side share group.

With virtual contexts, we are often current with a different surface. By
skipping the unnecessary MakeCurrent, we avoid stalling GPU main thread
waiting for vblank on Nvidia GPUs (where use_virtualized_gl_contexts is
on by default).

Bug:  894100 
Change-Id: Ibaf66ffffbe228d5413a50873e1809d115b86e59
Reviewed-on: https://chromium-review.googlesource.com/c/1320591
Commit-Queue: Jonathan Backer <backer@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#606103}(cherry picked from commit ce6ad323cd9aa0c49aa7245fe942dc5b9a91e421)
Reviewed-on: https://chromium-review.googlesource.com/c/1326801
Reviewed-by: Jonathan Backer <backer@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#580}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}