New issue
Advanced search Search tips

Issue 641280 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

3.8% regression in speedometer at 414366:414383

Project Member Reported by nikolaos@chromium.org, Aug 26 2016

Issue description

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

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


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

chromium-rel-win7-x64-dual
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Aug 26 2016

Cc: brucedaw...@chromium.org
Owner: brucedaw...@chromium.org

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

Hi brucedawson@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 : Adjust skia build settings to match gyp, improve perf
Author  : brucedawson
Commit description:
  
When official builds switched from gyp to gn some smoothness tests
regressed. Investigation showed that some skia functions were taking
up to 15x longer to execute. This was due to /GL /O2 being changed to
/O1 (LTCG and optimize-for-speed changing to optimize-for-size). This
change resolves most of the compiler option differences in skia and
dramatically reduces the CPU usage in the gn build, from 9.4 s to 5.3,
within spitting distance of the gyp build.

This increases the size of chrome_child.dll from 48,593,408 to
49,213,440 bytes. The gyp version is 49,032,192 bytes and if necessary
some more size optimizations can be applied to the gn build.

The discrepancy between gyp and gn optimization settings in skia occurred
when crrev.com/1410883008 landed for gyp only.

There may be other differences - investigation continues - but this fixes
the main issues noticed.

BUG= 632651 

Review-Url: https://codereview.chromium.org/2270693006
Cr-Commit-Position: refs/heads/master@{#414368}
Commit  : 3e80255c5139698d5fca9d8f1595a307f6cb3639
Date    : Thu Aug 25 09:11:12 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N   Good?
chromium@414365  990.793  7.15986  18  good
chromium@414367  994.231  8.41397  8   good
chromium@414368  1009.64  7.39037  5   bad    <--
chromium@414370  1005.23  15.9906  5   bad
chromium@414374  999.477  11.983   18  bad
chromium@414383  1002.41  7.86647  12  bad

Bisect job ran on: win_x64_perf_bisect
Bug ID: 641280

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --also-run-disabled-tests speedometer
Test Metric: EmberJS-TodoMVC/EmberJS-TodoMVC
Relative Change: 1.86%
Score: 99.5

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/win_x64_perf_bisect/builds/1446
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9003280249604656320


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

| 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!
What's the expected precision of the speedometer benchmark? The 3.8% regression looks like it is very near the noise level, and comment #3 says the change is actually 1.86%.

I'm also curious as to what sort of things speedometer measures. My CL changed the skia code so it was optimized for speed instead of size. This made it get larger so it now takes longer to load from disk and can cause more cache pressure, so it could make some startup scenarios and scenarios that make very light use of skia run slightly slower. Does this make sense?

If my change is responsible then I think we should close this as won't fix because the benefits (up to 16x performance improvement on some rendering pathways) outweigh the costs.

Thoughts?

I agree that speedometer numbers are in general pretty flaky.
In addition, this particular report seems to be at the noise level.
However, the sheriff's job is to report such things and draw the committers' attention.

If you think that this should be a WontFix, let's do that.
Status: WontFix (was: Assigned)
I think the change is close to the noise level and also falls in the unknowable/unpredictable area - my change switched parts of skia to optimize-for-speed in order to fix a large performance regression and larger code will, unfortunately, sometimes make other scenarios slightly slower. Not an ideal resolution, but I think it is correct.

Sign in to add a comment