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

Issue 875640 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

All benchmark tests are failing on chromium.perf/android-go-perf

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Aug 19

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of nednguyen@chromium.org

...... too many results, data snipped.... and 40 other(s) in performance_test_suite failing on chromium.perf/android-go-perf

Builders failed on: 
- android-go-perf: 
  https://ci.chromium.org/buildbot/chromium.perf/android-go-perf

They are all seem to be legit crash: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/7c20607a-a36e-11e8-b9cf-0242ac110006

 
Cc: perezju@chromium.org
Labels: -Pri-2 Pri-1
Cc: sadrul@chromium.org
Owner: sadrul@chromium.org
Status: Assigned (was: Available)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/129ae676640000

Reland "metrics: Initialize persistent metric allocator early." by sadrul@chromium.org
https://chromium.googlesource.com/chromium/src/+/63fab7f6fe0f538518d8d9557fdb7e0ebc4759b5
0 → 1 (+1)

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

Comment 4 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

Labels: -Pri-1 Pri-2
No longer failing after revert, lowering Pri
Project Member

Comment 6 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

Sign in to add a comment