mirror of
https://github.com/servo/servo.git
synced 2025-08-03 12:40:06 +01:00
style: Use only Origin during the cascade, rather than CascadeLevel.
The micro-benchmark `style-attr-1.html` regressed slightly with my patch, after the CascadeLevel size increase. This benchmark is meant to test for the "changing the style attribute doesn't cause selector-matching" optimization (which, mind you, keeps working). But in the process it creates 10k rules which form a perfect path in the rule tree and that we put into a SmallVec during the cascade, and the benchmark spends most of the time pushing to that SmallVec and iterating the declarations (as there's only one property to apply). So we could argue that the regression is minor and is not what the benchark is supposed to be testing, but given I did the digging... :) My patch made CascadeLevel bigger, which means that we create a somewhat bigger vector in this case. Thankfully it also removed the dependency in the CascadeLevel, so we can stop using that and use just Origin which is one byte to revert the perf regression. Differential Revision: https://phabricator.services.mozilla.com/D53181
This commit is contained in:
parent
45c64a7224
commit
425025c230
3 changed files with 26 additions and 23 deletions
|
@ -16,8 +16,8 @@ use crate::properties::animated_properties::AnimatedProperty;
|
||||||
use crate::properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection;
|
use crate::properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection;
|
||||||
use crate::properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState;
|
use crate::properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState;
|
||||||
use crate::properties::{self, CascadeMode, ComputedValues, LonghandId};
|
use crate::properties::{self, CascadeMode, ComputedValues, LonghandId};
|
||||||
use crate::rule_tree::CascadeLevel;
|
|
||||||
use crate::stylesheets::keyframes_rule::{KeyframesAnimation, KeyframesStep, KeyframesStepValue};
|
use crate::stylesheets::keyframes_rule::{KeyframesAnimation, KeyframesStep, KeyframesStepValue};
|
||||||
|
use crate::stylesheets::Origin;
|
||||||
use crate::timer::Timer;
|
use crate::timer::Timer;
|
||||||
use crate::values::computed::box_::TransitionProperty;
|
use crate::values::computed::box_::TransitionProperty;
|
||||||
use crate::values::computed::Time;
|
use crate::values::computed::Time;
|
||||||
|
@ -491,7 +491,7 @@ where
|
||||||
guard
|
guard
|
||||||
.normal_declaration_iter()
|
.normal_declaration_iter()
|
||||||
.filter(|declaration| declaration.is_animatable())
|
.filter(|declaration| declaration.is_animatable())
|
||||||
.map(|decl| (decl, CascadeLevel::Animations))
|
.map(|decl| (decl, Origin::Author))
|
||||||
};
|
};
|
||||||
|
|
||||||
// This currently ignores visited styles, which seems acceptable,
|
// This currently ignores visited styles, which seems acceptable,
|
||||||
|
|
|
@ -15,7 +15,7 @@ use crate::properties::{LonghandId, LonghandIdSet, CSSWideKeyword};
|
||||||
use crate::properties::{PropertyDeclaration, PropertyDeclarationId, DeclarationImportanceIterator};
|
use crate::properties::{PropertyDeclaration, PropertyDeclarationId, DeclarationImportanceIterator};
|
||||||
use crate::properties::CASCADE_PROPERTY;
|
use crate::properties::CASCADE_PROPERTY;
|
||||||
use crate::rule_cache::{RuleCache, RuleCacheConditions};
|
use crate::rule_cache::{RuleCache, RuleCacheConditions};
|
||||||
use crate::rule_tree::{CascadeLevel, StrongRuleNode};
|
use crate::rule_tree::StrongRuleNode;
|
||||||
use crate::selector_parser::PseudoElement;
|
use crate::selector_parser::PseudoElement;
|
||||||
use crate::stylesheets::{Origin, PerOrigin};
|
use crate::stylesheets::{Origin, PerOrigin};
|
||||||
use servo_arc::Arc;
|
use servo_arc::Arc;
|
||||||
|
@ -134,11 +134,15 @@ where
|
||||||
let restriction = pseudo.and_then(|p| p.property_restriction());
|
let restriction = pseudo.and_then(|p| p.property_restriction());
|
||||||
let iter_declarations = || {
|
let iter_declarations = || {
|
||||||
rule_node.self_and_ancestors().flat_map(|node| {
|
rule_node.self_and_ancestors().flat_map(|node| {
|
||||||
let cascade_level = node.cascade_level();
|
let origin = node.cascade_level().origin();
|
||||||
let node_importance = node.importance();
|
let node_importance = node.importance();
|
||||||
|
let guard = match origin {
|
||||||
|
Origin::Author => guards.author,
|
||||||
|
Origin::User | Origin::UserAgent => guards.ua_or_user,
|
||||||
|
};
|
||||||
let declarations = match node.style_source() {
|
let declarations = match node.style_source() {
|
||||||
Some(source) => source
|
Some(source) => source
|
||||||
.read(cascade_level.guard(guards))
|
.read(guard)
|
||||||
.declaration_importance_iter(),
|
.declaration_importance_iter(),
|
||||||
None => DeclarationImportanceIterator::new(&[], &empty),
|
None => DeclarationImportanceIterator::new(&[], &empty),
|
||||||
};
|
};
|
||||||
|
@ -153,14 +157,14 @@ where
|
||||||
// longhands are only allowed if they have our
|
// longhands are only allowed if they have our
|
||||||
// restriction flag set.
|
// restriction flag set.
|
||||||
if let PropertyDeclarationId::Longhand(id) = declaration.id() {
|
if let PropertyDeclarationId::Longhand(id) = declaration.id() {
|
||||||
if !id.flags().contains(restriction) && cascade_level.origin() != Origin::UserAgent {
|
if !id.flags().contains(restriction) && origin != Origin::UserAgent {
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if declaration_importance == node_importance {
|
if declaration_importance == node_importance {
|
||||||
Some((declaration, cascade_level))
|
Some((declaration, origin))
|
||||||
} else {
|
} else {
|
||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
@ -223,7 +227,7 @@ pub fn apply_declarations<'a, E, F, I>(
|
||||||
where
|
where
|
||||||
E: TElement,
|
E: TElement,
|
||||||
F: Fn() -> I,
|
F: Fn() -> I,
|
||||||
I: Iterator<Item = (&'a PropertyDeclaration, CascadeLevel)>,
|
I: Iterator<Item = (&'a PropertyDeclaration, Origin)>,
|
||||||
{
|
{
|
||||||
debug_assert!(layout_parent_style.is_none() || parent_style.is_some());
|
debug_assert!(layout_parent_style.is_none() || parent_style.is_some());
|
||||||
debug_assert_eq!(
|
debug_assert_eq!(
|
||||||
|
@ -242,17 +246,17 @@ where
|
||||||
|
|
||||||
let inherited_style = parent_style.unwrap_or(device.default_computed_values());
|
let inherited_style = parent_style.unwrap_or(device.default_computed_values());
|
||||||
|
|
||||||
let mut declarations = SmallVec::<[(&_, CascadeLevel); 32]>::new();
|
let mut declarations = SmallVec::<[(&_, Origin); 32]>::new();
|
||||||
let custom_properties = {
|
let custom_properties = {
|
||||||
let mut builder = CustomPropertiesBuilder::new(
|
let mut builder = CustomPropertiesBuilder::new(
|
||||||
inherited_style.custom_properties(),
|
inherited_style.custom_properties(),
|
||||||
device.environment(),
|
device.environment(),
|
||||||
);
|
);
|
||||||
|
|
||||||
for (declaration, cascade_level) in iter_declarations() {
|
for (declaration, origin) in iter_declarations() {
|
||||||
declarations.push((declaration, cascade_level));
|
declarations.push((declaration, origin));
|
||||||
if let PropertyDeclaration::Custom(ref declaration) = *declaration {
|
if let PropertyDeclaration::Custom(ref declaration) = *declaration {
|
||||||
builder.cascade(declaration, cascade_level.origin());
|
builder.cascade(declaration, origin);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -332,7 +336,7 @@ where
|
||||||
fn should_ignore_declaration_when_ignoring_document_colors(
|
fn should_ignore_declaration_when_ignoring_document_colors(
|
||||||
device: &Device,
|
device: &Device,
|
||||||
longhand_id: LonghandId,
|
longhand_id: LonghandId,
|
||||||
cascade_level: CascadeLevel,
|
origin: Origin,
|
||||||
pseudo: Option<&PseudoElement>,
|
pseudo: Option<&PseudoElement>,
|
||||||
declaration: &mut Cow<PropertyDeclaration>,
|
declaration: &mut Cow<PropertyDeclaration>,
|
||||||
) -> bool {
|
) -> bool {
|
||||||
|
@ -340,8 +344,7 @@ fn should_ignore_declaration_when_ignoring_document_colors(
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
let is_ua_or_user_rule =
|
let is_ua_or_user_rule = matches!(origin, Origin::User | Origin::UserAgent);
|
||||||
matches!(cascade_level.origin(), Origin::User | Origin::UserAgent);
|
|
||||||
if is_ua_or_user_rule {
|
if is_ua_or_user_rule {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
@ -446,7 +449,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
|
||||||
declarations: I,
|
declarations: I,
|
||||||
) where
|
) where
|
||||||
Phase: CascadePhase,
|
Phase: CascadePhase,
|
||||||
I: Iterator<Item = (&'decls PropertyDeclaration, CascadeLevel)>,
|
I: Iterator<Item = (&'decls PropertyDeclaration, Origin)>,
|
||||||
{
|
{
|
||||||
let apply_reset = apply_reset == ApplyResetProperties::Yes;
|
let apply_reset = apply_reset == ApplyResetProperties::Yes;
|
||||||
|
|
||||||
|
@ -459,9 +462,8 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
|
||||||
|
|
||||||
let ignore_colors = !self.context.builder.device.use_document_colors();
|
let ignore_colors = !self.context.builder.device.use_document_colors();
|
||||||
|
|
||||||
for (declaration, cascade_level) in declarations {
|
for (declaration, origin) in declarations {
|
||||||
let declaration_id = declaration.id();
|
let declaration_id = declaration.id();
|
||||||
let origin = cascade_level.origin();
|
|
||||||
let longhand_id = match declaration_id {
|
let longhand_id = match declaration_id {
|
||||||
PropertyDeclarationId::Longhand(id) => id,
|
PropertyDeclarationId::Longhand(id) => id,
|
||||||
PropertyDeclarationId::Custom(..) => continue,
|
PropertyDeclarationId::Custom(..) => continue,
|
||||||
|
@ -509,7 +511,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
|
||||||
let should_ignore = should_ignore_declaration_when_ignoring_document_colors(
|
let should_ignore = should_ignore_declaration_when_ignoring_document_colors(
|
||||||
self.context.builder.device,
|
self.context.builder.device,
|
||||||
longhand_id,
|
longhand_id,
|
||||||
cascade_level,
|
origin,
|
||||||
self.context.builder.pseudo,
|
self.context.builder.pseudo,
|
||||||
&mut declaration,
|
&mut declaration,
|
||||||
);
|
);
|
||||||
|
|
|
@ -1322,16 +1322,17 @@ impl Stylist {
|
||||||
let iter_declarations = || {
|
let iter_declarations = || {
|
||||||
block
|
block
|
||||||
.declaration_importance_iter()
|
.declaration_importance_iter()
|
||||||
.map(|(declaration, importance)| {
|
.map(|(declaration, _)| (declaration, Origin::Author))
|
||||||
debug_assert!(!importance.important());
|
|
||||||
(declaration, CascadeLevel::same_tree_author_normal())
|
|
||||||
})
|
|
||||||
};
|
};
|
||||||
|
|
||||||
let metrics = get_metrics_provider_for_product();
|
let metrics = get_metrics_provider_for_product();
|
||||||
|
|
||||||
// We don't bother inserting these declarations in the rule tree, since
|
// We don't bother inserting these declarations in the rule tree, since
|
||||||
// it'd be quite useless and slow.
|
// it'd be quite useless and slow.
|
||||||
|
//
|
||||||
|
// TODO(emilio): Now that we fixed bug 1493420, we should consider
|
||||||
|
// reversing this as it shouldn't be slow anymore, and should avoid
|
||||||
|
// generating two instantiations of apply_declarations.
|
||||||
properties::apply_declarations::<E, _, _>(
|
properties::apply_declarations::<E, _, _>(
|
||||||
&self.device,
|
&self.device,
|
||||||
/* pseudo = */ None,
|
/* pseudo = */ None,
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue