New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 672519 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocking:
issue 609747



Sign in to add a comment

Optional performance measuring in SubresourceFilter

Project Member Reported by pkalinnikov@chromium.org, Dec 8 2016

Issue description

The overhead of measuring SubresourceFiltering performance boils down to the cost of TimeTicks and ThreadTicks Now() method calls, which is significant compared to the cost of subresource load evaluation cost itself.

The idea is to activate performance measurement selectively, according to some policy, e.g., at random for 10% of subresource loads. Ideally the decision to activate should be taken on page-wise basis, i.e. all SubresourceFilter's corresponding to a page should be measuring performance at the same time. This will be especially useful for page-level aggregations to be informative.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/88e76494111e68722f089ad41da81fe258ae09ab

commit 88e76494111e68722f089ad41da81fe258ae09ab
Author: pkalinnikov <pkalinnikov@chromium.org>
Date: Mon Dec 12 09:41:45 2016

Support optional performance measuring in SubresourceFilter.

The purpose of this CL is to provide a mechanism for enabling or
disabling SubresourceFilter's performance measuring on a per-page basis,
i.e. consistently for all DocumentSubresourceFilter instances of a page.

A decision on whether to measure performance (currently always true)
originates in ContentSubresourceFilterDriverFactory. It is distributed
to SubresourceFilterAgents first in the ActivateForProvisionalLoad
message, and eventually reaches individual DocumentSubresourceFilter
instances corresponding to each subframe document.

BUG= 672519 

Review-Url: https://codereview.chromium.org/2556433003
Cr-Commit-Position: refs/heads/master@{#437837}

[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/components/subresource_filter/content/browser/content_subresource_filter_driver.cc
[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/components/subresource_filter/content/browser/content_subresource_filter_driver.h
[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc
[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/components/subresource_filter/content/common/subresource_filter_messages.h
[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/components/subresource_filter/content/renderer/document_subresource_filter.cc
[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/components/subresource_filter/content/renderer/document_subresource_filter.h
[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/components/subresource_filter/content/renderer/document_subresource_filter_unittest.cc
[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/components/subresource_filter/content/renderer/subresource_filter_agent.cc
[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/components/subresource_filter/content/renderer/subresource_filter_agent.h
[modify] https://crrev.com/88e76494111e68722f089ad41da81fe258ae09ab/components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b758a270a56b34f25f73bee8130a5f6ec816d6e

commit 3b758a270a56b34f25f73bee8130a5f6ec816d6e
Author: pkalinnikov <pkalinnikov@chromium.org>
Date: Wed Dec 14 18:35:41 2016

Introduce performance measurement varitation parameter for SubresourceFilter.

The parameter defines a fraction of page loads that should have extended
performance measurements enabled. ContentSubresourceFilterDriverFactory uses it
for each main-frame navigation as a probability for enabling
|measure_performance|, which then gets distributed to all
DocumentSubresourceFilter's corresponding to (sub-)frames of the page.

If the variation parameter is not present or malformed, the extended performance
measurement is switched off.

BUG= 672519 

Review-Url: https://codereview.chromium.org/2569693002
Cr-Commit-Position: refs/heads/master@{#438543}

[modify] https://crrev.com/3b758a270a56b34f25f73bee8130a5f6ec816d6e/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
[modify] https://crrev.com/3b758a270a56b34f25f73bee8130a5f6ec816d6e/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h
[modify] https://crrev.com/3b758a270a56b34f25f73bee8130a5f6ec816d6e/components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc
[modify] https://crrev.com/3b758a270a56b34f25f73bee8130a5f6ec816d6e/components/subresource_filter/core/browser/subresource_filter_features.cc
[modify] https://crrev.com/3b758a270a56b34f25f73bee8130a5f6ec816d6e/components/subresource_filter/core/browser/subresource_filter_features.h
[modify] https://crrev.com/3b758a270a56b34f25f73bee8130a5f6ec816d6e/components/subresource_filter/core/browser/subresource_filter_features_test_support.cc
[modify] https://crrev.com/3b758a270a56b34f25f73bee8130a5f6ec816d6e/components/subresource_filter/core/browser/subresource_filter_features_test_support.h
[modify] https://crrev.com/3b758a270a56b34f25f73bee8130a5f6ec816d6e/components/subresource_filter/core/browser/subresource_filter_features_unittest.cc

Status: Fixed (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22be4914542ba9072ee2e6c7c3480881ebcaa6fe

commit 22be4914542ba9072ee2e6c7c3480881ebcaa6fe
Author: pkalinnikov <pkalinnikov@chromium.org>
Date: Wed Dec 28 14:23:09 2016

Add page-level aggregation of SubresourceFilter time metrics.

Add a new message (SubresourceFilterHostMsg_DocumentLoadStatistics) that is sent
to a RenderFrameHost in the browser when a document load is finished, just
before the DidFinishLoad message, if performance measurements were enabled for
the load. Contains the total time spent on evaluating subresource loads in
DocumentSubresourceFilter::allowLoad() for a frame.

Measurements are aggregated in SubresourceFilterDriverFactory and recorded to
"SubresourceFilter.PageLoad.SubresourceEvaluation.*Duration" histograms when
DidFinishLoad is called for the main frame.

BUG=609747, 672519 

Review-Url: https://codereview.chromium.org/2581043003
Cr-Commit-Position: refs/heads/master@{#440849}

[modify] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
[modify] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/chrome/test/data/subresource_filter/frame_set.html
[add] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/chrome/test/data/subresource_filter/frame_with_allowed_script.html
[add] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/chrome/test/data/subresource_filter/frame_with_no_subresources.html
[add] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/chrome/test/data/subresource_filter/included_allowed_script.js
[modify] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
[modify] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h
[modify] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/components/subresource_filter/content/common/subresource_filter_messages.h
[modify] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/components/subresource_filter/content/renderer/subresource_filter_agent.cc
[modify] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/components/subresource_filter/content/renderer/subresource_filter_agent.h
[modify] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc
[modify] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/components/subresource_filter/core/browser/subresource_filter_features_test_support.cc
[modify] https://crrev.com/22be4914542ba9072ee2e6c7c3480881ebcaa6fe/tools/metrics/histograms/histograms.xml

Sign in to add a comment