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

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


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

chromium-rel-win8-dual

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

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

Hi pdr@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 : pdr
  Commit : 79fe848764520df822d23d9a234a7710a7ebfe83
  Date   : Fri Jul 28 03:03:35 2017
  Subject: Update transform tree scroll offset from PushScrollUpdatesFromMainThread

Bisect Details
  Configuration: win_8_perf_bisect
  Benchmark    : thread_times.tough_scrolling_cases
  Metric       : thread_total_all_cpu_time_per_frame/text_constant_full_page_raster_10000_pixels_per_second
  Change       : 4.25% | 15.1817380812 -> 15.8274709585

Revision             Result                   N
chromium@490187      15.1817 +- 0.82493       9      good
chromium@490222      15.3766 +- 0.626343      6      good
chromium@490227      15.682 +- 0.784186       6      good
chromium@490228      16.7496 +- 0.626125      6      bad       <--
chromium@490229      16.7329 +- 0.690172      6      bad
chromium@490231      16.676 +- 0.672466       6      bad
chromium@490239      16.9401 +- 0.887869      6      bad
chromium@490256      16.1501 +- 0.354325      6      bad
chromium@490325      15.8275 +- 0.709934      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=text.constant.full.page.raster.10000.pixels.per.second thread_times.tough_scrolling_cases

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

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


For feedback, file a bug with component Speed>Bisection

This was a real bug that was fixed and the perf change is sort of expected because we can do more work in some cases. The change is small and I think acceptable. There do not seem to be perf issues other than this test: https://chromeperf.appspot.com/group_report?rev=490228

Did we have an equivalent decrease in the previous change, where we had accidentally removed updating PropertyTrees in these cases? In which case there shouldn't be any overall change.

This did fix that regression ( crbug.com/747104 ) but introduced this new one. We now understand that the trees were not invalidated when they should have been, so I can imagine perf changing as a result of this change.

I was just trying to understand what the impact was from before [1]. IIUC [2] was just restoring what the code did before [1]. Or did I miss something?

[1]: https://chromium-review.googlesource.com/c/562556
[2]: https://chromium-review.googlesource.com/c/583565

I didn't test this but I think the perf regression comes from the extra work done in DidUpdateScrollOffset vs the very local and slightly different update pre-[1] did in LayerImpl::UpdatePropertyTreeScrollOffset. For example, the pre-[1] LayerImpl::UpdatePropertyTreeScrollOffset did not invalidate scrollbar geometries and did not mark the property trees as changed. In other words, [2] is really a bugfix and not just returning us to pre-[1].

Makes sense. I wouldn't expect the scrollbar geometries invalidation to occur with this change (happens only in single-threaded but the renderer is always threaded), but the rest of the fixes would have an impact. Thanks for the clarification!

 Issue 751974  has been merged into this issue.