Issue metadata
Sign in to add a comment
|
38.0ms render process startup (and jank) regression in SpellCheck::Initialize() |
||||||||||||||||||||||
Issue descriptionData from the UMA Sampling Profiler shows that SpellCheck::Initialize() runtime regressed during renderer process main thread startup on 64-bit Chrome on Windows by 38.0ms on average. This occurred in the 70.0.3532.2 Canary release. The regression is non-uniform across the userbase and is resulting in significant jank: 700ms at the 99th %ile and 2.5s at the 99.9th %ile. See attached SpellCheckJank.png for more detail. Suspected CL: Fix first word spellcheck. ganenkokb@yandex-team.ru e7f35a93db7e02a177ee4e768dd8c66e3043d7c4 Execution profile difference of Canary 70.0.3530.0 vs. 70.0.3532.2: https://uma.googleplex.com/p/chrome/callstacks?sid=33ed3ad7c755c5a8cdafbba95cc60a26 (Google-internal link; see attached SpellCheckProfile.png for the same profile information.) Assigning to groby@ who reviewed the original CL.
,
Sep 12
I don't see an easy way to address this - in that context, I'd recommend a rollback for the CL, unless ganenkob@ has a better solution? +yyushkina FYI
,
Sep 17
Seems like we should go ahead and revert the original CL, and try to merge the revert into M70. groby, do you want to do the honors?
,
Sep 17
I'm currently somewhat drowning in administrivia. Happy to rubberstamp a revert, but can't get to it myself soon-ish. Would you mind?
,
Sep 17
Sure, I'll push it through.
,
Sep 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5fe8de1abd4bd5a1d87d5e7e801b3949adf35230 commit 5fe8de1abd4bd5a1d87d5e7e801b3949adf35230 Author: Mike Wittman <wittman@chromium.org> Date: Mon Sep 17 21:26:02 2018 Revert "Fix first word spellcheck." This reverts commit e7f35a93db7e02a177ee4e768dd8c66e3043d7c4. Reason for revert: Causes significant startup and jank regressions. Original change's description: > Fix first word spellcheck. > > Do not process pending request if initialize call is come with not inited > dictionaries. > > R=groby@chromium.org > > Bug: 850424 > Change-Id: I0dd7213f9865b2c5949fd6367c524edfd09febf1 > Reviewed-on: https://chromium-review.googlesource.com/1085449 > Commit-Queue: Rachel Blum <groby@chromium.org> > Reviewed-by: Rachel Blum <groby@chromium.org> > Cr-Commit-Position: refs/heads/master@{#585283} TBR=groby@chromium.org,ganenkokb@yandex-team.ru # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 850424, 883398 Change-Id: If2b242b87a44d40010427a3e0a6474c103f8f054 Reviewed-on: https://chromium-review.googlesource.com/1228934 Reviewed-by: Mike Wittman <wittman@chromium.org> Reviewed-by: Rachel Blum <groby@chromium.org> Commit-Queue: Mike Wittman <wittman@chromium.org> Cr-Commit-Position: refs/heads/master@{#591821} [modify] https://crrev.com/5fe8de1abd4bd5a1d87d5e7e801b3949adf35230/components/spellcheck/renderer/spellcheck.cc [modify] https://crrev.com/5fe8de1abd4bd5a1d87d5e7e801b3949adf35230/components/spellcheck/renderer/spellcheck_unittest.cc
,
Sep 19
The regression is fixed per https://uma.googleplex.com/p/chrome/callstacks?sid=679f1fa87387473450a4586b312f1ae1. We see a 39.4ms average decrease in time spent by SpellCheck::Initialize().
,
Sep 19
Requesting merge for M70: this fixes a major renderer process startup regression. This fix is a clean revert to the behavior prior to 70.0.3532.0 and has been in canary for two days.
,
Sep 19
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 19
,
Sep 20
Ping for manual review as in comment #9.
,
Sep 20
,
Sep 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6acaa334121b4607e9417eaaff7141bd90e520dc commit 6acaa334121b4607e9417eaaff7141bd90e520dc Author: Mike Wittman <wittman@chromium.org> Date: Thu Sep 20 20:58:52 2018 Revert "Fix first word spellcheck." This reverts commit e7f35a93db7e02a177ee4e768dd8c66e3043d7c4. Reason for revert: Causes significant startup and jank regressions. Original change's description: > Fix first word spellcheck. > > Do not process pending request if initialize call is come with not inited > dictionaries. > > R=groby@chromium.org > > Bug: 850424 > Change-Id: I0dd7213f9865b2c5949fd6367c524edfd09febf1 > Reviewed-on: https://chromium-review.googlesource.com/1085449 > Commit-Queue: Rachel Blum <groby@chromium.org> > Reviewed-by: Rachel Blum <groby@chromium.org> > Cr-Commit-Position: refs/heads/master@{#585283} TBR=groby@chromium.org,ganenkokb@yandex-team.ru # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 850424, 883398 Change-Id: If2b242b87a44d40010427a3e0a6474c103f8f054 Reviewed-on: https://chromium-review.googlesource.com/1228934 Reviewed-by: Mike Wittman <wittman@chromium.org> Reviewed-by: Rachel Blum <groby@chromium.org> Commit-Queue: Mike Wittman <wittman@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#591821}(cherry picked from commit 5fe8de1abd4bd5a1d87d5e7e801b3949adf35230) Reviewed-on: https://chromium-review.googlesource.com/1237261 Reviewed-by: Xi Cheng <chengx@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#551} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/6acaa334121b4607e9417eaaff7141bd90e520dc/components/spellcheck/renderer/spellcheck.cc [modify] https://crrev.com/6acaa334121b4607e9417eaaff7141bd90e520dc/components/spellcheck/renderer/spellcheck_unittest.cc |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by wittman@chromium.org
, Sep 1254.7 KB
54.7 KB View Download
188 KB
188 KB View Download