Consider speculative ruleset lookups in ActivationStateComputingNavigationThrottle |
|||
Issue descriptionRight now this throttle always delays a navigation in WillProcessResponse to asynchronously check the ruleset. This can be the cause of a lot of slowness. See SubresourceFilter.DocumentLoad.ActivationComputingDelay* histograms. Another approach would be to speculatively perform a lookup at every URL we see in the URL chain (just like the safe browsing activation throttle). Note that if ad tagging is disabled, we should not be checking main frame navs before we know the page has activation. My guess is that something like this is needed if we ever want ad tagging on all requests.
,
Mar 13 2018
I can look into this briefly, though I may not end up doing the full work.
,
Mar 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7749896f385dc3f6cba8c8fd4ac0364c7099e55f commit 7749896f385dc3f6cba8c8fd4ac0364c7099e55f Author: Charles Harrison <csharrison@chromium.org> Date: Tue Mar 20 22:00:42 2018 [subresource_filter] Make subframe activation throttles speculative The ActivationStateComputingThrottle computes the ActivationState for a given frame. Normally, we wait until WillProcessResponse and delay the navigation while we compute the ActivationState asynchronously. This patch speculatively compute activation for subframes of an activated load at navigation start at at every redirect. Main frames should not be impacted. Observable behavior of this class should not be impacted. This patch should significantly speed up the SubresourceFilter.DocumentLoad.ActivationComputingDelay metric, at the cost of doing some extra ruleset lookups in the browser process (an extra one for every subframe redirect). A followup will implement this behavior for main frames as well, which is a bit trickier since we'll have to fake a page activation before Safe Browsing notifies the throttle how it should activate. Bug: 809504 Change-Id: I8c98a17a94d1b0ff09eb5929930ea5e47ba66e29 Reviewed-on: https://chromium-review.googlesource.com/963069 Reviewed-by: Josh Karlin <jkarlin@chromium.org> Commit-Queue: Charlie Harrison <csharrison@chromium.org> Cr-Commit-Position: refs/heads/master@{#544544} [modify] https://crrev.com/7749896f385dc3f6cba8c8fd4ac0364c7099e55f/components/subresource_filter/content/browser/activation_state_computing_navigation_throttle.cc [modify] https://crrev.com/7749896f385dc3f6cba8c8fd4ac0364c7099e55f/components/subresource_filter/content/browser/activation_state_computing_navigation_throttle.h [modify] https://crrev.com/7749896f385dc3f6cba8c8fd4ac0364c7099e55f/components/subresource_filter/content/browser/activation_state_computing_navigation_throttle_unittest.cc [modify] https://crrev.com/7749896f385dc3f6cba8c8fd4ac0364c7099e55f/components/subresource_filter/content/browser/async_document_subresource_filter.h [modify] https://crrev.com/7749896f385dc3f6cba8c8fd4ac0364c7099e55f/components/subresource_filter/content/browser/async_document_subresource_filter_test_utils.h [modify] https://crrev.com/7749896f385dc3f6cba8c8fd4ac0364c7099e55f/components/subresource_filter/content/browser/async_document_subresource_filter_unittest.cc
,
Mar 21 2018
Recent canary shows almost no delay for subframes now: https://uma.googleplex.com/p/chrome/histograms/?endDate=20180321&dayCount=1&histograms=SubresourceFilter.DocumentLoad.ActivationComputingDelay%2CSubresourceFilter.DocumentLoad.ActivationComputingDelay.MainFrame&fixupData=true&showMax=true&analysis=0.25%200.5%200.75%200.95%200.99%200.995&filters=simple_version%2Ceq%2C67.0.3377.0%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial w00t.
,
Mar 21 2018
Excellent!
,
Mar 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/061ce628723ae02ae835e1829f284ab943c124cb commit 061ce628723ae02ae835e1829f284ab943c124cb Author: Charlie Harrison <csharrison@chromium.org> Date: Thu Mar 22 18:39:43 2018 [subresource_filter] enable main frame speculative activation computation This patch adds the ability for main frame throttles to receive potentially multiple calls to notify of page-level activation. In these cases, there is some logic at the end of the navigation to "normalize" the computed ActivationState to apply to the most recent page-level ActivationState given. This patch should have no observable effects to subresource_filter logic, with or without ad frame. Bug: 809504 Change-Id: Icb85e36c737fd092db4b7a134aabd03a1382b875 Reviewed-on: https://chromium-review.googlesource.com/961534 Reviewed-by: Josh Karlin <jkarlin@chromium.org> Commit-Queue: Charlie Harrison <csharrison@chromium.org> Cr-Commit-Position: refs/heads/master@{#545167} [modify] https://crrev.com/061ce628723ae02ae835e1829f284ab943c124cb/components/subresource_filter/content/browser/activation_state_computing_navigation_throttle.cc [modify] https://crrev.com/061ce628723ae02ae835e1829f284ab943c124cb/components/subresource_filter/content/browser/activation_state_computing_navigation_throttle.h [modify] https://crrev.com/061ce628723ae02ae835e1829f284ab943c124cb/components/subresource_filter/content/browser/activation_state_computing_navigation_throttle_unittest.cc [modify] https://crrev.com/061ce628723ae02ae835e1829f284ab943c124cb/components/subresource_filter/content/browser/async_document_subresource_filter.cc [modify] https://crrev.com/061ce628723ae02ae835e1829f284ab943c124cb/components/subresource_filter/content/browser/async_document_subresource_filter.h
,
Mar 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/36b2e1e82bab94a2ad746fbb9af6656f9d306503 commit 36b2e1e82bab94a2ad746fbb9af6656f9d306503 Author: Charlie Harrison <csharrison@chromium.org> Date: Fri Mar 23 15:44:36 2018 [subresource_filter] Properly update state in ADSF's DSF A previous patch updated the ActivationState in the AsyncDocumentSubresourceFilter, but neglected to forward this update to the internal DocumentSubresourceFilter for load state filtering. Bug: 809504 Change-Id: I53d637c35e6bcfc868d0941c1abadc3627c05749 Reviewed-on: https://chromium-review.googlesource.com/976529 Commit-Queue: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Josh Karlin <jkarlin@chromium.org> Cr-Commit-Position: refs/heads/master@{#545461} [modify] https://crrev.com/36b2e1e82bab94a2ad746fbb9af6656f9d306503/components/subresource_filter/content/browser/async_document_subresource_filter.cc [modify] https://crrev.com/36b2e1e82bab94a2ad746fbb9af6656f9d306503/components/subresource_filter/content/browser/async_document_subresource_filter.h [modify] https://crrev.com/36b2e1e82bab94a2ad746fbb9af6656f9d306503/components/subresource_filter/content/browser/async_document_subresource_filter_unittest.cc [modify] https://crrev.com/36b2e1e82bab94a2ad746fbb9af6656f9d306503/components/subresource_filter/core/common/document_subresource_filter.h
,
Mar 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5921f8484489081db0c8a9280d97fb2aba0b921a commit 5921f8484489081db0c8a9280d97fb2aba0b921a Author: Charlie Harrison <csharrison@chromium.org> Date: Fri Mar 23 18:35:01 2018 [ad-tagging] speculate DRYRUN for activation computation This CL initializes main frame activation computating throttles with a DRYRUN page activation, so they can speculatively access the ruleset on start + redirects. This should be the final CL to drive ActivationComputingDelay metrics to 0. Bug: 809504 Change-Id: If2b3e871cc97a00400e777081a8bace80f728e9f Reviewed-on: https://chromium-review.googlesource.com/967663 Commit-Queue: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Josh Karlin <jkarlin@chromium.org> Cr-Commit-Position: refs/heads/master@{#545527} [modify] https://crrev.com/5921f8484489081db0c8a9280d97fb2aba0b921a/chrome/browser/subresource_filter/subresource_filter_browsertest.cc [modify] https://crrev.com/5921f8484489081db0c8a9280d97fb2aba0b921a/components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc
,
Mar 23 2018
Fixed. Will report back with metrics changes after I have them.
,
Mar 27 2018
3381 has this and was recently released: https://uma.googleplex.com/p/chrome/histograms/?endDate=20180327&dayCount=1&histograms=SubresourceFilter.DocumentLoad.ActivationComputingDelay%2CSubresourceFilter.DocumentLoad.ActivationComputingDelay.MainFrame&fixupData=true&showMax=true&analysis=0.25%200.5%200.75%200.95%200.99%200.995&filters=simple_version%2Ceq%2C67.0.3381.0%2Cfinch%2Cstgr_filter%2C1092316494%7C1061820383%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial Looks like it's working, > 99% of navigations have 0 delay. Need to continue to monitor just in case the last 1% is "fat", probably due to ruleset verification.
,
Mar 27 2018
That's pretty darn close to 0 :) |
|||
►
Sign in to add a comment |
|||
Comment 1 by csharrison@chromium.org
, Feb 7 2018