Issue metadata
Sign in to add a comment
|
6.3%-24.1% regression in rasterize_and_record_micro.top_25 at 505470:505503 |
||||||||||||||||||||
Issue descriptionGrouped with regressions in: - thread_times.tough_scrolling_cases/thread_raster_cpu_time_per_frame - smoothness.tough_animation_cases which I think are related.
,
Oct 2 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8966816789041651312
,
Oct 2 2017
=== Auto-CCing suspected CL author jschuh@chromium.org === Hi jschuh@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 : Justin Schuh Commit : 3725ec39917fea4cc350f0abb8dde4552e955054 Date : Sun Oct 01 03:44:27 2017 Subject: Make ClampedNumeric and CheckedNumeric constexpr Bisect Details Configuration: win_perf_bisect Benchmark : rasterize_and_record_micro.top_25 Metric : record_time/file___static_top_25_twitter.html Change : 27.84% | 0.111333333333 -> 0.142333333333 Revision Result N chromium@505470 0.111333 +- 0.0011547 6 good chromium@505487 0.1125 +- 0.003937 6 good chromium@505491 0.111667 +- 0.0011547 6 good chromium@505493 0.112333 +- 0.00522813 6 good chromium@505494 0.143333 +- 0.00416333 6 bad <-- chromium@505495 0.142167 +- 0.000912871 6 bad chromium@505503 0.142333 +- 0.0011547 6 bad To Run This Test src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=file...static.top.25.twitter.html rasterize_and_record_micro.top_25 More information on addressing performance regressions: http://g.co/ChromePerformanceRegressions Debug information about this bisect: https://chromeperf.appspot.com/buildbucket_job_status/8966816789041651312 For feedback, file a bug with component Speed>Bisection
,
Oct 3 2017
The MSVC performance regression isn't surprising, and should be fixed by reordering the saturation calculation. The Mac one doesn't seem like it should be related, since Clang takes the intrinsic path.
,
Oct 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ffc71c032540fc188f8b8104b1b26387d3e18a1b commit ffc71c032540fc188f8b8104b1b26387d3e18a1b Author: Justin Schuh <jschuh@chromium.org> Date: Tue Oct 03 17:05:26 2017 Reorder saturation calculation for ClampAddOp and ClampSupOp The order of operations changed in crrev.com/505494 with C++14 constexpr optimizations. However, that change appears to have regressed performance in some cases. So, this CL tweaks the code to match the old structure and trigger the same compiler optimization heuristics. Bug: 770832 Change-Id: I28f57ddf53825619eb73c9a92c663fc8463e16a7 Reviewed-on: https://chromium-review.googlesource.com/696721 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Justin Schuh <jschuh@chromium.org> Cr-Commit-Position: refs/heads/master@{#506078} [modify] https://crrev.com/ffc71c032540fc188f8b8104b1b26387d3e18a1b/base/numerics/clamped_math_impl.h
,
Oct 3 2017
,
Oct 4 2017
,
Oct 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95a989c109f41b856fba698bd0c96bfcd69f78d9 commit 95a989c109f41b856fba698bd0c96bfcd69f78d9 Author: Justin Schuh <jschuh@chromium.org> Date: Wed Oct 04 23:24:37 2017 Revert "Reorder saturation calculation for ClampAddOp and ClampSupOp" This reverts commit ffc71c032540fc188f8b8104b1b26387d3e18a1b. Reason for revert: Didn't improve the perf issue. Original change's description: > Reorder saturation calculation for ClampAddOp and ClampSupOp > > The order of operations changed in crrev.com/505494 with C++14 > constexpr optimizations. However, that change appears to have regressed > performance in some cases. So, this CL tweaks the code to match the > old structure and trigger the same compiler optimization heuristics. > > Bug: 770832 > Change-Id: I28f57ddf53825619eb73c9a92c663fc8463e16a7 > Reviewed-on: https://chromium-review.googlesource.com/696721 > Reviewed-by: Tom Sepez <tsepez@chromium.org> > Commit-Queue: Justin Schuh <jschuh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#506078} TBR=jschuh@chromium.org,tsepez@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 770832 Change-Id: I076c93215b422e321ad4bf39a72bab144cc0195d Reviewed-on: https://chromium-review.googlesource.com/700775 Reviewed-by: Justin Schuh <jschuh@chromium.org> Commit-Queue: Justin Schuh <jschuh@chromium.org> Cr-Commit-Position: refs/heads/master@{#506559} [modify] https://crrev.com/95a989c109f41b856fba698bd0c96bfcd69f78d9/base/numerics/clamped_math_impl.h
,
Oct 6 2017
,
Oct 6 2017
Issue 772279 has been merged into this issue.
,
Oct 6 2017
Issue 772279 has been merged into this issue.
,
Oct 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5ec7403b4e7843336ec726a4668ae78cc5ed53e2 commit 5ec7403b4e7843336ec726a4668ae78cc5ed53e2 Author: Justin Schuh <jschuh@chromium.org> Date: Thu Oct 12 23:58:42 2017 Simplify ClampedSubOp and ClampedAddOp This is a speculative change to track down a potential performance regression on MSVC. It reverts changes from crrev.com/505494 such that the remaining change has no other effect than applying constexpr. Bug: 770832 Change-Id: Ib700270bbf59002336f16b63acbb3b91c00a24dd Reviewed-on: https://chromium-review.googlesource.com/711114 Reviewed-by: Bruce Dawson <brucedawson@chromium.org> Commit-Queue: Justin Schuh <jschuh@chromium.org> Cr-Commit-Position: refs/heads/master@{#508551} [modify] https://crrev.com/5ec7403b4e7843336ec726a4668ae78cc5ed53e2/base/numerics/clamped_math_impl.h
,
Oct 13 2017
The MSVC regression is fixed (and now constexpr appears to be providing a performance win on average). My best guess for what happened is that the MSVC optimizer just wasn't doing a good job of folding the compile-time constants in the more complex saturation computation. The mac-air11 regression just doesn't make any sense to me, and I don't think is related to my change, because clang is doing the right thing everywhere else. So, unless anyone has any bright ideas I'm going to close this.
,
Oct 17 2017
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Oct 2 2017