Issue metadata
Sign in to add a comment
|
440ms startup performance regression in TtsEngineExtensionObserver::OnListenerAdded() |
||||||||||||||||||||||
Issue descriptionData from the UMA Sampling Profiler shows that the canary population experienced an average regression of 440ms over the first 30 seconds of startup in 72.0.3624.0 canary Windows 64-bit browser process UI thread The regression was detected between versions 72.0.3623.0 and 72.0.3624.0. The following CL is suspected to have caused the regression because it touched functions that regressed or related code: revision: 840d953a618f4ff059f1b1027cc93dc0655228a4 summary: Refactors TTS controller to content from chrome. author: katie@chromium.org Please refer to the detailed execution profiles for TtsEngineExtensionObserver::OnListenerAdded() to understand what caused the regression. 72.0.3623.0: https://uma.googleplex.com/p/chrome/callstacks?sid=4eb6322c78f5213e413469716c4afd65 72.0.3624.0: https://uma.googleplex.com/p/chrome/callstacks?sid=21d4f59d16ccae2dda73344aaa4f57fe
,
Dec 3
Thanks Dominic for help figuring out where this came from. Change coming soon.
,
Dec 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/257151ca52226cee4825d69c8ee42c170717fdea commit 257151ca52226cee4825d69c8ee42c170717fdea Author: Katie D <katie@chromium.org> Date: Mon Dec 03 20:11:54 2018 Don't create a TTS platform impl if none exists on VoicesChanged. This fixes a regression where the platform impl was created even when it wasn't yet needed. Bug: 911184 Change-Id: I6555a81ff86327fd12994bf4de5e44074dfeee15 Reviewed-on: https://chromium-review.googlesource.com/c/1358919 Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Commit-Queue: Katie Dektar <katie@chromium.org> Cr-Commit-Position: refs/heads/master@{#613207} [modify] https://crrev.com/257151ca52226cee4825d69c8ee42c170717fdea/chrome/browser/speech/tts_controller_delegate_impl.cc
,
Dec 3
,
Dec 11
The regression is gone in 73.0.3630.0: https://uma.googleplex.com/p/chrome/callstacks?sid=d6cc92e736fc786ac75621e04e2efe91 Thanks for the fix!
,
Dec 14
I just realized that this regression remains in the branched dev release: https://uma.googleplex.com/p/chrome/callstacks?sid=43dff8ff119e37952f14c260d26c88e0 Katie, can you merge the fix to the release branch?
,
Dec 17
,
Dec 17
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 17
approved for M72, branch: 3626
,
Dec 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9b49a3abcbfab833f326ad77e442f63222d1e19f commit 9b49a3abcbfab833f326ad77e442f63222d1e19f Author: Katie D <katie@chromium.org> Date: Mon Dec 17 18:56:14 2018 Cherry-pick to M72: Don't create a TTS platform impl if none exists on VoicesChanged. This fixes a regression where the platform impl was created even when it wasn't yet needed. Bug: 911184 Change-Id: I6555a81ff86327fd12994bf4de5e44074dfeee15 Reviewed-on: https://chromium-review.googlesource.com/c/1358919 Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Commit-Queue: Katie Dektar <katie@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613207}(cherry picked from commit 257151ca52226cee4825d69c8ee42c170717fdea) TBR: dmazzoni@chromium.org Reviewed-on: https://chromium-review.googlesource.com/c/1380691 Reviewed-by: Katie Dektar <katie@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#397} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/9b49a3abcbfab833f326ad77e442f63222d1e19f/chrome/browser/speech/tts_controller_delegate_impl.cc
,
Dec 19
Here's a summary of the rules that were executed: - OnlyMergeApprovedChange: Rule Failed -- Revision 9b49a3abcbfab833f326ad77e442f63222d1e19f was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! Please explain why this change was merged to the branch! - AcknowledgeMerge: Notification Required --
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9b49a3abcbfab833f326ad77e442f63222d1e19f Commit: 9b49a3abcbfab833f326ad77e442f63222d1e19f Author: katie@chromium.org Commiter: katie@chromium.org Date: 2018-12-17 18:56:14 +0000 UTC Cherry-pick to M72: Don't create a TTS platform impl if none exists on VoicesChanged. This fixes a regression where the platform impl was created even when it wasn't yet needed. Bug: 911184 Change-Id: I6555a81ff86327fd12994bf4de5e44074dfeee15 Reviewed-on: https://chromium-review.googlesource.com/c/1358919 Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Commit-Queue: Katie Dektar <katie@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613207}(cherry picked from commit 257151ca52226cee4825d69c8ee42c170717fdea) TBR: dmazzoni@chromium.org Reviewed-on: https://chromium-review.googlesource.com/c/1380691 Reviewed-by: Katie Dektar <katie@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#397} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Dec 26
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by katie@chromium.org
, Dec 3