New issue
Advanced search Search tips

Issue 654020 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

5.3%-7.2% regression in rasterize_and_record_micro.key_mobile_sites_smooth at 423134:423245

Project Member Reported by qyears...@chromium.org, Oct 7 2016

Issue description

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

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

Hi enne@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 : Fix overflow/underflow in ui/gfx once and for all
Author  : enne
Commit description:
  
ui/gfx/geometry currently has a number of spots where adding or
subtracting two integers can cause overflow or underflow.  Fix this so
that Point/Vector2d/Size/Rect all correctly clamp integer arithmetic to
not fall into undefined overflow or underflow.

The behavior is now that any operation that would have overflowed or
underflowed is now clamped to the max or min integer.  Additionally, for
rects, if their size is large enough to overflow right/bottom, this will
also be clamped.

gfx::Insets is left unchanged as it is only used by ui/views and so is
much more likely to be under user control and not overflow.

This patch also fixes an overflow error in the unit test that happened
in Rect::Inset.

BUG= 648214 

Review-Url: https://codereview.chromium.org/2354783004
Cr-Commit-Position: refs/heads/master@{#423243}
Commit  : 8c14bf7d37bec879a5caa162f2c72a303da1a6e0
Date    : Wed Oct 05 19:13:19 2016


===== TESTED REVISIONS =====
Revision         Mean       Std Dev      N  Good?
chromium@423133  0.0428167  0.000168222  5  good
chromium@423189  0.0425667  9.12871e-05  5  good
chromium@423217  0.0427417  0.000245445  5  good
chromium@423231  0.0426     0.000123603  5  good
chromium@423238  0.042475   6.31906e-05  5  good
chromium@423242  0.04275    0.000114109  5  good
chromium@423243  0.045525   0.000127066  5  bad    <--
chromium@423244  0.045225   0.000180662  5  bad
chromium@423245  0.0454333  0.000127066  5  bad

Bisect job ran on: linux_perf_bisect
Bug ID: 654020

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests rasterize_and_record_micro.top_25_smooth
Test Metric: record_time/record_time
Relative Change: 6.11%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/linux_perf_bisect/builds/6760
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8999432607105129936


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

| 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 4 by enne@chromium.org, Oct 10 2016

Cc: chrishtr@chromium.org
Status: WontFix (was: Untriaged)
An additional 0.003ms when recording seems fine to have rects that don't overflow.
Sounds reasonable to me, thanks for taking a look :-)

Sign in to add a comment