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

Issue 778749 link

Starred by 4 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug

Blocked on:
issue 779316
issue 784454
issue 795760

Blocking:
issue 730193



Sign in to add a comment

viz: Collect and anaylze performance numbers for VizDisplayCompositor

Project Member Reported by kylec...@chromium.org, Oct 26 2017

Issue description

We need to collect some performance numbers for Linux desktop Chrome running normally vs with --enable-viz. We need to verify the numbers for --enable-viz are accurate and then identify the cause of any regressions.
 
Blocking: 770833
Labels: -770833

Comment 2 by sadrul@chromium.org, Oct 31 2017

Cc: danakj@chromium.org gklassen@chromium.org kylec...@chromium.org rjkroege@chromium.org fsam...@chromium.org
 Issue 779315  has been merged into this issue.

Comment 3 by sadrul@chromium.org, Oct 31 2017

Blockedon: 779316

Comment 4 by laforge@google.com, Nov 8 2017

Components: -Internals>Viz Internals>Services>Viz
Migrating from Internals>Viz to Internals>Services>Viz.

Comment 5 by sadrul@chromium.org, Nov 13 2017

Blockedon: 784454
Cc: -mfomitchev@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 1 2017

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

commit 44a8aa23c3a957e72bef44d7759421fe2fa5c0da
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Dec 01 03:19:55 2017

Add support for smoothness in cluster telemetry.

Bug: 778749
Change-Id: I34f85b22f6b292c05820cf010471d8fb9f861e3b
Reviewed-on: https://chromium-review.googlesource.com/789412
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Ravi Mistry <rmistry@chromium.org>
Reviewed-by: Victor Miura <vmiura@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520831}
[add] https://crrev.com/44a8aa23c3a957e72bef44d7759421fe2fa5c0da/tools/perf/contrib/cluster_telemetry/smoothness_ct.py
[modify] https://crrev.com/44a8aa23c3a957e72bef44d7759421fe2fa5c0da/tools/perf/validate_wpr_archives

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 1 2017

The following revision refers to this bug:
  https://skia.googlesource.com/buildbot/+/ea7e8a2b4866e8623439674294992b7522a1352f

commit ea7e8a2b4866e8623439674294992b7522a1352f
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Dec 01 17:51:43 2017

Add smoothness_ct in the frontend as a supported benchmark.

Bug: chromium:778749
Change-Id: If57bc08db0d41d55ae1956db0aa6843a3424a5d0
Reviewed-on: https://skia-review.googlesource.com/79162
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>

[modify] https://crrev.com/ea7e8a2b4866e8623439674294992b7522a1352f/ct/go/util/constants.go

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/218a522b36c160cf2fea45f7bdbcaccf08b5a94a

commit 218a522b36c160cf2fea45f7bdbcaccf08b5a94a
Author: kylechar <kylechar@chromium.org>
Date: Mon Dec 04 22:32:11 2017

Add 'CompositorThread' to telemetry GPU thread names.

When running with --enable-viz code is shuffled around between
processes. Some trace events for the smoothness benchmark are output in
the GPU process on the 'CompositorThread' with --enable-viz. Other GPU
threads don't output trace events. Telemetry doesn't look for
'CompositorThread' and misses all trace events from the GPU process as a
result.

Update TraceEventTimelineImporter to look for 'CompositorThread' when
it's trying to identify the GPU process.

Bug: chromium:778749
Test: Verified telemetry works with --enable-viz locally.
Change-Id: Iea2bcc1cdc412c1ea39e0d3c0f008977c3725563
Reviewed-on: https://chromium-review.googlesource.com/806717
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>

[modify] https://crrev.com/218a522b36c160cf2fea45f7bdbcaccf08b5a94a/telemetry/telemetry/timeline/trace_event_importer.py

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 15 2017

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

commit 8ecd3fb09902627acfd56aa41243d395604bc4b4
Author: Jonathan <jonross@chromium.org>
Date: Fri Dec 15 21:41:32 2017

Testing of Telemetry Perf and Unittests with Viz

We want to make sure that --enable-viz does not break telemetry,
so that we can properly collect metrics.

