New issue
Advanced search Search tips

Issue 893884 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

8.6% regression in rendering.mobile/thread_total_all_cpu_time_per_frame at 597543:597562

Project Member Reported by m...@chromium.org, Oct 9

Issue description

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

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


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

Android Nexus6 WebView Perf

rendering.mobile - Benchmark documentation link:
  https://bit.ly/rendering-benchmarks
Cc: r...@igalia.com
Owner: r...@igalia.com
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/150e2172e40000

[css-grid] Fix percentages in relative offsets for grid items by rego@igalia.com
https://chromium.googlesource.com/chromium/src/+/6298f794c7a71b2bd3f3efe137add7ceac5ae38d
thread_total_all_cpu_time_per_frame: 18.01 → 19.72 (+1.71)

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

Benchmark documentation link:
  https://bit.ly/rendering-benchmarks
Cc: jfernan...@igalia.com e...@chromium.org
Status: Started (was: Assigned)
I've a patch that I'd like to check if it fixes the performance regression.

But when I'm trying to run a similar try job in pinpoint I got a message "Access denied":
* Bug ID: 893884
* Gerrit URL: https://chromium-review.googlesource.com/c/chromium/src/+/1273117
* Bot: Android Nexus6 WebView Perf
* Benchmark: rendering.mobile
* Story: nyc.gov.scroll.2018

Please, could someone run it for me?

I just kicked it off for you: https://pinpoint-dot-chromeperf.appspot.com/job/1281b90ae40000

Cheers! :)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1281b90ae40000

Fix perf regression in LayoutBoxModelObject::RelativePositionOffset() by rego@igalia.com
https://chromium-review.googlesource.com/c/chromium/src/+/1273117/2
thread_total_all_cpu_time_per_frame: 19.27 → 17.4 (-1.877)

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

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

Comment 8 by bugdroid1@chromium.org, Oct 12

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

commit 4226ddf99103e493d7afb23a4c7902ee496108b6
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Fri Oct 12 00:23:12 2018

Fix perf regression in LayoutBoxModelObject::RelativePositionOffset()

In r597543 we introduced a performance regression
due to the changes in LayoutBoxModelObject::RelativePositionOffset().
One of the main differences is that AvailableHeight|Width()
was called always as part of the method,
while that was not the case before.

This patches moves the calls to AvailableHeight|Width()
to the moment where they are needed, trying to minimize
the impact in performance and come back to previous numbers.

No new tests as it's already covered by existent ones.

BUG= 893884 , 835607 

Change-Id: Id8aaba4736a821af9f401492206840c12a2be0e6
Reviewed-on: https://chromium-review.googlesource.com/c/1273117
Commit-Queue: Manuel Rego <rego@igalia.com>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#599034}
[modify] https://crrev.com/4226ddf99103e493d7afb23a4c7902ee496108b6/third_party/blink/renderer/core/layout/layout_box_model_object.cc

Status: Fixed (was: Started)

Sign in to add a comment