Download history being loaded at browser startup |
|||||
Issue descriptionLooks like we have a dependency chain from chrome::ProfileImpl -> content::StoragePartitionImpl -> content::BackgroundFetchContext -> chrome::BackgroundFetchDelegateImpl -> download::DownloadService -> download::BuildDownloadService() -> content::DownloadManagerImpl The action of getting the download manager in download::BuildDownloadService() results in invoking DownloadCoreServiceImpl::GetDownloadManagerDelegate(). The latter instantiates the HistoryService and DownloadHistory. DownloadHistory loads the downloads history, and upon completion, initiates the download history file existence check (DownloadManager::CheckForHistoryFilesRemoval) All this happens at every browser startup. Loading the entire downloads history and checking for the existence of all the downloads is quite expensive. We could make this a little easier on browser startup by not loading the download history, or at least forgoing the file existence check until someone loads chrome://downloads. Note that opening the downloads page or running an extension that invokes download.search() independently invoke the history file existence check. We may be able to trivially get rid of the same check at browser startup.
,
Apr 4 2018
,
Apr 5 2018
Another thing to think about is to delay loading the background fetch service. Unless it needs to start work right away (unlikely), it could be delayed until sometime after browser startup.
,
Apr 5 2018
Perhaps I'm misreading the suggestion, but we shouldn't be doing the existence checks at all if we don't need to. Even if it's out of the immediate startup path, it would still impact performance generally (shortly after startup). And on top of that, we really should have a baseline of not accessing a resource until and unless we need to (both for performance and to avoid the kind of confusion we just dealt with). I'm also bumping the priority on this bug and setting a target milestone, but ideally I'd like to see this fixed sooner and merged to both M66 and M67.
,
Apr 5 2018
#4: Yup. The suggestion in the short run is to skip the existence check at the time downloads history finishes loading. The places where it's needed (chrome://downloads, download home, and download extensions that use download.search()) already invoke the existence check manually. The additional suggestion for delaying background fetch service is to reduce the work the browser has to do at profile creation time. Background fetches don't need to activate immediately, so it can yield to other higher priority tasks.
,
Apr 5 2018
asanka@ - Gotcha, sorry for misreading that.
,
Apr 6 2018
,
Apr 6 2018
If you are hitting this issue and you want a fix right now then go to chrome://downloads in your browser, go to the menu in the top right, and select Clear All. That will clear Chrome's list of downloaded files so that it won't have any files to existence-check at startup. If you have a large list of downloaded files then this will improve startup time slightly. This will be fixed so if you do nothing the problem will also go away, once the fix ships to your version of Chrome.
,
Apr 9 2018
+ConOps hotlist as this may be related to some of our "Chrome is slow at startup" reports.
,
Apr 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dee931eba042a6023229ec856c987f195956aacb commit dee931eba042a6023229ec856c987f195956aacb Author: Min Qin <qinmin@chromium.org> Date: Wed Apr 11 02:09:47 2018 Don't perform file existence check when loading DownloadHistory DownloadHistory is currently loaded on browser start up. And that will trigger file existence check, which is very costly. File existence check is not needed until user opens chrome://downloads page or loads a download extension. Currently android download home and desktop downloads page already checks for file existence on startup. And download search api will also perform file existence check. So this file existence check on DownloadHistory ctor is not needed. BUG=829044 Change-Id: I3558d2baa6952eda6adde4eec1f3c79efe8e4d11 Reviewed-on: https://chromium-review.googlesource.com/996576 Commit-Queue: Min Qin <qinmin@chromium.org> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Reviewed-by: Xing Liu <xingliu@chromium.org> Reviewed-by: Asanka Herath <asanka@chromium.org> Cr-Commit-Position: refs/heads/master@{#549725} [modify] https://crrev.com/dee931eba042a6023229ec856c987f195956aacb/chrome/browser/download/download_browsertest.cc [modify] https://crrev.com/dee931eba042a6023229ec856c987f195956aacb/chrome/browser/download/download_history.cc [modify] https://crrev.com/dee931eba042a6023229ec856c987f195956aacb/chrome/browser/download/download_history_unittest.cc [modify] https://crrev.com/dee931eba042a6023229ec856c987f195956aacb/chrome/browser/download/download_ui_controller_unittest.cc [modify] https://crrev.com/dee931eba042a6023229ec856c987f195956aacb/chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by asanka@chromium.org
, Apr 4 2018