This change adds both to the Linux Viz FYI bot. A few known
failures are being skipped while we investigate them.

TEST=telemetry_perf_unittests telemetry_unittests

Bug: 778749
Change-Id: I4862bfd12ca255265d0e67406278cbd41202ac85
Reviewed-on: https://chromium-review.googlesource.com/827031
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524471}
[modify] https://crrev.com/8ecd3fb09902627acfd56aa41243d395604bc4b4/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/8ecd3fb09902627acfd56aa41243d395604bc4b4/testing/buildbot/test_suites.pyl
[modify] https://crrev.com/8ecd3fb09902627acfd56aa41243d395604bc4b4/testing/buildbot/waterfalls.pyl

Blockedon: 795760
Blocking: -770833 787097
Summary: viz: Collect and anaylze performance numbers for VizDisplayCompositor (was: viz: Collect and anaylze performance numbers for --enable-viz.)
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 1 2018

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

commit f5364a7d2520786491ecdc5ec6b066a305b36a80
Author: kylechar <kylechar@chromium.org>
Date: Thu Mar 01 13:31:21 2018

Add display compositor thread to thread_times metrics

The VizDisplayCompositor feature moves the display compositor off the
browser main thread and into the GPU process on a dedicated thread. Make
sure that the display compositor thread is recorded separately in the
thread_times benchmark and counted as part of the fast path total.

Bug: 778749
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Ic18c3564ecf5e192f2dcf1e2e272dac3c99d8873
Reviewed-on: https://chromium-review.googlesource.com/937398
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Brian Anderson <brianderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540146}
[modify] https://crrev.com/f5364a7d2520786491ecdc5ec6b066a305b36a80/components/viz/service/main/viz_main_impl.cc
[modify] https://crrev.com/f5364a7d2520786491ecdc5ec6b066a305b36a80/tools/perf/metrics/timeline.py

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/21ff400bb4884e9a601168e7fa08559a76e06a75

commit 21ff400bb4884e9a601168e7fa08559a76e06a75
Author: kylechar <kylechar@chromium.org>
Date: Fri Mar 09 20:28:06 2018

Update VizCompositorThread name.

The viz service display compositor thread was renamed
VizCompositorThread to disambiguate it from the renderer compositor
thread.

Bug: chromium:778749
Change-Id: If22dc93219d575cc63ee0dab06835d57fef877c8
Reviewed-on: https://chromium-review.googlesource.com/942982
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: kylechar <kylechar@chromium.org>

[modify] https://crrev.com/21ff400bb4884e9a601168e7fa08559a76e06a75/telemetry/telemetry/timeline/trace_event_importer.py

I ran a few benchmarks on Windows 10 with pinpoint. Click on "Analyze benchmark results" in the pinpoint link to see full results. The second run is with OOP-D enabled.

smoothness.top_25_smooth
https://pinpoint-dot-chromeperf.appspot.com/job/149d3498240000
All changes are small and marked as insignificant compared to baseline.

thread_times.tough_compositor_cases
https://pinpoint-dot-chromeperf.appspot.com/job/14ef20e8240000
Less CPU time spent in browser and IO threads. More CPU time spend in display compositor thread. This is totally expected. What is weird here more tasks and more CPU time spent in GPU thread. This isn't expected. There is a small increase in total CPU time and fast path CPU time, it's marked as insignificant but it's about the same size as increase in GPU thread CPU time. 

thread_times.tough_scrolling_cases
https://pinpoint-dot-chromeperf.appspot.com/job/14edefc8240000
Less CPU time spent in browser and IO threads. More CPU time spend in display compositor thread. This is totally expected. No major difference in GPU thread tasks or CPU time in contrast to tough_compositor_cases. No significant difference in total or fast path CPU time.

Everything looks pretty good except thread_times.tough_compositor_cases. The increase in tasks and CPU time for the GPU thread warrants more investigation. Nothing serious enough to block canary testing though.
danakj pointed out that for thread_times.tough_compositor_cases there are 460 GPU tasks per frame in the baseline vs 465 with OOP-D, so an extra 5 tasks per frame isn't huge. These are probably GPU raster tasks, so we should investigate if there is a real regression and if so why are we rastering more tiles?
Here is a second run of thread_times.tough_compositor_cases on Windows 10. In this run OOP-D had a decrease in GPU thread tasks and CPU time, similar magnitude to the increase in #16. It looks like that difference is just noise.

