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

Issue 899969 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug-Regression



Sign in to add a comment

10.1%-11.2% regression in character_fallback,floats_50_100_nested at 603105:603179

Project Member Reported by 42576172...@developer.gserviceaccount.com, Oct 29

Issue description

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

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


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

android-nexus5x-perf
linux-perf

blink_perf.layout - Benchmark documentation link:
  https://bit.ly/blink-perf-benchmarks
Cc: bokan@chromium.org cduvall@chromium.org skobes@chromium.org chromium...@skia-public.iam.gserviceaccount.com
Owner: bokan@chromium.org
Status: Assigned (was: Untriaged)
📍 Found significant differences after each of 4 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/151817c9e40000

Fix all process types getting the name "Browser" in the tracelog on Android by cduvall@chromium.org
https://chromium.googlesource.com/chromium/src/+/f6401b7d08fb4c6cf435d189117e62a3edcef5c0
character_fallback: 2335 → 2310 (-25.51)

Roll src/third_party/skia e2fd74b48f6f..7d20bc42f453 (5 commits) by chromium-autoroll@skia-public.iam.gserviceaccount.com
https://chromium.googlesource.com/chromium/src/+/938efe8f66800ec57cd4e30b72d6af6882ec364e
character_fallback: 2334 → 2281 (-52.53)

Track layout jank score in PageTimingMetricsSender. by skobes@chromium.org
https://chromium.googlesource.com/chromium/src/+/9f9b8e7ecb85b9be04920f5cfd195744e7e56dd5
character_fallback: 2274 → 2315 (+41.42)

Performance fix for top controls-affected raster by bokan@chromium.org
https://chromium.googlesource.com/chromium/src/+/b2d410710e7ea7c5a33798ff6b5d5191b1951b5e
character_fallback: 2320 → 2529 (+209)

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

Benchmark documentation link:
  https://bit.ly/blink-perf-benchmarks
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/149b4524140000

All of the runs failed. The most common error (10/20 runs) was:
HTTPException: HTTP status code 400: {"error": {"message": "CIPD package path is required. Use \".\" to install to run dir."}}
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/12a89924140000

All of the runs failed. The most common error (10/20 runs) was:
HTTPException: HTTP status code 400: {"error": {"message": "CIPD package path is required. Use \".\" to install to run dir."}}
Cc: machenb...@chromium.org kylec...@chromium.org scro...@google.com pkotw...@chromium.org dpa...@chromium.org
📍 Found significant differences after each of 11 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/12235fd4140000

Fix all process types getting the name "Browser" in the tracelog on Android by cduvall@chromium.org
https://chromium.googlesource.com/chromium/src/+/f6401b7d08fb4c6cf435d189117e62a3edcef5c0
character_fallback: 2342 → 2301 (-40.77)

Blacklist CG platform generator on incInterlaced by scroggo@google.com
https://skia.googlesource.com/skia/+/a66ac00dc84ca84ca8d7b7d1515ccb9b88f0b234
character_fallback: 2316 → 2341 (+25.19)

Roll src/third_party/skia e2fd74b48f6f..7d20bc42f453 (5 commits) by chromium-autoroll@skia-public.iam.gserviceaccount.com
https://chromium.googlesource.com/chromium/src/+/938efe8f66800ec57cd4e30b72d6af6882ec364e
character_fallback: 2341 → 2283 (-58.13)

Track layout jank score in PageTimingMetricsSender. by skobes@chromium.org
https://chromium.googlesource.com/chromium/src/+/9f9b8e7ecb85b9be04920f5cfd195744e7e56dd5
character_fallback: 2277 → 2323 (+45.38)

Performance fix for top controls-affected raster by bokan@chromium.org
https://chromium.googlesource.com/chromium/src/+/b2d410710e7ea7c5a33798ff6b5d5191b1951b5e
character_fallback: 2312 → 2529 (+217.1)

