Commit a4b63eb6 authored by Camillo Bruni's avatar Camillo Bruni Committed by Chromium LUCI CQ

[blink] Speed up AdTracker::isKnownAdScript

Delay calling into V8 for detected the topmost script. Crossing the
API and getting a stack trace can be avoided in many case. This has a
measurable impact on Speedometer.

Bug: 808503
Change-Id: I6ed1b8734056ecb2efd55cf83f1d7a3302690f52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2571091Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834629}
parent c524eb9d
...@@ -213,7 +213,9 @@ bool AdTracker::CalculateIfAdSubresource( ...@@ -213,7 +213,9 @@ bool AdTracker::CalculateIfAdSubresource(
const FetchInitiatorInfo& initiator_info, const FetchInitiatorInfo& initiator_info,
bool known_ad) { bool known_ad) {
// Check if the document loading the resource is an ad. // Check if the document loading the resource is an ad.
known_ad = known_ad || IsKnownAdExecutionContext(execution_context); const bool is_ad_execution_context =
IsKnownAdExecutionContext(execution_context);
known_ad = known_ad || is_ad_execution_context;
// We skip script checking for stylesheet-initiated resource requests as the // We skip script checking for stylesheet-initiated resource requests as the
// stack may represent the cause of a style recalculation rather than the // stack may represent the cause of a style recalculation rather than the
...@@ -232,7 +234,7 @@ bool AdTracker::CalculateIfAdSubresource( ...@@ -232,7 +234,7 @@ bool AdTracker::CalculateIfAdSubresource(
// contexts, because any script executed inside an ad context is considered an // contexts, because any script executed inside an ad context is considered an
// ad script by IsKnownAdScript. // ad script by IsKnownAdScript.
if (resource_type == ResourceType::kScript && known_ad && if (resource_type == ResourceType::kScript && known_ad &&
!IsKnownAdExecutionContext(execution_context)) { !is_ad_execution_context) {
AppendToKnownAdScripts(*execution_context, request.Url().GetString()); AppendToKnownAdScripts(*execution_context, request.Url().GetString());
} }
...@@ -286,7 +288,7 @@ bool AdTracker::IsAdScriptInStack(StackType stack_type) { ...@@ -286,7 +288,7 @@ bool AdTracker::IsAdScriptInStack(StackType stack_type) {
// (e.g., when v8 is executed) but not the entire stack. For a small cost we // (e.g., when v8 is executed) but not the entire stack. For a small cost we
// can also check the top of the stack (this is much cheaper than getting the // can also check the top of the stack (this is much cheaper than getting the
// full stack from v8). // full stack from v8).
return IsKnownAdScript(execution_context, ScriptAtTopOfStack()); return IsKnownAdScriptForCheckedContext(*execution_context, String());
} }
bool AdTracker::IsKnownAdScript(ExecutionContext* execution_context, bool AdTracker::IsKnownAdScript(ExecutionContext* execution_context,
...@@ -297,13 +299,25 @@ bool AdTracker::IsKnownAdScript(ExecutionContext* execution_context, ...@@ -297,13 +299,25 @@ bool AdTracker::IsKnownAdScript(ExecutionContext* execution_context,
if (IsKnownAdExecutionContext(execution_context)) if (IsKnownAdExecutionContext(execution_context))
return true; return true;
if (url.IsEmpty()) return IsKnownAdScriptForCheckedContext(*execution_context, url);
return false; }
auto it = known_ad_scripts_.find(execution_context); bool AdTracker::IsKnownAdScriptForCheckedContext(
ExecutionContext& execution_context,
const String& url) {
DCHECK(!IsKnownAdExecutionContext(&execution_context));
auto it = known_ad_scripts_.find(&execution_context);
if (it == known_ad_scripts_.end()) if (it == known_ad_scripts_.end())
return false; return false;
return it->value.Contains(url);
if (it->value.IsEmpty())
return false;
// Delay calling ScriptAtTopOfStack() as much as possible due to its cost.
String script_url = url.IsNull() ? ScriptAtTopOfStack() : url;
if (script_url.IsEmpty())
return false;
return it->value.Contains(script_url);
} }
// This is a separate function for testing purposes. // This is a separate function for testing purposes.
......
...@@ -110,7 +110,8 @@ class CORE_EXPORT AdTracker : public GarbageCollected<AdTracker> { ...@@ -110,7 +110,8 @@ class CORE_EXPORT AdTracker : public GarbageCollected<AdTracker> {
const String& script_name, const String& script_name,
int script_id); int script_id);
void DidExecuteScript(); void DidExecuteScript();
bool IsKnownAdScript(ExecutionContext* execution_context, const String& url); bool IsKnownAdScript(ExecutionContext*, const String& url);
bool IsKnownAdScriptForCheckedContext(ExecutionContext&, const String& url);
void AppendToKnownAdScripts(ExecutionContext&, const String& url); void AppendToKnownAdScripts(ExecutionContext&, const String& url);
Member<LocalFrame> local_root_; Member<LocalFrame> local_root_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment