New issue
Advanced search Search tips

Issue 619254 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Reenable page_cycler_v2.typical_25 on reference builds

Project Member Reported by fmea...@chromium.org, Jun 11 2016

Issue description

Revision range first seen: 399176:299220
Link to failing step log: https://build.chromium.org/p/chromium.perf/builders/Mac%2010.10%20Perf%20%282%29/builds/3489/steps/page_cycler_v2.typical_25.reference/logs/stdio


If the test is disabled, please downgrade to Pri-2.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 11 2016

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

commit a0777d3adce51c721375b8f1dfd71bb002911d1c
Author: fmeawad <fmeawad@chromium.org>
Date: Sat Jun 11 01:04:59 2016

Revert of [PCv2] Add tracing category needed for computing ttfmp (patchset #3 id:40001 of https://codereview.chromium.org/2049593002/ )

Reason for revert:
Speculative revert, suspecting crashing the reference test page_cycler_v2.typical_25.reference on mac.

BUG= 619254 

Original issue's description:
> [PCv2] Add tracing category needed for computing ttfmp
>
> The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled.
> This CL enables recording of the category in PCv2.
>
> https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metrics/system_health/first_paint_metric.html
>
> BUG=None
>
> Committed: https://crrev.com/f9bc2493524f028607a8755c5bcfd8f51ba17998
> Cr-Commit-Position: refs/heads/master@{#399186}

TBR=nednguyen@google.com,ksakamoto@chromium.org,kouhei@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None

Review-Url: https://codereview.chromium.org/2057413002
Cr-Commit-Position: refs/heads/master@{#399342}

[modify] https://crrev.com/a0777d3adce51c721375b8f1dfd71bb002911d1c/tools/perf/benchmarks/page_cycler_v2.py

Cc: ksakamoto@chromium.org kouhei@chromium.org
Ah, I bet this is because Sakamoto's fix to reduce the size of trace to be used for fmp doesn't affect the ref build, hence it crashes d8 due to size blows up.
Cc: -ksakamoto@chromium.org fmea...@chromium.org
Owner: ksakamoto@chromium.org
The revert fixed the issue, assigning to ksakamoto@ for further triage.

Comment 4 by zh...@chromium.org, Jun 13 2016

Labels: -Pri-1 Pri-2
More specific: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_8-2016-06-10_11-49-01-61732.html shows "Inflated gzip data too long to fit into a string".

For now, I think the best action is just disable the benchmark on "reference" completely when you reland the CL that add extra categories.
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 15 2016

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

commit a0777d3adce51c721375b8f1dfd71bb002911d1c
Author: fmeawad <fmeawad@chromium.org>
Date: Sat Jun 11 01:04:59 2016

Revert of [PCv2] Add tracing category needed for computing ttfmp (patchset #3 id:40001 of https://codereview.chromium.org/2049593002/ )

Reason for revert:
Speculative revert, suspecting crashing the reference test page_cycler_v2.typical_25.reference on mac.

BUG= 619254 

Original issue's description:
> [PCv2] Add tracing category needed for computing ttfmp
>
> The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled.
> This CL enables recording of the category in PCv2.
>
> https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metrics/system_health/first_paint_metric.html
>
> BUG=None
>
> Committed: https://crrev.com/f9bc2493524f028607a8755c5bcfd8f51ba17998
> Cr-Commit-Position: refs/heads/master@{#399186}

TBR=nednguyen@google.com,ksakamoto@chromium.org,kouhei@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None

Review-Url: https://codereview.chromium.org/2057413002
Cr-Commit-Position: refs/heads/master@{#399342}

[modify] https://crrev.com/a0777d3adce51c721375b8f1dfd71bb002911d1c/tools/perf/benchmarks/page_cycler_v2.py

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 17 2016

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

commit e598d9f6f9d6bf4cd90c46503cb0c207791a02db
Author: ksakamoto <ksakamoto@chromium.org>
Date: Fri Jun 17 14:57:03 2016

Reland: [PCv2] Add tracing category needed for computing ttfmp

The time-to-first-meaningful-paint computation requires extra tracing categories to be enabled.
This CL enables recording of the category in PCv2.

https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metrics/system_health/first_paint_metric.html

This is a reland of https://codereview.chromium.org/2049593002/ with
disabling the benchmark on reference build. In the reference build, the
disabled-by-default-blink.debug.layout category brows up trace size and
causes d8 crash.

BUG= 615605 , 619254 

Review-Url: https://codereview.chromium.org/2070773002
Cr-Commit-Position: refs/heads/master@{#400426}

[modify] https://crrev.com/e598d9f6f9d6bf4cd90c46503cb0c207791a02db/tools/perf/benchmarks/page_cycler_v2.py

Status: Assigned (was: Untriaged)
Summary: Reenable page_cycler_v2.typical_25 on reference builds (was: page_cycler_v2.typical_25.reference on mac failure on chromium.perf at 399176)
Let me keep this bug open until the test is re-enabled.
We can re-enable it once ref build is updated to M53.

Labels: Performance-Sheriff-BotHealth
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 17 2016

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

commit fc7e75b896b8b0905a295300d9482af562e023e9
Author: eakuefner <eakuefner@chromium.org>
Date: Wed Aug 17 00:41:19 2016

[BotHealth] Disable PCv2 on reference using ShouldDisable

As of crrev.com/2249623002, super-/subclass decorator usages no longer
affect sub-/superclasses respectively. PCv2 is currently intended to be
disabled on all reference builds until the ref build is rolled, and this had
previously been done using the @Disabled decorator on the superclass.

This CL ports that functionality into PCv2's ShouldDisable method.

BUG= 619254 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq

Review-Url: https://codereview.chromium.org/2250123003
Cr-Commit-Position: refs/heads/master@{#412400}

[modify] https://crrev.com/fc7e75b896b8b0905a295300d9482af562e023e9/tools/perf/benchmarks/page_cycler_v2.py

We updated TTFMP implementation used in PCv2 ( Issue 638124 ).
It now requires the firstMeaningfulPaintCandidate trace event added in https://crrev.com/2276573003/ (included in M54).
Labels: -Performance-BotHealth
Status: Archived (was: Assigned)

Sign in to add a comment