All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=692955

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguMGFtgoM


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

win-high-dpi

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8987514643350372736

=== Auto-CCing suspected CL author karlo@opera.com ===

Hi karlo@opera.com, 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 : karlo
  Commit : 934becac5daa91ea979fb66e4ae21761ca11ebc9
  Date   : Mon Feb 13 13:52:07 2017
  Subject: Support subpixel layout of borders.

Bisect Details
  Configuration: winx64_high_dpi_perf_bisect
  Benchmark    : blink_perf.paint
  Metric       : paint-offset-changes/paint-offset-changes
  Change       : 57.67% | 219.389166667 -> 345.901333333

Revision             Result                  N
chromium@449921      219.389 +- 6.2433       6      good
chromium@449933      225.866 +- 6.02116      6      good
chromium@449939      227.898 +- 40.1352      6      good
chromium@449942      225.26 +- 8.37492       6      good
chromium@449943      350.557 +- 19.8235      6      bad       <--
chromium@449944      347.993 +- 18.6086      6      bad
chromium@449945      356.383 +- 22.4091      6      bad
chromium@449968      345.901 +- 14.5757      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 blink_perf.paint

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8987514643350372736

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5014020639162368


| 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 Speed>Bisection.  Thank you!

I've tried to reproduce this on Windows, but I can't really see any meaningful change on my end.  A similarly sized performance regression is visible on win-zenbook, but other Windows targets see regressions of only a few percent.  The patch will exercise roundf(), floating point multiplications and divisions, so some performance loss is to be expected on this test, but not 50+%.

Is there something special with win-high-dpi and win-zenbook?  Is the test run differently on these bots?

The big thing with high-dpi is that it runs at a higher device pixel ratio, could that be related?

Yes, that's my only lead at the moment, although the patch shouldn't behave differently on high-dpi devices (the required paint side support for pixel ratio > 1 isn't in place yet).

Is win-zenbook also a high-dpi device?  I tried to force high dpi on my Windows machine, but it only doubled every pixel in content_shell, it didn't actually change the pixel ratio, and performance was unaffected.  I also tried -force-device-scale-factor=2, but that didn't seem to affect performance either.  Do you have any idea how high-dpi affects content_shell?

FYI - this patch caused a 1-5% regression on memory_mobile as well. What's the plan?

I need someone to get me some more information on the win-high-dpi and win-zenbook machines in order to make progress on the blink_perf.paint performance regression.  I can't reproduce the slowdown locally, so I need to understand what's different about those machines.

There are some other slowdowns that are becoming evident in blink_perf.layout.  I'll see if I can reproduce those locally.

The memory regressions look like they appear after 449959, which is after my patch (449943).

Sorry for the delay, karlo! What specifics would help you understand more about the machines? benhenry@, can you help karlo with getting the details? (Note he's not a Googler so will not be able to log into bots himself)

win-zenbook looks like it's also high dpi, can you confirm that?

Also, which dpi and device pixel ratio are these machines using?  Which Windows version are they running?

Thanks for any assistance!

Assigning to benhenry to answer #10 and then reassign back to karlo@opera.com. Also cc efoo to point out that it should be easier to get this info.

Here's the info we have for the machine which bisected to find your CL:
"""
architecture: x64
os version: 10.0.10240
processor count: 1
processor type: Intel(R) Core(TM) i5-5200U CPU @ 2.20GHz
product name: XPS 13 9343
"""

The bot which originally found the information was a VM:
"""
architecture: x64
os version: 10.0.10586
processor count: 2
processor type: Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz
product name: VM
"""

I don't know specifics about DPI or pixel ratio. Stephen - can you help?

I'm unsure how to proceed with this issue.  I've tested on a number of different Windows machines, with and without hidpi, and so far I've not been able to reproduce this performance regression.

The only thing I've seen so far is a slowdown of a few percent, which is mostly as expected.  The test case has a large amount of divs with borders (looks like 20k, but I might be mistaken), so a smaller slowdown is expected, seeing that the patch will incur extra roundf() for border sizes, etc., but I don't know what might cause such a large performance hit.

I've investigated this a little further.  On my Linux machine, I observe a slowdown of a around 4% on flexbox-with-stretch-layout.  The remaining tests didn't exhibit a clear pattern either way.

Most of this slowdown is attributable to the fact that BorderValue now uses fixed point math, that all borders now use LayoutUnit instead of floats and extra calls to roundf().  It's possible to use the fact that BorderValue and LayoutUnit use the same fixed point representation to avoid some extra math, in exchange for readability.  In testing, this approach recovers 2% of the lost performance.  However, I'm not sure if the obfuscation is worth recovering the performance on such a synthetic test case.

I've finally observed the issue.  The device pixel ratio had to be >= 1.5 and < 2.  Fix forthcoming.

Review in https://codereview.chromium.org/2771093003/

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

commit dd547f46cfb46ea92f554aa842d06bde19ceef6e
Author: karlo <karlo@opera.com>
Date: Mon Mar 27 13:29:09 2017

Fix for performance regression on high dpi devices.

ComputedStyle::getRoundedInnerBorderFor() could generate negative
content boxes for boxes with no content and sub pixel borders that
round up (eg. 1.5px-1.99px).  Negative content boxes are illegal per
spec, and causes BoxBorderPainter::paintBorderFastPath() to bail,
and performance to be substantially degraded.

This issue would be evident on the paint-offset-changes perftest when
device pixel ratio was set to eg. 1.5.  The issue was introduced in
934becac5daa91ea979fb66e4ae21761ca11ebc9

BUG= 692955 

Review-Url: https://codereview.chromium.org/2771093003
Cr-Commit-Position: refs/heads/master@{#459770}

[modify] https://crrev.com/dd547f46cfb46ea92f554aa842d06bde19ceef6e/third_party/WebKit/Source/core/style/ComputedStyle.cpp

The fix was dead on arrival, new review in https://codereview.chromium.org/2782153002

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

commit 501d97b0c948ca7c5056016083ec5279ce1edf84
Author: karlo <karlo@opera.com>
Date: Wed Mar 29 20:59:39 2017

Fix for performance regression on high dpi devices.

ComputedStyle::getRoundedInnerBorderFor() could generate negative
content boxes for boxes with no content and sub pixel borders that
round up (eg. 1.5px-1.99px).  Negative content boxes are illegal per
spec, and causes BoxBorderPainter::paintBorderFastPath() to bail,
and performance to be substantially degraded.

This issue would be evident on the paint-offset-changes perftest when
device pixel ratio was set to eg. 1.5.  The issue was introduced in
934becac5daa91ea979fb66e4ae21761ca11ebc9

BUG= 692955 

Review-Url: https://codereview.chromium.org/2782153002
Cr-Commit-Position: refs/heads/master@{#460534}

[modify] https://crrev.com/501d97b0c948ca7c5056016083ec5279ce1edf84/third_party/WebKit/Source/core/style/ComputedStyle.cpp

win-high-dpi appears to be back to normal performance on blink_perf.paint after the patch.