https://pinpoint-dot-chromeperf.appspot.com/job/14b53838240000


Project Member

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

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

commit 2fb268996d6965b824cf83e6acf092826232b27e
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Mon Jun 18 20:10:30 2018

oopd: Run desktop rendering benchmarks with oopd turned on.

BUG=778749

Change-Id: I3ff438e53838e3e8c9c387fab837af0a31bcdac2
Reviewed-on: https://chromium-review.googlesource.com/1101643
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568127}
[add] https://crrev.com/2fb268996d6965b824cf83e6acf092826232b27e/tools/perf/contrib/oopd/OWNERS
[add] https://crrev.com/2fb268996d6965b824cf83e6acf092826232b27e/tools/perf/contrib/oopd/__init__.py
[add] https://crrev.com/2fb268996d6965b824cf83e6acf092826232b27e/tools/perf/contrib/oopd/oopd.py

Project Member

Comment 20 by bugdroid1@chromium.org, Jun 18 2018

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

commit 9d9297098b49afc466705957de6a95b6f1ba0593
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Mon Jun 18 23:05:21 2018

oopd: Add the oopd rendering benchmark to fyi perf waterfall.

BUG=778749

Change-Id: I0f3cfea90ffefa739c82a673a4e2e8abd5dfba00
Reviewed-on: https://chromium-review.googlesource.com/1104582
Reviewed-by: Emily Hanley <eyaich@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568215}
[modify] https://crrev.com/9d9297098b49afc466705957de6a95b6f1ba0593/testing/buildbot/chromium.perf.fyi.json

Project Member

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

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

commit e9860e98b649c9b1b80ed83498bd037167364a44
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Tue Jun 26 18:20:19 2018

oopd: Add rendering benchmark for oopd on perf.fyi bots.

BUG=778749

Change-Id: I35cf5c2f9c391fc019fe13f2d1d8d31b3cb948ed
Reviewed-on: https://chromium-review.googlesource.com/1114466
Reviewed-by: Emily Hanley <eyaich@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570470}
[modify] https://crrev.com/e9860e98b649c9b1b80ed83498bd037167364a44/testing/buildbot/chromium.perf.fyi.json
[modify] https://crrev.com/e9860e98b649c9b1b80ed83498bd037167364a44/tools/perf/contrib/oopd/oopd.py

Project Member

Comment 22 by bugdroid1@chromium.org, Aug 8

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

commit f3b3b58a3a813fe519a229e0a39418e4adb4f7a0
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Wed Aug 08 03:42:51 2018

viz: Report the reason for not drawing in display scheduler.

Add some metrics for why the display-scheduler aborts a draw operation.
This metric should help explain why fewer frames are drawn when oop-d
is turned on.

BUG=865179, 778749

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I68e9a1e9c89b42042ac1611e9b6516bf7948657f
Reviewed-on: https://chromium-review.googlesource.com/1165485
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581463}
[modify] https://crrev.com/f3b3b58a3a813fe519a229e0a39418e4adb4f7a0/components/viz/service/display/display_scheduler.cc
[modify] https://crrev.com/f3b3b58a3a813fe519a229e0a39418e4adb4f7a0/components/viz/service/display/display_scheduler.h
[modify] https://crrev.com/f3b3b58a3a813fe519a229e0a39418e4adb4f7a0/tools/metrics/histograms/histograms.xml

Project Member

Comment 23 by bugdroid1@chromium.org, Aug 15

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

commit 2353ca5213ac93e36dc694c12fb8a5cc357d2115
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Wed Aug 15 15:53:26 2018

viz/metrics: Increase the shmem size for gpu process.

With the display-compositor moving into the gpu process, the shared
memory segment used for storing metrics needs to be larger. The UsedPct
metrics for the various processes are usually around 20-30%. But for
the gpu process with the display compositor, the usage shoots up to
~75% [1]. So increase the limit from 64K to 256K, to avoid overruns.

