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

Issue 903762 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

1.9%-9.5% regression in rendering.desktop/tasks_per_frame_total_all at 606302:606355

Project Member Reported by alexclarke@chromium.org, Nov 9

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=903762

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=db25010e54e7be47857fb460d35aed4a6784f5fedf3886bf0f8e35803be57f43


Bot(s) for this bug's original alert(s):

linux-perf

rendering.desktop - Benchmark documentation link:
  https://bit.ly/rendering-benchmarks
Cc: oysteine@chromium.org
Owner: oysteine@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/12f63a40140000

Reland "Perfetto: Optimization to avoid string table lookup for _END events" by oysteine@chromium.org
https://chromium.googlesource.com/chromium/src/+/c972056412f05ea6253d8c678cd0c789f9396244
tasks_per_frame_total_all: 249.1 → 274.6 (+25.5)

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

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

Comment 4 by bugdroid1@chromium.org, Nov 13

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

commit a6bcdc64c6eec541ccc82f67a166f6854b6180cf
Author: oysteine <oysteine@chromium.org>
Date: Tue Nov 13 21:44:28 2018

Revert "Reland "Perfetto: Optimization to avoid string table lookup for _END events""

This reverts commit c972056412f05ea6253d8c678cd0c789f9396244.

Reverting this; looks like the increased bytes used due to stringtable indices being hashes rather than sequential numbering is causing the trace chunks to fill up quicker and negates the optimization.

BUG= 903762 
Original change's description:
> Reland "Perfetto: Optimization to avoid string table lookup for _END events"
> 
> This reverts commit 7acd0b524621d76fbaefdf397fdb37662bd8bf66.
> 
> Reason for revert: Re-landing with crashfix
> 
> Original change's description:
> > Revert "Perfetto: Optimization to avoid string table lookup for _END events"
> > 
> > This reverts commit a7afe430785635f9608a2c0889295d698547a87b.
> > 
> > Reason for revert: I suspect this may have caused crashes on https://ci.chromium.org/p/chrome/builders/luci.chrome.ci/linux-perf-fyi#
> > 
> > Original change's description:
> > > Perfetto: Optimization to avoid string table lookup for _END events
> > > 
> > > If we're emitting an _END trace event, we know that the string
> > > table entries for the name and the categories have already been
> > > emitted and so we don't need to check the string table in this
> > > case.
> > > 
> > > R=​primiano@chromium.org,skyostil@chromium.org
> > > 
> > > Change-Id: I9107bffb0b48f7b71d0a42961a78a3861ffcb76f
> > > Reviewed-on: https://chromium-review.googlesource.com/c/1258043
> > > Commit-Queue: oysteine <oysteine@chromium.org>
> > > Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#600183}
> > 
> > TBR=primiano@chromium.org,oysteine@chromium.org,skyostil@chromium.org
> > 
> > # Not skipping CQ checks because original CL landed > 1 day ago.
> > 
> > Change-Id: Ia460750fcaf338c4bf24ee7317d58b7aeede9520
> > Reviewed-on: https://chromium-review.googlesource.com/c/1293612
> > Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
> > Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#601586}
> 
> TBR=cbiesinger@chromium.org,primiano@chromium.org,oysteine@chromium.org,skyostil@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Change-Id: I8f3b5c21494d66fe9e6c25363627ec156f5abf12
> Reviewed-on: https://chromium-review.googlesource.com/c/1325077
> Commit-Queue: oysteine <oysteine@chromium.org>
> Reviewed-by: oysteine <oysteine@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#606308}

TBR=cbiesinger@chromium.org,primiano@chromium.org,oysteine@chromium.org,skyostil@chromium.org

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

Change-Id: I4719cde108d34cee7223615861c1f7cfac31bcd0
Reviewed-on: https://chromium-review.googlesource.com/c/1334199
Reviewed-by: oysteine <oysteine@chromium.org>
Commit-Queue: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607747}
[modify] https://crrev.com/a6bcdc64c6eec541ccc82f67a166f6854b6180cf/services/tracing/perfetto/json_trace_exporter.cc
[modify] https://crrev.com/a6bcdc64c6eec541ccc82f67a166f6854b6180cf/services/tracing/public/cpp/perfetto/trace_event_data_source.cc

Status: Fixed (was: Assigned)
Graphs recovered.

Sign in to add a comment