Issue metadata
Sign in to add a comment
|
Regression: Delay is seen in zipping a file for the first time |
||||||||||||||||||||||
Issue descriptionChrome Version: 72.0.3612.0/11278.0.0 dev-channel Kip,Daisy,Celes OS: Chrome OS What steps will reproduce the problem? (1)Sign-in to user>> Open Files App and take a screenshot (2)Zip the screenshot and observe delay is seen in zipping a file for the first time Actual: Delay is seen in zipping a file for the first time Expected: No such delay should be seen This is a Regression issue as same is working fine in M71 beta Attaching screencasts for reference.. NOTE: Issue is not seen from second time
,
Dec 10
,
Dec 14
What's happening is that the first time a user runs some zip functionality, the zip archiver NaCl module is loaded which is translated from PNaCl. The PNaCl translation is what's taking a long time. The archiver used to load the NaCl module always on login, which is inefficient, but it had the side effect of causing a PNaCl translation the first time a user logs in. Note, the translation will only happen the first time because Chrome OS caches the translated NaCl binary. Going back to always loading the NaCl module on every login is an unnecessary inefficiency (and causes significant slowdowns in browser tests). However, doing it purely lazily causes this regression on first use. A compromise is to do it the first time a user logs in or when Chrome OS is updated. chrome.runtime.onInstalled gives us this capability.
,
Dec 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/86fa5708fc2dce1bf5f418bb640fc305b56c491e commit 86fa5708fc2dce1bf5f418bb640fc305b56c491e Author: Anand K. Mistry <amistry@chromium.org> Date: Fri Dec 14 05:50:07 2018 Preload the zip archiver NaCl module on first login or after update. Loading the module the first time causes a PNaCl translation, which is cached for subsequent loads. Loading lazily causes a performance regression the first time the user uses zip functionaility. BUG= 907032 Change-Id: Iba4ee63ec0f940a1bfaa4bbbf52801a5a5dbea5a Reviewed-on: https://chromium-review.googlesource.com/c/1377953 Reviewed-by: Noel Gordon <noel@chromium.org> Commit-Queue: Anand Mistry <amistry@chromium.org> Cr-Commit-Position: refs/heads/master@{#616590} [modify] https://crrev.com/86fa5708fc2dce1bf5f418bb640fc305b56c491e/chrome/browser/resources/chromeos/zip_archiver/js/background.js
,
Dec 14
#3 Agree, no easy compromise here. Putting things back to the way they were, with some improvement via patch #4, SGTM. A reasonable compromise, given the imperfect situation, I believe. Thank you amistry@ for taking on this bug. Not easy.
,
Dec 17
Setting NextAction to wait for canary before requesting 72 merge.
,
Dec 19
The NextAction date has arrived: 2018-12-19
,
Dec 19
Verified on canary 73.0.3644.0.
,
Dec 19
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 20
,
Dec 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdbffae85372c96c3fa311c13f7281307d505094 commit bdbffae85372c96c3fa311c13f7281307d505094 Author: Anand K. Mistry <amistry@chromium.org> Date: Thu Dec 20 23:23:47 2018 Preload the zip archiver NaCl module on first login or after update. Loading the module the first time causes a PNaCl translation, which is cached for subsequent loads. Loading lazily causes a performance regression the first time the user uses zip functionaility. BUG= 907032 Change-Id: Iba4ee63ec0f940a1bfaa4bbbf52801a5a5dbea5a Reviewed-on: https://chromium-review.googlesource.com/c/1377953 Reviewed-by: Noel Gordon <noel@chromium.org> Commit-Queue: Anand Mistry <amistry@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#616590}(cherry picked from commit 86fa5708fc2dce1bf5f418bb640fc305b56c491e) Reviewed-on: https://chromium-review.googlesource.com/c/1388044 Reviewed-by: Anand Mistry <amistry@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#492} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/bdbffae85372c96c3fa311c13f7281307d505094/chrome/browser/resources/chromeos/zip_archiver/js/background.js
,
Dec 20
Here's a summary of the rules that were executed: - OnlyMergeApprovedChange: Rule Failed -- Revision bdbffae85372c96c3fa311c13f7281307d505094 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 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdbffae85372c96c3fa311c13f7281307d505094 Commit: bdbffae85372c96c3fa311c13f7281307d505094 Author: amistry@chromium.org Commiter: amistry@chromium.org Date: 2018-12-20 23:23:47 +0000 UTC Preload the zip archiver NaCl module on first login or after update. Loading the module the first time causes a PNaCl translation, which is cached for subsequent loads. Loading lazily causes a performance regression the first time the user uses zip functionaility. BUG= 907032 Change-Id: Iba4ee63ec0f940a1bfaa4bbbf52801a5a5dbea5a Reviewed-on: https://chromium-review.googlesource.com/c/1377953 Reviewed-by: Noel Gordon <noel@chromium.org> Commit-Queue: Anand Mistry <amistry@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#616590}(cherry picked from commit 86fa5708fc2dce1bf5f418bb640fc305b56c491e) Reviewed-on: https://chromium-review.googlesource.com/c/1388044 Reviewed-by: Anand Mistry <amistry@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#492} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by amistry@chromium.org
, Nov 23Labels: CrOSFilesFeature-Zip
Status: Available (was: Untriaged)