New issue
Advanced search Search tips

Issue 759693 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 727518
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

11.5%-14.5% regression in media_perftests at 493984:497652

Project Member Reported by dalecurtis@google.com, Aug 28 2017

Issue description

media_perftests performance seems to change significantly ~10-15% with the 2017 MSVC++ update. These are micro benchmarks so it's not a release blocker or anything, but they often signal some larger change.

https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg5dLqjgsM
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Aug 28 2017

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

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


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

chromium-rel-win10
chromium-rel-win7-gpu-ati
chromium-rel-win7-gpu-nvidia
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Aug 28 2017

Cc: brucedaw...@chromium.org

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

Hi brucedawson@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 : Bruce Dawson
  Commit : ec9a1e6ce2708f93ea08f82dd76a1f556310af91
  Date   : Fri Aug 25 23:55:03 2017
  Subject: Change default VC++ compiler from 2015 to 2017

Bisect Details
  Configuration: winx64_10_perf_bisect
  Benchmark    : media_perftests
  Metric       : sinc_resampler_convolve/unoptimized_aligned
  Change       : 11.33% | 36305.1313253 -> 32190.2872629

Revision             Result                  N
chromium@497543      36305.1 +- 351.952      6      good
chromium@497591      36378.1 +- 17.3361      6      good
chromium@497597      36080.2 +- 1554.26      6      good
chromium@497598      36367.7 +- 50.3155      6      good
chromium@497599      31638.0 +- 12.8387      6      bad       <--
chromium@497600      31643.3 +- 32.4605      6      bad
chromium@497603      31633.5 +- 42.6541      6      bad
chromium@497615      31613.8 +- 68.1683      6      bad
chromium@497638      32190.3 +- 49.5241      6      bad

To Run This Test
  .\src\out\Release_x64\media_perftests.exe --single-process-tests

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

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


For feedback, file a bug with component Speed>Bisection
Mergedinto: 727518
Status: Duplicate (was: Assigned)
This is a duplicate of 727518. The media portion of that bug is simultaneously well understood and completely mysterious. Examination of the code-gen of the crucial functions shows that the only differences are movaps versus movups. So, perfectly understood. Except that Intel/AMD both promise that there should be zero (or practically zero) performance difference from this change.

My best guess is that these microbenchmarks hit these instructions hard enough that they hit some new limitations, perhaps issue ports or some-such, so that even though the individual instructions are just as fast they end up hitting some obscure bottleneck.

Anyway, duplicate, and I'm glad it's not a release blocker.

Sign in to add a comment