New issue
Advanced search Search tips

Issue 625434 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jul 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

8.6%-59.4% regression in smoothness.top_25_smooth at 402910:402959

Project Member Reported by qyears...@chromium.org, Jul 3 2016

Issue description

See the link to graphs below.
 
Cc: pdr@chromium.org
Owner: pdr@chromium.org

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

Hi pdr@chromium.org, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Implement the new text-size-adjust CSS property
Author  : pdr
Commit description:
  
This patch adds a new CSS property for controlling text autosizing:
text-size-adjust (https://drafts.csswg.org/css-size-adjust).

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/-vHFK4g93jA/JW9wAJyKAQAJ

At a high level, this patch adds a new CSS property and plumbs it to
storage on the rare inherited style data. The new datatype for this is
"TextSizeAdjust" which just wraps a float with logic for storing both
a floating point adjustment value, and whether 'auto' should be used.
The text autosizer itself has been modified to read this new property.

A few changes were needed to allow the text autosizer to use
multipliers less than 1 (see: TextAutosizer::computeAutosizedFontSize
and callers to that function).

The simple-paragraph.html layouttest has been switched to a unit test
to prove the unit test infrastructure works, and many new tests have
been added for the text-size-adjust feature.

BUG= 623158 

Review-Url: https://codereview.chromium.org/2100013002
Cr-Commit-Position: refs/heads/master@{#402916}
Commit  : 5365b8ad85c2f5cee6034b8baea3c764ed78e1ea
Date    : Wed Jun 29 20:09:42 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N   Good?
chromium@402909  14.7654  2.84764   12  good
chromium@402913  13.52    0.866409  5   good
chromium@402915  15.1072  1.16582   5   good
chromium@402916  19.9921  1.91625   8   bad    <--
chromium@402922  19.1905  2.96192   8   bad
chromium@402934  18.8037  2.73676   12  bad
chromium@402959  21.6699  4.05541   8   bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 625434

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests smoothness.top_25_smooth
Test Metric: mean_input_event_latency/Wordpress
Relative Change: 32.97%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/757
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9008203386271263152


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5783978003398656

| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Tests>AutoBisect.  Thank you!

Comment 3 by pdr@chromium.org, Jul 7 2016

Status: WontFix (was: Assigned)
I looked into these and I think the results are expected.

The wordpress perf ones are the worst regressions and I inspected the page and found they trigger some different behavior based on the new property that was added.

A few of the graphs mysteriously improved on their own:
ChromiumPerf/android-galaxy-s5/smoothness.top_25_smooth / mean_input_event_latency / Wordpress

Lastly, this seems unrelated to the property that was added and I think this test is just new:
ChromiumPerf/chromium-rel-win7-x64-dual/smoothness.top_25_smooth / frame_times / http___games.yahoo.com
Thanks for looking into it Philip :-)

Sign in to add a comment