[1] https://uma.googleplex.com/p/chrome/variations/?sid=9dbcefdbd5a988d093ae8c4b66629c01

BUG=778749

Change-Id: I5328bae58ce9f031293ed01ccd2c011581daf853
Reviewed-on: https://chromium-review.googlesource.com/1173353
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583260}
[modify] https://crrev.com/2353ca5213ac93e36dc694c12fb8a5cc357d2115/content/browser/browser_child_process_host_impl.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Aug 16

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

commit 0cdc1458535ab3ff7eac2fd7281ce2f8972eaffa
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Thu Aug 16 15:54:46 2018

metrics: Initialize persistent metric allocator early.

The PersistentMemoryAllocator in the gpu (and other) process is created
asynchronously. If any uma-histogram is reported before that time, then
those Histograms are not stored in the shared memory. This has some
implications:
 . If the process crashes (which is common for the gpu process on
   android), then these metrics are lost.
 . If the process stays alive, then the metrics are reported
   periodically, but because it is buffered, it doesn't match up
   correctly with the other metrics that are reported in the shared
   memory.

The allocator is created in response to a message from the browser,
which includes the handle for the shared memory to use for the allocator.
The browser waits until the process has been launched and connected.
So this happens much later, compared to other messages which are sent
from browser over mojo without waiting for the connection to have
been established first. Therefore, change this so that the browser
immediately shares the shared-memory segment with the gpu process. mojo
takes care of waiting for the process-launch etc.

BUG=865179, 778749

Change-Id: Iccd3d4422de3e22ead4bc6510afc5ea207bf6b6e
Reviewed-on: https://chromium-review.googlesource.com/1174709
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583673}
[modify] https://crrev.com/0cdc1458535ab3ff7eac2fd7281ce2f8972eaffa/content/browser/browser_child_process_host_impl.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Aug 16

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

commit 4e2d1e0dbd6fca9f09c1608c05c4c636e7621d15
Author: Matthew Jones <mdjones@chromium.org>
Date: Thu Aug 16 22:29:57 2018

Revert "metrics: Initialize persistent metric allocator early."

This reverts commit 0cdc1458535ab3ff7eac2fd7281ce2f8972eaffa.

Reason for revert: Breaks ToastHWATest* on Marshmallow 64 bit Tester:
[FATAL:persistent_histogram_allocator.cc(893)] Check failed: !subtle::NoBarrier_Load(&g_histogram_allocator).

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Marshmallow%2064%20bit%20Tester/23305

Original change's description:
> metrics: Initialize persistent metric allocator early.
> 
> The PersistentMemoryAllocator in the gpu (and other) process is created
> asynchronously. If any uma-histogram is reported before that time, then
> those Histograms are not stored in the shared memory. This has some
> implications:
>  . If the process crashes (which is common for the gpu process on
>    android), then these metrics are lost.
>  . If the process stays alive, then the metrics are reported
>    periodically, but because it is buffered, it doesn't match up
>    correctly with the other metrics that are reported in the shared
>    memory.
> 
> The allocator is created in response to a message from the browser,
> which includes the handle for the shared memory to use for the allocator.
> The browser waits until the process has been launched and connected.
> So this happens much later, compared to other messages which are sent
> from browser over mojo without waiting for the connection to have
> been established first. Therefore, change this so that the browser
> immediately shares the shared-memory segment with the gpu process. mojo
> takes care of waiting for the process-launch etc.
> 
> BUG=865179, 778749
> 
> Change-Id: Iccd3d4422de3e22ead4bc6510afc5ea207bf6b6e
> Reviewed-on: https://chromium-review.googlesource.com/1174709
> Reviewed-by: Brian White <bcwhite@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583673}

TBR=avi@chromium.org,sadrul@chromium.org,bcwhite@chromium.org

Change-Id: I65e9ad62e6613d0eaf08e834def5a988a60c4a19
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 865179, 778749
Reviewed-on: https://chromium-review.googlesource.com/1179041
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583859}
[modify] https://crrev.com/4e2d1e0dbd6fca9f09c1608c05c4c636e7621d15/content/browser/browser_child_process_host_impl.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Aug 17

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

