Skip to content

Commit

Permalink
Aho-Corasick attribute selector pruning.
Browse files Browse the repository at this point in the history
If a stylesheet has a lot of different rules bucketed on the same
attribute (some extensions inject user stylesheets that do this,
for one), we can do better than testing each and every one of them
individually. As an optimization, we can use SubstringSetMatcher
(an implementation of the Aho-Corasick string matching algorithm)
to rapidly test against all of them in linear time in the length of
the attribute value. This takes a bit of RAM for such stylesheets
(e.g. a 743-element tree from the Extension subtest takes ~153 kB),
but speeds up their matching significantly.

Style microbenchmarks (Zen 2, LTO but no PGO; only Extension,
News, Sports and Video actually get behavioral differences):

Parse (µs)             Before     After    Perf      95% CI (BCa)
=================== ========= ========= ======= =================
ECommerce                1693      1717   -1.4%  [ -1.5%,  -1.2%]
Encyclopedia             9170      9179   -0.1%  [ -0.2%,  -0.0%]
Extension                1637      1662   -1.5%  [ -1.6%,  -1.3%]
News                    10267     10288   -0.2%  [ -0.3%,  -0.1%]
Search                   6236      6260   -0.4%  [ -0.4%,  -0.3%]
Social1                 21045     21046   -0.0%  [ -0.1%,  +0.1%]
Social2                   722       737   -2.0%  [ -2.2%,  -1.9%]
Sports                  73974     74119   -0.2%  [ -0.3%,  -0.1%]
Video                   38936     39101   -0.4%  [ -0.5%,  -0.4%]
Geometric mean                            -0.7%  [ -0.8%,  -0.6%]

Initial style (µs)     Before     After    Perf      95% CI (BCa)
=================== ========= ========= ======= =================
ECommerce               11477     11419   +0.5%  [ +0.4%,  +0.6%]
Encyclopedia           110338    110068   +0.2%  [ +0.2%,  +0.3%]
Extension              261834    137113  +91.0%  [+90.8%, +91.1%]
News                    43919     43823   +0.2%  [ +0.1%,  +0.3%]
Search                   2775      2800   -0.9%  [ -1.0%,  -0.8%]
Social1                 26140     26114   +0.1%  [ +0.0%,  +0.2%]
Social2                  1087      1061   +2.4%  [ +2.2%,  +2.6%]
Sports                  37874     36289   +4.4%  [ +4.3%,  +4.4%]
Video                   41291     41292   -0.0%  [ -0.1%,  +0.1%]
Geometric mean                            +8.3%  [ +8.2%,  +8.3%]

Recalc style (µs)      Before     After    Perf      95% CI (BCa)
=================== ========= ========= ======= =================
ECommerce               14568     14544   +0.2%  [ +0.0%,  +0.3%]
Encyclopedia            98240     97627   +0.6%  [ +0.5%,  +0.8%]
Extension              257682    130824  +97.0%  [+96.8%, +97.2%]
News                    33016     33000   +0.0%  [ -0.1%,  +0.2%]
Search                    251       249   +0.6%  [ +0.4%,  +0.8%]
Social1                 17993     17884   +0.6%  [ +0.5%,  +0.7%]
Social2                   602       580   +3.8%  [ +3.4%,  +4.0%]
Sports                  29106     27465   +6.0%  [ +5.9%,  +6.1%]
Video                   29407     29343   +0.2%  [ +0.1%,  +0.5%]
Geometric mean                            +9.2%  [ +9.1%,  +9.3%]

Change-Id: I5fe9d74fae11c30e7dc5c2cd62f2944e2cba50eb
Bug: 1318773
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3581992
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Steinar H Gunderson <sesse@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002855}
  • Loading branch information
