Issue metadata
Sign in to add a comment
|
17.5%-562% regression in system_health.common_desktop at 513534:513616 |
||||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Nov 6 2017
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/125a3572f80000
,
Nov 6 2017
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/125a3572f80000 Roll HarfBuzz to 1.6.3 By drott@chromium.org · Thu Nov 02 17:49:34 2017 chromium @ d9338280305c81f3dac8aa1a3da3e459e3f341f9 Understanding performance regressions: http://g.co/ChromePerformanceRegressions
,
Nov 7 2017
The benchmark regressions are in many cases in time to first paint / time to first contentful paint metrics. Could it be that HarfBuzz now always does some additional upfront analysis work that it didn't used to do, which is only relevant for the ligature search requests? For the benchmarks listed in #1, all system_health.common_desktop regressions are only on linux release, so I believe it is most likely something about the ligatures analysis steps for FT/HarfBuzz, and sadly, it has gotten slower than before? We could experimentally disable #define FT_CONFIG_OPTION_USE_HARFBUZZ to see if this performance regression recovers even beyond the level of what it was before to see if this hypothesis is true.
,
Nov 16 2017
Behdad, I am trying to get more insights by splitting all HarfBuzz changes we've applied between 1.5.1 and 1.6.3 in basically three groups. I've set up three perf_bisect runs: https://chromium-review.googlesource.com/c/chromium/src/+/774880 HB 1.6.3 contains work on hb_set https://chromium-review.googlesource.com/c/chromium/src/+/774879 Which is 1.6.0 + some changes before the set work started, plus the ICU 60 build fixes https://chromium-review.googlesource.com/c/chromium/src/+/774878 Which is 1.5.1 plus ICU build fixes These should run the following stories, each 5 times: browse:social:twitter browse:social:twitter_infinite_scroll browse:news:cnn browse:news:hackernews And as we should check for timeToFirstMeaningfulPaint, timeToFirstContentFulPaint, timeToFirstPaint and total:500ms_window:renderer_eqt_max differences. The linux_perf_bisect bot on each of these CLs is the relevant bot to look at, and the HTML results on these bots should be available from the bot logs. This should be done latest in a few hours from now, would you take a look at these results, and see if that helps us get further?
,
Nov 16 2017
Also, could you explain what the benchmark you've been testing with is doing? I think these metrics here are not affected by raw shaping throughput, but rather by work that's done before the first character/glyph getting painted. I suspect it's mostly "setup/upfront-work costs" which goes into these. Could we modify your benchmark in that direction? I.e. set up an hb_font/hb_face, get ready to shape, and either only get ready, or shape only one character or something, not necessarily even a ligature, then discard everything and set-up again, and repeat this a few hundred times? Thanks!
,
Nov 17 2017
My benchmark was loading the font using FreeType and then just loading one glyph. Repeating this a thousand times. I'll check again tomorrow, don't have my laptop tonight. How do the bisected perf results look?
,
Nov 17 2017
It's possible to see the full results for these by going to the above CLs, clicking on the linux_perf_bisect bot, then clicking on "HTML Results". The short summary is that the bot seems to think all of these are a bit faster than tip of tree, but to varying amounts. https://chromium-review.googlesource.com/c/chromium/src/+/774880 Not very significant. https://chromium-review.googlesource.com/c/chromium/src/+/774879 cpuTimeToFirstMeaningfulPaint and total:500ms_window:renderer_eqt_cpu are significantly faster with the patch than at current tip of tree. https://chromium-review.googlesource.com/c/chromium/src/+/774878 cpuTimeToFirstMeaningfulPaint, timeToFirstContentfulPaint, timeToFirstMeaningfulPaint, timeToFirstPaint, total:500ms_window:renderer_eqt, and total:500ms_window:renderer_eqt_cpu are significantly faster with the patch than at current tip of tree.
,
Nov 17 2017
I started two more: https://chromium-review.googlesource.com/c/chromium/src/+/776879 harfBuzzPerfBisect_noOp_control - same as TOT, except no call to set_ptem https://chromium-review.googlesource.com/c/chromium/src/+/776783 change only README, set ptem I am awaiting these results to have some controls / verification. I was surprised to see that https://chromium-review.googlesource.com/c/chromium/src/+/774880 actually did have differences to trunk. The real functional difference there was only not calling hb_font_set_ptem.
,
Nov 17 2017
I've filed issue 786441 since even a no-op CL only touching a README produces 24% regressions on the timeToFirst...Paint metrics. Given these circumstance, I am doing either doing something wrong in the way I set up these perf bisects, or the tooling has some significant bugs. Primiano, can you CC people who can help with analysing what's really going on here? The HarfBuzz roll in question has important correctness fixes and we need to get a clearer picture if we really have a performance issue here or not. As mentioned in the new issue 786441 I ran the bisects using the following command: tools/perf/run_benchmark try linux system_health.common_desktop --pageset-repeat=5 --story-filter='(.*browse.*social.*twitter.*|.*browse.*news*.cnn.*|.*browse.*news.*hackernews.*)' Setting to Available for triaging in Speed>Dashboard & Speed>Telemetry.
,
Nov 17 2017
>a no-op CL only touching a README produces 24% regressions on the timeToFirst...Paint metrics +Dave/Simon: this seems liek a problem with the perf bisect.
,
Nov 21 2017
I've commented the blocking bug crbug.com/786441
,
Nov 22 2017
,
Nov 23 2017
In https://chromium-review.googlesource.com/c/chromium/src/+/788393 I've made a simple HarfBuzz performance test and ran this against the Chrome based on 1.5.1 plus ICU fixes and against 1.6.3. With this relatively simple test I can't detect any performance changes. Behdad, could you take a look a the benchmark and sanity check it? "sans-serif" 1.5.1 ICUFixes [----------] 1 test from HarfBuzzShaperTest (13238 ms total) [----------] 1 test from HarfBuzzShaperTest (13739 ms total) [----------] 1 test from HarfBuzzShaperTest (13319 ms total) [----------] 1 test from HarfBuzzShaperTest (14409 ms total) [----------] 1 test from HarfBuzzShaperTest (13265 ms total) 1.6.3 branch [----------] 1 test from HarfBuzzShaperTest (14012 ms total) [----------] 1 test from HarfBuzzShaperTest (13707 ms total) [----------] 1 test from HarfBuzzShaperTest (13335 ms total) [----------] 1 test from HarfBuzzShaperTest (13346 ms total) [----------] 1 test from HarfBuzzShaperTest (13878 ms total) ROBOTO 1.5.1 ICU fixes [==========] 1 test from 1 test case ran. (17377 ms total) [==========] 1 test from 1 test case ran. (17613 ms total) [==========] 1 test from 1 test case ran. (18169 ms total) [==========] 1 test from 1 test case ran. (18092 ms total) [==========] 1 test from 1 test case ran. (18190 ms total) 1.6.3 branch [==========] 1 test from 1 test case ran. (17447 ms total) [==========] 1 test from 1 test case ran. (18356 ms total) [==========] 1 test from 1 test case ran. (18178 ms total) [==========] 1 test from 1 test case ran. (17725 ms total) [==========] 1 test from 1 test case ran. (17608 ms total)
,
Dec 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8db92d3744fe418e7871e863ad53b58b0209e412 commit 8db92d3744fe418e7871e863ad53b58b0209e412 Author: Dominik Röttsches <drott@chromium.org> Date: Fri Dec 01 21:41:48 2017 Patch hb_set implementation back to pre 1.6.1 implementation Perf bisects indicate that the hb_set changes may be the reason for seing performance regressions in issues 781794 and 782220 . This is somewhat speculative as there were issues with reproducibility in the perf_bisect runs. Results from the main perf bot graphs should help figure out whether the hb_set changes were the culprit here. Bug: 782220 , 781794 Change-Id: I5462b37503ca76a299c99cf41388d0d195722358 Tbr: behdad@chromium.org Reviewed-on: https://chromium-review.googlesource.com/804374 Reviewed-by: Dominik Röttsches <drott@chromium.org> Commit-Queue: Dominik Röttsches <drott@chromium.org> Cr-Commit-Position: refs/heads/master@{#521069} [modify] https://crrev.com/8db92d3744fe418e7871e863ad53b58b0209e412/third_party/harfbuzz-ng/README.chromium [modify] https://crrev.com/8db92d3744fe418e7871e863ad53b58b0209e412/third_party/harfbuzz-ng/src/hb-set-private.hh [modify] https://crrev.com/8db92d3744fe418e7871e863ad53b58b0209e412/third_party/harfbuzz-ng/src/hb-set.cc
,
Dec 2 2017
,
Jan 16
(6 days ago)
,
Jan 16
(6 days ago)
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Nov 6 2017