Remove unused lofi_requested field from DataReductionProxyData by cduvall@chromium.org
https://chromium.googlesource.com/chromium/src/+/8cad73214a91bc2bb7ea930b61b9707cbbe52f33
character_fallback: 2524 → 2550 (+26.48)

[test] Skip test on endurance fuzzer by machenbach@chromium.org
https://chromium.googlesource.com/v8/v8/+/02d1e6c8eb4f58f5676cc1ece11eb4c5c841ef3b
character_fallback: 2550 → 2567 (+17.79)

Fix global state leak in gl_tests. by kylechar@chromium.org
https://chromium.googlesource.com/chromium/src/+/d6d589300fb1898becd2103568ff72b76013b1d5
character_fallback: 2572 → 2524 (-47.04)

Convert BeginFrameSource to use base::flat_set. by kylechar@chromium.org
https://chromium.googlesource.com/chromium/src/+/4ca7192d31df6c8271f4a45126b0c728fdb950da
character_fallback: 2515 → 2558 (+42.42)

[Android WebAPK] Move manifest config files to manifest/ directory by pkotwicz@chromium.org
https://chromium.googlesource.com/chromium/src/+/62e33bd2f07eba06b6e0b8d0e96934c56a9983a6
character_fallback: 2552 → 2574 (+21.94)

WebUI: Fix CrScrollableBehavior initialization with Polymer 2. by dpapad@chromium.org
https://chromium.googlesource.com/chromium/src/+/9c22b239be04e729e3ffc8f2180cc195cd041837
character_fallback: 2574 → 2585 (+11.29)

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

Benchmark documentation link:
  https://bit.ly/blink-perf-benchmarks
Cc: -scro...@google.com
My commit (https://skia.googlesource.com/skia/+/a66ac00dc84ca84ca8d7b7d1515ccb9b88f0b234) just affects our tests, and should have no effect on this regression.
Status: Started (was: Assigned)
Cc: -machenb...@chromium.org
Components: Blink>Layout
Labels: -Pri-2 Pri-3
Owner: ericrk@chromium.org
Status: Assigned (was: Started)
My CL expands a raster rect so we might be doing more raster. My guess is the added raster workload is affecting the main thread throughput. However, my CL is adding *back* the expanded rect after a previous CL of mine removed it and caused a different regression. So I would expect to see this undoing an earlier improvement but that's not the case because before that CL we only expanded the rect if we were actively scrolling. I had a follow-up CL that added back the "in-scroll" condition but it surprisingly didn't help.

To summarize the somewhat confusing order of events:

  - Landed: #1 crrev.com/77b67445ba6e415e8b41b5b6fd5c016c7195206d "Expands rect exactly by URL bar shown ratio, remove scroll condition"
    - Caused Regression:  bug 890870 
  - Landed: #2 crrev.com/b2d410710e7ea7c5a33798ff6b5d5191b1951b5e "Expands rect by entire URL bar height"
    - Fixed  bug 890870 
    - Caused This Regression
    - Caused Regression:  bug 900928 
  - Landed: #3 crrev.com/e799978a0188b827aef4ef94bdf31d7b0df5ae7d "Only expand during an active scroll"
    - Fixed  bug 900928 

CL #3 effectively takes us back to before #1 so I'm surprised this graph didn't recover. I reproduced locally and applying #3 right after #2 does undo the regression so something changed between #2 and #3. I manually bisected down to:

commit 924edd7aee62613bf0d3f51b4c4d00e63f5c104e
Author: Eric Karl <ericrk@chromium.org>
Date:   Wed Oct 31 03:23:49 2018 +0000

    EnableGpuRasterizationViewportRestriction bot config

After this commit, #3 doesn't help the graph. It looks like that CL enables GPU raster on "non-mobile" sites - of which this test is one. I'm guessing that the GPU raster path might be calculating its own rect?

In any case, I this test is measuring layout work only inside Blink so I don't think this is a real regression. Given #3 fixes the issue on its own, I think this is a WontFix but over to ericrk@ as FYI just in case.


Sign in to add a comment