New issue
Advanced search Search tips

Issue 771760 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

4.7% regression in smoothness.tough_animation_cases at 504979:505063

Project Member Reported by m...@chromium.org, Oct 4 2017

Issue description

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

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


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

chromium-rel-win7-gpu-intel
Cc: lukasza@chromium.org
Owner: lukasza@chromium.org
Status: Assigned (was: Untriaged)

=== Auto-CCing suspected CL author lukasza@chromium.org ===

Hi lukasza@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Lukasz Anforowicz
  Commit : dddf6ec11c0aec4241c455764f578373e23d6a4f
  Date   : Thu Sep 28 14:04:00 2017
  Subject: Removing callers of the WebContents::GetRenderProcessHost() method.

Bisect Details
  Configuration: winx64intel_perf_bisect
  Benchmark    : smoothness.tough_animation_cases
  Metric       : frame_times/balls_css_transition_2_properties.html
  Change       : 4.04% | 22.094211888 -> 23.0822656751

Revision             Result                   N
chromium@504978      22.0942 +- 0.857648      9       good
chromium@505000      22.2155 +- 1.77733       9       good
chromium@505011      23.4549 +- 1.11769       14      good
chromium@505012      23.5265 +- 1.06612       9       good
chromium@505013      23.0772 +- 0.710238      9       bad       <--
chromium@505017      22.9682 +- 1.01105       14      bad
chromium@505021      23.0905 +- 0.659078      9       bad
chromium@505063      23.0823 +- 0.867457      6       bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=balls.css.transition.2.properties.html smoothness.tough_animation_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8966630455839810880


For feedback, file a bug with component Speed>Bisection
Cc: shend@chromium.org
Owner: shend@chromium.org

=== Auto-CCing suspected CL author shend@chromium.org ===

Hi shend@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Darren Shen
  Commit : 34de07a7da1921205306d2f6299187dcddd42d2d
  Date   : Thu Sep 28 13:43:01 2017
  Subject: [CSSParser] Use streaming parser for CSS parsing.

Bisect Details
  Configuration: winx64intel_perf_bisect
  Benchmark    : smoothness.tough_animation_cases
  Metric       : frame_times/balls_css_transition_2_properties.html
  Change       : 6.26% | 21.8987969475 -> 23.2690813831

Revision             Result                   N
chromium@504978      21.8988 +- 0.340484      6      good
chromium@505000      22.1549 +- 0.920982      6      good
chromium@505003      22.3166 +- 0.704113      6      good
chromium@505004      22.3599 +- 1.31648       6      good
chromium@505005      23.4718 +- 0.475393      6      bad       <--
chromium@505006      23.5222 +- 0.572346      6      bad
chromium@505011      23.3409 +- 0.694857      6      bad
chromium@505021      23.3878 +- 0.305549      6      bad
chromium@505063      23.2691 +- 0.909119      6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=balls.css.transition.2.properties.html smoothness.tough_animation_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8966614357329772304


For feedback, file a bug with component Speed>Bisection

=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Darren Shen
  Commit : 34de07a7da1921205306d2f6299187dcddd42d2d
  Date   : Thu Sep 28 13:43:01 2017
  Subject: [CSSParser] Use streaming parser for CSS parsing.

Bisect Details
  Configuration: winx64intel_perf_bisect
  Benchmark    : smoothness.tough_animation_cases
  Metric       : frame_times/balls_css_transition_2_properties.html
  Change       : 5.62% | 22.1178037431 -> 23.3600956029

Revision             Result                   N
chromium@504978      22.1178 +- 0.523085      6      good
chromium@505000      22.3107 +- 1.19008       6      good
chromium@505003      22.2742 +- 1.34752       9      good
chromium@505004      22.3007 +- 0.765061      6      good
chromium@505005      23.4211 +- 0.373952      6      bad       <--
chromium@505006      23.5457 +- 0.604535      6      bad
chromium@505011      23.812 +- 0.641418       6      bad
chromium@505021      23.5983 +- 0.865207      6      bad
chromium@505063      23.3601 +- 0.959196      6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=balls.css.transition.2.properties.html smoothness.tough_animation_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8966606392599324016


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 10 2017

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

commit fd770fea29e678f8862af6f631f53e7e85068de5
Author: Darren Shen <shend@chromium.org>
Date: Tue Oct 10 07:14:10 2017

[CSSParser] Speculatively improve speed of tokenizing to end.

Tokenizing a string into a vector of tokens is important when we need to
parse a lot of property values (e.g. for animations). This patch tries
to speculatively improve the performance of this operation by inlining
a bunch of things and not doing unnecessary work.

BUG= 771760 

Change-Id: Ideb49af1e6de47c540b45c701840284d546da8a5
Reviewed-on: https://chromium-review.googlesource.com/701903
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: nainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507610}
[modify] https://crrev.com/fd770fea29e678f8862af6f631f53e7e85068de5/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp

shend: It looks like the graph didn't improve after r507610. That said, only one benchmark/platform were affected and it's been a few months; should we close this bug?
Status: WontFix (was: Assigned)
Yeah the regression doesn't seem to have broken anything and it seems hard to optimise this any further.

Sign in to add a comment