Auto merge of #18532 - bholley:more_cache_insertion, r=emilio

Undo #18497 in the MatchAndCascade case

It's easy to construct examples where not inserting in those cases causes performance
to get worse (for example, any long list of siblings that match the same selectors
while having some non-effectual differences in LocalName/Class/Id/etc). And the LRU
nature of the cache already does the right thing of pruning non-useful entries.

Fixing this causes several hundred more sharing hits on wikipedia.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18532)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-09-16 02:09:20 -05:00 committed by GitHub
commit da64abc429
2 changed files with 23 additions and 11 deletions

View file

@ -554,11 +554,6 @@ impl<E: TElement> StyleSharingCache<E> {
validation_data_holder: Option<&mut StyleSharingTarget<E>>,
dom_depth: usize,
) {
if style.0.reused_via_rule_node {
debug!("Failing to insert into the cached: this was a cached style");
return;
}
let parent = match element.traversal_parent() {
Some(element) => element,
None => {

View file

@ -723,12 +723,29 @@ where
resolver.cascade_styles_with_default_parents(cascade_inputs)
};
context.thread_local.sharing_cache.insert_if_possible(
&element,
&new_styles.primary,
None,
traversal_data.current_dom_depth,
);
// Insert into the cache, but only if this style isn't reused from a
// sibling or cousin. Otherwise, recascading a bunch of identical
// elements would unnecessarily flood the cache with identical entries.
//
// This is analagous to the obvious "don't insert an element that just
// got a hit in the style sharing cache" behavior in the MatchAndCascade
// handling above.
//
// Note that, for the MatchAndCascade path, we still insert elements that
// shared styles via the rule node, because we know that there's something
// different about them that caused them to miss the sharing cache before
// selector matching. If we didn't, we would still end up with the same
// number of eventual styles, but would potentially miss out on various
// opportunities for skipping selector matching, which could hurt
// performance.
if !new_styles.primary.0.reused_via_rule_node {
context.thread_local.sharing_cache.insert_if_possible(
&element,
&new_styles.primary,
None,
traversal_data.current_dom_depth,
);
}
new_styles
}