Steinar H. Gunderson authored and Chromium LUCI CQ committed May 12, 2022
1 parent d36e1f5 commit 0a15e2d
Show file tree
Hide file tree
Showing 9 changed files with 282 additions and 19 deletions.
20 changes: 20 additions & 0 deletions testing/variations/fieldtrial_testing_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -1550,6 +1550,26 @@
]
}
],
"BlinkAhoCorasickForAttributeSelectors": [
{
"platforms": [
"android",
"chromeos",
"chromeos_lacros",
"linux",
"mac",
"windows"
],
"experiments": [
{
"name": "Enabled",
"enable_features": [
"SubstringSetTreeForAttributeBuckets"
]
}
]
}
],
"BlinkCompositorUseDisplayThreadPriority": [
{
"platforms": [
Expand Down
3 changes: 3 additions & 0 deletions third_party/blink/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1462,5 +1462,8 @@ const base::Feature kDispatchPopstateSync{"DispatchPopstateSync",
const base::Feature kWebRtcExposeNonStandardStats{
"WebRtc-ExposeNonStandardStats", base::FEATURE_DISABLED_BY_DEFAULT};

const base::Feature kSubstringSetTreeForAttributeBuckets{
"SubstringSetTreeForAttributeBuckets", base::FEATURE_DISABLED_BY_DEFAULT};

} // namespace features
} // namespace blink
5 changes: 5 additions & 0 deletions third_party/blink/public/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,11 @@ BLINK_COMMON_EXPORT extern const base::Feature kDispatchPopstateSync;
// If enabled, expose non-standard stats in the WebRTC getStats API.
BLINK_COMMON_EXPORT extern const base::Feature kWebRtcExposeNonStandardStats;

// If enabled, CSS rulesets with many different rules on the same attribute
// will be attempted accelerated with a substring set tree.
BLINK_COMMON_EXPORT extern const base::Feature
kSubstringSetTreeForAttributeBuckets;

} // namespace features
} // namespace blink

Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/core/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ include_rules = [
"+base/memory/scoped_refptr.h",
"+base/profiler/sample_metadata.h",
"+base/strings/stringprintf.h",
"+base/substring_set_matcher/substring_set_matcher.h",
"+base/task/sequence_manager/task_time_observer.h",
"+base/test/bind.h",
"+base/test/simple_test_tick_clock.h",
Expand Down
14 changes: 10 additions & 4 deletions third_party/blink/renderer/core/css/element_rule_collector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "third_party/blink/renderer/core/css/element_rule_collector.h"

#include "base/containers/span.h"
#include "base/substring_set_matcher/substring_set_matcher.h"
#include "base/trace_event/common/trace_event_common.h"
#include "third_party/blink/renderer/core/css/cascade_layer_map.h"
#include "third_party/blink/renderer/core/css/check_pseudo_has_cache_scope.h"
Expand Down Expand Up @@ -597,10 +598,15 @@ void ElementRuleCollector::CollectMatchingRules(
: attribute_name;
for (const auto bundle : match_request.AllRuleSets()) {
if (bundle.rule_set->HasAnyAttrRules()) {
CollectMatchingRulesForList(bundle.rule_set->AttrRules(lower_name),
match_request, bundle.rule_set,
bundle.style_sheet,
bundle.style_sheet_index, checker);
const HeapVector<Member<const RuleData>>* list =
bundle.rule_set->AttrRules(lower_name);
if (list && !bundle.rule_set->CanIgnoreEntireList(
list, lower_name, attributes[attr_idx].Value())) {
CollectMatchingRulesForList(bundle.rule_set->AttrRules(lower_name),
match_request, bundle.rule_set,
bundle.style_sheet,
bundle.style_sheet_index, checker);
}
}
}

Expand Down
161 changes: 146 additions & 15 deletions third_party/blink/renderer/core/css/rule_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@

#include "third_party/blink/renderer/core/css/rule_set.h"

#include <memory>
#include <type_traits>
#include <vector>

#include "base/substring_set_matcher/substring_set_matcher.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/renderer/core/css/css_font_selector.h"
#include "third_party/blink/renderer/core/css/css_selector.h"
#include "third_party/blink/renderer/core/css/css_selector_list.h"
Expand All @@ -44,6 +48,9 @@
#include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin.h"

using base::StringPattern;
using base::SubstringSetMatcher;

namespace blink {

static inline ValidPropertyFilter DetermineValidPropertyFilter(
Expand Down Expand Up @@ -154,6 +161,7 @@ static void ExtractSelectorValues(const CSSSelector* selector,
AtomicString& id,
AtomicString& class_name,
AtomicString& attr_name,
AtomicString& attr_value,
AtomicString& custom_pseudo_element_name,
AtomicString& tag_name,
AtomicString& part_name,
Expand Down Expand Up @@ -206,9 +214,9 @@ static void ExtractSelectorValues(const CSSSelector* selector,
// like a normal selector (save for specificity), and we can put it
// into a bucket based on that selector.
if (selector_list->HasOneSelector()) {
ExtractSelectorValues(selector_list->First(), id, class_name,
attr_name, custom_pseudo_element_name,
tag_name, part_name, pseudo_type);
ExtractSelectorValues(
selector_list->First(), id, class_name, attr_name, attr_value,
custom_pseudo_element_name, tag_name, part_name, pseudo_type);
}
} break;
default:
Expand All @@ -223,17 +231,48 @@ static void ExtractSelectorValues(const CSSSelector* selector,
case CSSSelector::kAttributeBegin:
case CSSSelector::kAttributeEnd:
attr_name = selector->Attribute().LocalName();
attr_value = selector->Value();
break;
default:
break;
}
}

// For a (possibly compound) selector, extract the values used for determining
// its buckets (e.g. for “.foo[baz]”, will return foo for class_name and
// baz for attr_name). Returns the last subselector in the group, which is also
// the one given the highest priority.
static const CSSSelector* ExtractBestSelectorValues(
const CSSSelector& component,
AtomicString& id,
AtomicString& class_name,
AtomicString& attr_name,
AtomicString& attr_value,
AtomicString& custom_pseudo_element_name,
AtomicString& tag_name,
AtomicString& part_name,
CSSSelector::PseudoType& pseudo_type) {
const CSSSelector* it = &component;
for (; it && it->Relation() == CSSSelector::kSubSelector;
it = it->TagHistory()) {
ExtractSelectorValues(it, id, class_name, attr_name, attr_value,
custom_pseudo_element_name, tag_name, part_name,
pseudo_type);
}
if (it) {
ExtractSelectorValues(it, id, class_name, attr_name, attr_value,
custom_pseudo_element_name, tag_name, part_name,
pseudo_type);
}
return it;
}

bool RuleSet::FindBestRuleSetAndAdd(const CSSSelector& component,
RuleData* rule_data) {
AtomicString id;
AtomicString class_name;
AtomicString attr_name;
AtomicString attr_value; // Unused.
AtomicString custom_pseudo_element_name;
AtomicString tag_name;
AtomicString part_name;
Expand All @@ -243,18 +282,9 @@ bool RuleSet::FindBestRuleSetAndAdd(const CSSSelector& component,
all_rules_.push_back(rule_data);
#endif

const CSSSelector* it = &component;
for (; it && it->Relation() == CSSSelector::kSubSelector;
it = it->TagHistory()) {
ExtractSelectorValues(it, id, class_name, attr_name,
custom_pseudo_element_name, tag_name, part_name,
pseudo_type);
}
if (it) {
ExtractSelectorValues(it, id, class_name, attr_name,
custom_pseudo_element_name, tag_name, part_name,
pseudo_type);
}
const CSSSelector* it = ExtractBestSelectorValues(
component, id, class_name, attr_name, attr_value,
custom_pseudo_element_name, tag_name, part_name, pseudo_type);

// Prefer rule sets in order of most likely to apply infrequently.
if (!id.IsEmpty()) {
Expand Down Expand Up @@ -613,12 +643,113 @@ void RuleSet::CompactPendingRules(PendingRuleMap& pending_map,
}
}

static wtf_size_t GetMinimumRulesetSizeForSubstringMatcher() {
// It's not worth going through the Aho-Corasick matcher unless we can
// reject a reasonable number of rules in one go. Practical ad-hoc testing
// suggests the break-even point between using the tree and just testing
// all of the rules individually lies somewhere around 20–40 rules
// (depending a bit on e.g. how hot the tree is in the cache, the length
// of the value that we match against, and of course whether we actually
// have a match). We add a little bit of margin to compensate for the fact
// that we also need to spend time building the tree, and the extra memory
// in use.
//
// TODO(sesse): When the Finch experiment finishes, lock this to 50.
return base::FeatureList::IsEnabled(
blink::features::kSubstringSetTreeForAttributeBuckets)
? 50
: std::numeric_limits<wtf_size_t>::max();
}

bool RuleSet::CanIgnoreEntireList(
const HeapVector<Member<const RuleData>>* list,
const AtomicString& key,
const AtomicString& value) const {
DCHECK(!pending_rules_);
DCHECK_EQ(attr_rules_.find(key)->value, list);
if (list->size() < GetMinimumRulesetSizeForSubstringMatcher()) {
// Too small to build up a tree, so always check.
DCHECK_EQ(attr_substring_matchers_.find(key),
attr_substring_matchers_.end());
return false;
}

// See CreateSubstringMatchers().
if (value.IsEmpty()) {
return false;
}

auto it = attr_substring_matchers_.find(key);
if (it == attr_substring_matchers_.end()) {
// Building the tree failed, so always check.
return false;
}
return !it->value->AnyMatch(value.LowerASCII().Utf8());
}

void RuleSet::CreateSubstringMatchers(
CompactRuleMap& attr_map,
RuleSet::SubstringMatcherMap& substring_matcher_map) {
for (const auto& [/*AtomicString*/ attr, /*Member<RuleSet>*/ ruleset] :
attr_map) {
if (ruleset->size() < GetMinimumRulesetSizeForSubstringMatcher()) {
continue;
}
std::vector<StringPattern> patterns;
int rule_index = 0;
for (const Member<const RuleData>& rule : *ruleset) {
AtomicString id;
AtomicString class_name;
AtomicString attr_name;
AtomicString attr_value;
AtomicString custom_pseudo_element_name;
AtomicString tag_name;
AtomicString part_name;
CSSSelector::PseudoType pseudo_type = CSSSelector::kPseudoUnknown;
ExtractBestSelectorValues(rule->Selector(), id, class_name, attr_name,
attr_value, custom_pseudo_element_name,
tag_name, part_name, pseudo_type);
DCHECK(!attr_name.IsEmpty());

if (attr_value.IsEmpty()) {
// The empty string would make the entire tree useless (it is
// a substring of every possible value), so as a special case,
// we ignore it, and have a separate check in CanIgnoreEntireList().
continue;
}

std::string pattern = attr_value.LowerASCII().Utf8();

// SubstringSetMatcher doesn't like duplicates, and since we only
// use the tree for true/false information anyway, we can remove them.
bool already_exists =
any_of(patterns.begin(), patterns.end(),
[&pattern](const StringPattern& existing_pattern) {
return existing_pattern.pattern() == pattern;
});
if (!already_exists) {
patterns.emplace_back(pattern, rule_index);
}
++rule_index;
}

auto substring_matcher = std::make_unique<SubstringSetMatcher>();
if (!substring_matcher->Build(patterns)) {
// Should never really happen unless there are megabytes and megabytes
// of such classes, so we just drop out to the slow path.
} else {
substring_matcher_map.insert(attr, std::move(substring_matcher));
}
}
}

void RuleSet::CompactRules() {
DCHECK(pending_rules_);
PendingRuleMaps* pending_rules = pending_rules_.Release();
CompactPendingRules(pending_rules->id_rules, id_rules_);
CompactPendingRules(pending_rules->class_rules, class_rules_);
CompactPendingRules(pending_rules->attr_rules, attr_rules_);
CreateSubstringMatchers(attr_rules_, attr_substring_matchers_);
CompactPendingRules(pending_rules->tag_rules, tag_rules_);
CompactPendingRules(pending_rules->ua_shadow_pseudo_element_rules,
ua_shadow_pseudo_element_rules_);
Expand Down
31 changes: 31 additions & 0 deletions third_party/blink/renderer/core/css/rule_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_RULE_SET_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_RULE_SET_H_

#include "base/substring_set_matcher/substring_set_matcher.h"
#include "base/types/pass_key.h"
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/css/cascade_layer.h"
Expand Down Expand Up @@ -260,6 +261,9 @@ class CORE_EXPORT RuleSet final : public GarbageCollected<RuleSet> {
auto it = attr_rules_.find(key);
return it != attr_rules_.end() ? it->value : nullptr;
}
bool CanIgnoreEntireList(const HeapVector<Member<const RuleData>>* list,
const AtomicString& key,
const AtomicString& value) const;
const HeapVector<Member<const RuleData>>* TagRules(
const AtomicString& key) const {
DCHECK(!pending_rules_);
Expand Down Expand Up @@ -404,6 +408,8 @@ class CORE_EXPORT RuleSet final : public GarbageCollected<RuleSet> {
Member<HeapLinkedStack<Member<const RuleData>>>>;
using CompactRuleMap =
HeapHashMap<AtomicString, Member<HeapVector<Member<const RuleData>>>>;
using SubstringMatcherMap =
HashMap<AtomicString, std::unique_ptr<base::SubstringSetMatcher>>;

void AddToRuleSet(const AtomicString& key, PendingRuleMap&, const RuleData*);
void AddPageRule(StyleRulePage*);
Expand Down Expand Up @@ -435,6 +441,9 @@ class CORE_EXPORT RuleSet final : public GarbageCollected<RuleSet> {

void CompactRules();
static void CompactPendingRules(PendingRuleMap&, CompactRuleMap&);
static void CreateSubstringMatchers(
CompactRuleMap& attr_map,
SubstringMatcherMap& substring_matcher_map);

class PendingRuleMaps : public GarbageCollected<PendingRuleMaps> {
public:
Expand Down Expand Up @@ -475,6 +484,28 @@ class CORE_EXPORT RuleSet final : public GarbageCollected<RuleSet> {
CompactRuleMap id_rules_;
CompactRuleMap class_rules_;
CompactRuleMap attr_rules_;
// A structure for quickly rejecting an entire attribute rule set
// (from attr_rules_). If we have many rules in the same bucket,
// we build up a case-insensitive substring-matching structure of all
// the values we can match on (all attribute selectors are either substring,
// or something stricter than substring). We can then use that structure
// to see in linear time (of the length of the attribute value in the DOM)
// whether we can have any matches at all.
//
// If we find any matches, we need to recheck each rule, because the rule in
// question may actually be case-sensitive, or we might want e.g. a prefix
// match instead of a substring match. (We could solve prefix/suffix by
// means of inserting special start-of-string and end-of-string tokens,
// but we keep it simple for now.) Also, the way we use the
// SubstringSetMatcher, we don't actually get back which rules matched.
//
// This element does not exist, if there are few enough rules that we don't
// deem this step worth it, or if the build of the tree failed. (In
// particular, if there is only a single rule in this bucket, it's
// pointless to run the entire Aho-Corasick algorithm instead of just
// doing a simple match.) Check GetMinimumRulesetSizeForSubstringMatcher()
// before looking up for a cheaper test.
SubstringMatcherMap attr_substring_matchers_;
CompactRuleMap tag_rules_;
CompactRuleMap ua_shadow_pseudo_element_rules_;
HeapVector<Member<const RuleData>> link_pseudo_class_rules_;
Expand Down

0 comments on commit 0a15e2d

Please sign in to comment.