commit 63fab7f6fe0f538518d8d9557fdb7e0ebc4759b5
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Aug 17 21:41:00 2018

Reland "metrics: Initialize persistent metric allocator early."

This is a reland of 0cdc1458535ab3ff7eac2fd7281ce2f8972eaffa

Original change's description:
> metrics: Initialize persistent metric allocator early.
>
> The PersistentMemoryAllocator in the gpu (and other) process is created
> asynchronously. If any uma-histogram is reported before that time, then
> those Histograms are not stored in the shared memory. This has some
> implications:
>  . If the process crashes (which is common for the gpu process on
>    android), then these metrics are lost.
>  . If the process stays alive, then the metrics are reported
>    periodically, but because it is buffered, it doesn't match up
>    correctly with the other metrics that are reported in the shared
>    memory.
>
> The allocator is created in response to a message from the browser,
> which includes the handle for the shared memory to use for the allocator.
> The browser waits until the process has been launched and connected.
> So this happens much later, compared to other messages which are sent
> from browser over mojo without waiting for the connection to have
> been established first. Therefore, change this so that the browser
> immediately shares the shared-memory segment with the gpu process. mojo
> takes care of waiting for the process-launch etc.
>
> BUG=865179, 778749
>
> Change-Id: Iccd3d4422de3e22ead4bc6510afc5ea207bf6b6e
> Reviewed-on: https://chromium-review.googlesource.com/1174709
> Reviewed-by: Brian White <bcwhite@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583673}
TBR=avi,sadrul

Bug: 865179, 778749
Change-Id: I205221ed82b073464e1c4d52593ba727ebc211a5
Reviewed-on: https://chromium-review.googlesource.com/1179346
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584203}
[modify] https://crrev.com/63fab7f6fe0f538518d8d9557fdb7e0ebc4759b5/content/browser/browser_child_process_host_impl.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Aug 19

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

commit a3819689366a317736b43244039cdf3c639ee546
Author: Ned Nguyen <nednguyen@google.com>
Date: Sun Aug 19 19:18:12 2018

Revert "Reland "metrics: Initialize persistent metric allocator early.""

This reverts commit 63fab7f6fe0f538518d8d9557fdb7e0ebc4759b5.

Reason for revert: breaking android-go-perf on chrome perf waterfall
BUG:875640

Original change's description:
> Reland "metrics: Initialize persistent metric allocator early."
> 
> This is a reland of 0cdc1458535ab3ff7eac2fd7281ce2f8972eaffa
> 
> Original change's description:
> > metrics: Initialize persistent metric allocator early.
> >
> > The PersistentMemoryAllocator in the gpu (and other) process is created
> > asynchronously. If any uma-histogram is reported before that time, then
> > those Histograms are not stored in the shared memory. This has some
> > implications:
> >  . If the process crashes (which is common for the gpu process on
> >    android), then these metrics are lost.
> >  . If the process stays alive, then the metrics are reported
> >    periodically, but because it is buffered, it doesn't match up
> >    correctly with the other metrics that are reported in the shared
> >    memory.
> >
> > The allocator is created in response to a message from the browser,
> > which includes the handle for the shared memory to use for the allocator.
> > The browser waits until the process has been launched and connected.
> > So this happens much later, compared to other messages which are sent
> > from browser over mojo without waiting for the connection to have
> > been established first. Therefore, change this so that the browser
> > immediately shares the shared-memory segment with the gpu process. mojo
> > takes care of waiting for the process-launch etc.
> >
> > BUG=865179, 778749
> >
> > Change-Id: Iccd3d4422de3e22ead4bc6510afc5ea207bf6b6e
> > Reviewed-on: https://chromium-review.googlesource.com/1174709
> > Reviewed-by: Brian White <bcwhite@chromium.org>
> > Reviewed-by: Avi Drissman <avi@chromium.org>
> > Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#583673}
> TBR=avi,sadrul
> 
> Bug: 865179, 778749
> Change-Id: I205221ed82b073464e1c4d52593ba727ebc211a5
> Reviewed-on: https://chromium-review.googlesource.com/1179346
> Reviewed-by: Matthew Jones <mdjones@chromium.org>
> Commit-Queue: Matthew Jones <mdjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#584203}

