Add a persistent database to store server hints provided by ComponentUpdater |
||||||
Issue descriptionThe server hints provided by the Cacao server are stored on disk by component updater. This solution has worked well so far, however, going forward, this has many limitations: 1. Currently, all the hints stored by the component updater are read to the memory at Chrome startup. To avoid regressing memory, the current mechanism places a tight bound on how many hints we can push using the component updater. 2. Chrome will soon use one-platform API to pull user-specific and navigation specific hints. Those hints can't be stored in the component updater, and a separate place is needed to store them so they can be reused across browser restarts. To increase the coverage of server hints to more webpages, a persistent database (leveldb or SQLite) needs to be added to Chrome that can store hints pushed by component updater as well as hints pulled using the one platform API. The database format should support adding of new fields to the server hints proto, without requiring significant effort to update the database.
,
Sep 25
Taking this and reducing scope of this bug for initial version that supports scaling up Component provided data (prior to having server API available).
,
Oct 10
We decided to not do persistent db for just the ComponentUpdater after all. Jered taking over HintCache persistance so moving this bug to him. He can decide if this bug gets expanded scope, duped, or marked wontfix.
,
Oct 31
,
Dec 17
Refreshed during triage. I understand from Jered that this is coded and work on test in underway now.
,
Jan 14
Not sure why this is RVG. I am going to remove that label.
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dedfb57f58577ece7bbd584049590ff094f4d7d5 commit dedfb57f58577ece7bbd584049590ff094f4d7d5 Author: Jered Gray <jegray@google.com> Date: Wed Jan 16 23:02:39 2019 Implement HintCacheStore and HintCacheLevelDBStore An abstract HintCacheStore base class and concrete HintCacheLevelDBStore derived class have been added to provide a hint backing store for the HintCache, which will enable the number of hints sent down to the client in hint components to significantly expand. The HintCacheStore provides the cache the ability to query the availability of a hint for a specific host suffix and to asynchronously load individual hints on demand. The HintCacheLevelDBStore database is composed of metadata entries that include basic store information, such as its schema version and its component version, and component hint entries. Eventually, the HintCacheLevelDBStore will be expanded to include OP API hints as well. Unittests have been added for HintCacheLevelDBStore. Bug: 888097 Change-Id: Ibfeb52ada5cdf1a4a17395611f4bd85c711ccb62 Reviewed-on: https://chromium-review.googlesource.com/c/1403335 Reviewed-by: Tommy Nyquist <nyquist@chromium.org> Reviewed-by: Tarun Bansal <tbansal@chromium.org> Reviewed-by: Stephen Chenney <schenney@chromium.org> Reviewed-by: Doug Arnett <dougarnett@chromium.org> Commit-Queue: Jered Gray <jegray@chromium.org> Cr-Commit-Position: refs/heads/master@{#623425} [modify] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/BUILD.gn [modify] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/DEPS [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/hint_cache_leveldb_store.cc [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/hint_cache_leveldb_store.h [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/hint_cache_leveldb_store_unittest.cc [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/hint_cache_store.h [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/proto/BUILD.gn [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/proto/DEPS [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/proto/hint_cache.proto [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/proto/hints_header_include.h [modify] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/tools/metrics/histograms/histograms.xml
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dedfb57f58577ece7bbd584049590ff094f4d7d5 commit dedfb57f58577ece7bbd584049590ff094f4d7d5 Author: Jered Gray <jegray@google.com> Date: Wed Jan 16 23:02:39 2019 Implement HintCacheStore and HintCacheLevelDBStore An abstract HintCacheStore base class and concrete HintCacheLevelDBStore derived class have been added to provide a hint backing store for the HintCache, which will enable the number of hints sent down to the client in hint components to significantly expand. The HintCacheStore provides the cache the ability to query the availability of a hint for a specific host suffix and to asynchronously load individual hints on demand. The HintCacheLevelDBStore database is composed of metadata entries that include basic store information, such as its schema version and its component version, and component hint entries. Eventually, the HintCacheLevelDBStore will be expanded to include OP API hints as well. Unittests have been added for HintCacheLevelDBStore. Bug: 888097 Change-Id: Ibfeb52ada5cdf1a4a17395611f4bd85c711ccb62 Reviewed-on: https://chromium-review.googlesource.com/c/1403335 Reviewed-by: Tommy Nyquist <nyquist@chromium.org> Reviewed-by: Tarun Bansal <tbansal@chromium.org> Reviewed-by: Stephen Chenney <schenney@chromium.org> Reviewed-by: Doug Arnett <dougarnett@chromium.org> Commit-Queue: Jered Gray <jegray@chromium.org> Cr-Commit-Position: refs/heads/master@{#623425} [modify] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/BUILD.gn [modify] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/DEPS [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/hint_cache_leveldb_store.cc [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/hint_cache_leveldb_store.h [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/hint_cache_leveldb_store_unittest.cc [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/hint_cache_store.h [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/proto/BUILD.gn [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/proto/DEPS [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/proto/hint_cache.proto [add] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/components/previews/content/proto/hints_header_include.h [modify] https://crrev.com/dedfb57f58577ece7bbd584049590ff094f4d7d5/tools/metrics/histograms/histograms.xml
,
Jan 17
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947 commit 1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947 Author: Jered Gray <jegray@google.com> Date: Thu Jan 17 00:41:58 2019 Integrate HintCacheStore into Previews The HintCache has been updated to use a HintCacheStore, which it owns. It uses the hint backing store to query for the availability of hints on the client and to asynchronously load hints on demand. The cache also contains an MRU memory cache, which retains the most recently loaded hints for synchronous access. PreviewsOptimizationGuide creates both the hint cache and HintCacheLevelDBStore (the concrete derived class of HintCacheStore that we currently use) on construction and owns the hint cache. This allows the logic to retain the same cache and store through component updates, something that would not be possible if PreviewsHints owned them. The optimization guide does not register for hint component updates until after the cache is fully initialized, guaranteeing that all information that PreviewsHints::Create() needs for component processing is available. The component processing logic itself in PreviewsHints::Create() has been modified to use a ComponentUpdateData object provided by the hint cache store. This will be a nullptr when the component is not newer than the cache already has, allowing all component hint processing to be skipped; otherwise, hints from the component are moved into ComponentUpdateData, allowing us to eliminate superfluous hint copies and allocations and to minimize processing that must occur on the UI thread. The NOSCRIPT logic in PreviewsDeciderImpl::ShouldAllowPreviewPerOptimizationHints has been modified to match the RESOURCE_LOADING_HINTS logic to account for the fact that NoScript page hints may need to be asynchronously loaded. Both types now rely on the commit time check. Additionally, a command line switch, "purge_hint_cache_store", has been added. When it is included, the existing HintCacheStore data is purged during startup. Various unittests have been updated to account for the fact that loading hints from HintCache is now asynchronous and new unittests have been added to test the new functionality. Bug: 888097 Change-Id: I258980322b3ae2cfbfd4cc69bd536262ed1049f4 Reviewed-on: https://chromium-review.googlesource.com/c/1402283 Commit-Queue: Jered Gray <jegray@chromium.org> Reviewed-by: Tarun Bansal <tbansal@chromium.org> Reviewed-by: Doug Arnett <dougarnett@chromium.org> Cr-Commit-Position: refs/heads/master@{#623460} [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/chrome/browser/previews/previews_service.cc [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/optimization_guide/optimization_guide_service.h [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/BUILD.gn [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/hint_cache.cc [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/hint_cache.h [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/hint_cache_unittest.cc [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/previews_decider_impl.cc [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/previews_decider_impl_unittest.cc [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/previews_hints.cc [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/previews_hints.h [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/previews_hints_unittest.cc [add] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/previews_hints_util.cc [add] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/previews_hints_util.h [add] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/previews_hints_util_unittest.cc [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/previews_optimization_guide.cc [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/previews_optimization_guide.h [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/content/previews_optimization_guide_unittest.cc [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/core/previews_switches.cc [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/components/previews/core/previews_switches.h [modify] https://crrev.com/1b7c45f89ed5f3fdd3a04fe8f35ae6fc3ecb8947/tools/metrics/histograms/enums.xml |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by tbansal@chromium.org
, Sep 25Status: Available (was: Untriaged)