TBR=avi@chromium.org,sadrul@chromium.org,bcwhite@chromium.org,mdjones@chromium.org

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

Bug: 865179, 778749
Change-Id: I7fd057c81494dff8072a2e847510821ec6ff8530
Reviewed-on: https://chromium-review.googlesource.com/1180741
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#584342}
[modify] https://crrev.com/a3819689366a317736b43244039cdf3c639ee546/content/browser/browser_child_process_host_impl.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Aug 21

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

commit bd6bbb6ea49f7f755b649e472e20b1b2f6450d22
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Tue Aug 21 03:53:17 2018

Reland "metrics: Initialize persistent metric allocator early."

The fix from the original change is to share the shared-memory handle
with the child process only if a child process is actually launched.
There can be configurations where a child-process is not launched (e.g.
for in-process gpu etc.), and in such cases, it is not necessary to
create a separate PersistentMemoryAllocator, since there already is an
existing one for the browser process.

Original change's description:
metrics: Initialize persistent metric allocator early.

The PersistentMemoryAllocator in the gpu (and other) process is created
asynchronously. If any uma-histogram is reported before that time, then
those Histograms are not stored in the shared memory. This has some
implications:
 . If the process crashes (which is common for the gpu process on
   android), then these metrics are lost.
 . If the process stays alive, then the metrics are reported
   periodically, but because it is buffered, it doesn't match up
   correctly with the other metrics that are reported in the shared
   memory.

The allocator is created in response to a message from the browser,
which includes the handle for the shared memory to use for the allocator.
The browser waits until the process has been launched and connected.
So this happens much later, compared to other messages which are sent
from browser over mojo without waiting for the connection to have
been established first. Therefore, change this so that the browser
immediately shares the shared-memory segment with the gpu process. mojo
takes care of waiting for the process-launch etc.

BUG=865179, 778749, 875640

Change-Id: Ieffc014ae711d4bc922cda8034bc4a05fd861bde
Reviewed-on: https://chromium-review.googlesource.com/1182041
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584642}
[modify] https://crrev.com/bd6bbb6ea49f7f755b649e472e20b1b2f6450d22/content/browser/browser_child_process_host_impl.cc

Blocking: -787097 730193
Project Member

Comment 30 by bugdroid1@chromium.org, Dec 5

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

commit 7b291c01dc42f41f4b35fadd29389995001cc09d
Author: kylechar <kylechar@chromium.org>
Date: Wed Dec 05 17:40:09 2018

Remove rendering.oop perf benchmarks.

OOP-D is on by default for perf bots on Linux and Android so the
rendering.oopd.desktop and rendering.oopd.mobile benchmarks are no
longer needed. Stop running the benchmarks on FYI perf bots and remove
the benchmark.

The shard map file was updated using the following command:

$ ./tools/perf/generate_perf_sharding update -w perf-fyi

Bug: 778749,  911421 
Change-Id: I68f695fc20d8ba0b897f43158d8ada58481fb4f6
Reviewed-on: https://chromium-review.googlesource.com/c/1361668
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614010}
[modify] https://crrev.com/7b291c01dc42f41f4b35fadd29389995001cc09d/testing/buildbot/chromium.perf.fyi.json
[delete] https://crrev.com/c19880775b223830ee39f172e2b9fc3fb7039e1f/tools/perf/contrib/oopd/OWNERS
[delete] https://crrev.com/c19880775b223830ee39f172e2b9fc3fb7039e1f/tools/perf/contrib/oopd/__init__.py
[delete] https://crrev.com/c19880775b223830ee39f172e2b9fc3fb7039e1f/tools/perf/contrib/oopd/oopd.py
[modify] https://crrev.com/7b291c01dc42f41f4b35fadd29389995001cc09d/tools/perf/core/bot_platforms.py
[modify] https://crrev.com/7b291c01dc42f41f4b35fadd29389995001cc09d/tools/perf/core/shard_maps/android-nexus5x-perf-fyi_map.json

Sign in to add a comment