Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-07-31 19:20:22 +01:00
Code Issues Projects Releases Packages Wiki Activity

Reduce allocator churn when parsing property declaration blocks (fixes #15060)

Browse source
This commit is contained in:
Manish Goregaokar 2017-01-17 18:56:42 -08:00
parent f010fb58fd
commit b5cb401aef
6 changed files with 81 additions and 49 deletions
Download patch file Download diff file Expand all files Collapse all files

9
components/script/dom/cssstyledeclaration.rs
View file

@ -193,7 +193,7 @@ impl CSSStyleDeclaration {
ParserContextExtraData::default()); ParserContextExtraData::default());
// Step 7 // Step 7
let declarations = match declarations { let mut declarations = match declarations {
Ok(declarations) => declarations, Ok(declarations) => declarations,
Err(_) => return Ok(()) Err(_) => return Ok(())
}; };
@ -204,7 +204,7 @@ impl CSSStyleDeclaration {
Some(ref lock) => { Some(ref lock) => {
let mut style_attribute = lock.write(); let mut style_attribute = lock.write();
for declaration in declarations { for declaration in declarations {
style_attribute.set_parsed_declaration(declaration, importance); style_attribute.set_parsed_declaration(declaration.0, importance);
} }
self.owner.flush_style(&style_attribute); self.owner.flush_style(&style_attribute);
} }
@ -214,8 +214,11 @@ impl CSSStyleDeclaration {
} else { } else {
0 0
}; };
for decl in &mut declarations {
decl.1 = importance
}
let block = PropertyDeclarationBlock { let block = PropertyDeclarationBlock {
declarations: declarations.into_iter().map(|d| (d, importance)).collect(), declarations: declarations,
important_count: important_count, important_count: important_count,
}; };
if let CSSStyleOwner::Element(ref el) = self.owner { if let CSSStyleOwner::Element(ref el) = self.owner {

31
components/style/keyframes.rs
View file

@ -325,14 +325,14 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> {
fn parse_block(&mut self, prelude: Self::Prelude, input: &mut Parser) fn parse_block(&mut self, prelude: Self::Prelude, input: &mut Parser)
-> Result<Self::QualifiedRule, ()> { -> Result<Self::QualifiedRule, ()> {
let mut declarations = Vec::new();
let parser = KeyframeDeclarationParser { let parser = KeyframeDeclarationParser {
context: self.context, context: self.context,
declarations: vec![],
}; };
let mut iter = DeclarationListParser::new(input, parser); let mut iter = DeclarationListParser::new(input, parser);
while let Some(declaration) = iter.next() { while let Some(declaration) = iter.next() {
match declaration { match declaration {
Ok(d) => declarations.extend(d.into_iter().map(|d| (d, Importance::Normal))), Ok(_) => (),
Err(range) => { Err(range) => {
let pos = range.start; let pos = range.start;
let message = format!("Unsupported keyframe property declaration: '{}'", let message = format!("Unsupported keyframe property declaration: '{}'",
@ -345,7 +345,7 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> {
Ok(Arc::new(RwLock::new(Keyframe { Ok(Arc::new(RwLock::new(Keyframe {
selector: prelude, selector: prelude,
block: Arc::new(RwLock::new(PropertyDeclarationBlock { block: Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: declarations, declarations: iter.parser.declarations,
important_count: 0, important_count: 0,
})), })),
}))) })))
@ -354,24 +354,35 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> {
struct KeyframeDeclarationParser<'a, 'b: 'a> { struct KeyframeDeclarationParser<'a, 'b: 'a> {
context: &'a ParserContext<'b>, context: &'a ParserContext<'b>,
declarations: Vec<(PropertyDeclaration, Importance)>
} }
/// Default methods reject all at rules. /// Default methods reject all at rules.
impl<'a, 'b> AtRuleParser for KeyframeDeclarationParser<'a, 'b> { impl<'a, 'b> AtRuleParser for KeyframeDeclarationParser<'a, 'b> {
type Prelude = (); type Prelude = ();
type AtRule = Vec<PropertyDeclaration>; type AtRule = ();
} }
impl<'a, 'b> DeclarationParser for KeyframeDeclarationParser<'a, 'b> { impl<'a, 'b> DeclarationParser for KeyframeDeclarationParser<'a, 'b> {
type Declaration = Vec<PropertyDeclaration>; /// We parse rules directly into the declarations object
type Declaration = ();
fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<Vec<PropertyDeclaration>, ()> { fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<(), ()> {
let id = try!(PropertyId::parse(name.into())); let id = try!(PropertyId::parse(name.into()));
let mut results = Vec::new(); let old_len = self.declarations.len();
match PropertyDeclaration::parse(id, self.context, input, &mut results, true) { match PropertyDeclaration::parse(id, self.context, input, &mut self.declarations, true) {
PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => {} PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => {}
_ => return Err(()) _ => {
self.declarations.truncate(old_len);
return Err(());
}
}
// In case there is still unparsed text in the declaration, we should roll back.
if !input.is_exhausted() {
self.declarations.truncate(old_len);
Err(())
} else {
Ok(())
} }
Ok(results)
} }
} }

52
components/style/properties/declaration_block.rs
View file

@ -495,12 +495,15 @@ pub fn parse_style_attribute(input: &str,
/// Parse a given property declaration. Can result in multiple /// Parse a given property declaration. Can result in multiple
/// `PropertyDeclaration`s when expanding a shorthand, for example. /// `PropertyDeclaration`s when expanding a shorthand, for example.
///
/// The vector returned will not have the importance set;
/// this does not attempt to parse !important at all
pub fn parse_one_declaration(id: PropertyId, pub fn parse_one_declaration(id: PropertyId,
input: &str, input: &str,
base_url: &ServoUrl, base_url: &ServoUrl,
error_reporter: StdBox<ParseErrorReporter + Send>, error_reporter: StdBox<ParseErrorReporter + Send>,
extra_data: ParserContextExtraData) extra_data: ParserContextExtraData)
-> Result<Vec<PropertyDeclaration>, ()> { -> Result<Vec<(PropertyDeclaration, Importance)>, ()> {
let context = ParserContext::new_with_extra_data(Origin::Author, base_url, error_reporter, extra_data); let context = ParserContext::new_with_extra_data(Origin::Author, base_url, error_reporter, extra_data);
let mut results = vec![]; let mut results = vec![];
match PropertyDeclaration::parse(id, &context, &mut Parser::new(input), &mut results, false) { match PropertyDeclaration::parse(id, &context, &mut Parser::new(input), &mut results, false) {
@ -512,39 +515,51 @@ pub fn parse_one_declaration(id: PropertyId,
/// A struct to parse property declarations. /// A struct to parse property declarations.
struct PropertyDeclarationParser<'a, 'b: 'a> { struct PropertyDeclarationParser<'a, 'b: 'a> {
context: &'a ParserContext<'b>, context: &'a ParserContext<'b>,
declarations: Vec<(PropertyDeclaration, Importance)>
} }
/// Default methods reject all at rules. /// Default methods reject all at rules.
impl<'a, 'b> AtRuleParser for PropertyDeclarationParser<'a, 'b> { impl<'a, 'b> AtRuleParser for PropertyDeclarationParser<'a, 'b> {
type Prelude = (); type Prelude = ();
type AtRule = (Vec<PropertyDeclaration>, Importance); type AtRule = (u32, Importance);
} }
impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> { impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
/// A single declaration may be expanded into multiple ones if it's a /// Declarations are pushed to the internal vector. This will only
/// shorthand for example, so that's why this is a vector. /// let you know if the decl was !important and how many longhands
/// /// it expanded to
/// TODO(emilio): Seems like there's potentially a bunch of performance work type Declaration = (u32, Importance);
/// we could do here.
type Declaration = (Vec<PropertyDeclaration>, Importance);
fn parse_value(&mut self, name: &str, input: &mut Parser) fn parse_value(&mut self, name: &str, input: &mut Parser)
-> Result<(Vec<PropertyDeclaration>, Importance), ()> { -> Result<(u32, Importance), ()> {
let id = try!(PropertyId::parse(name.into())); let id = try!(PropertyId::parse(name.into()));
let mut results = vec![]; let old_len = self.declarations.len();
try!(input.parse_until_before(Delimiter::Bang, |input| { let parse_result = input.parse_until_before(Delimiter::Bang, |input| {
match PropertyDeclaration::parse(id, self.context, input, &mut results, false) { match PropertyDeclaration::parse(id, self.context, input, &mut self.declarations, false) {
PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => Ok(()), PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => Ok(()),
_ => Err(()) _ => Err(())
} }
})); });
if let Err(_) = parse_result {
// rollback
self.declarations.truncate(old_len);
return Err(())
}
let importance = match input.try(parse_important) { let importance = match input.try(parse_important) {
Ok(()) => Importance::Important, Ok(()) => Importance::Important,
Err(()) => Importance::Normal, Err(()) => Importance::Normal,
}; };
Ok((results, importance)) // In case there is still unparsed text in the declaration, we should roll back.
if !input.is_exhausted() {
self.declarations.truncate(old_len);
return Err(())
}
for decl in &mut self.declarations[old_len..] {
decl.1 = importance
}
Ok(((self.declarations.len() - old_len) as u32, importance))
} }
} }
@ -554,19 +569,18 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
pub fn parse_property_declaration_list(context: &ParserContext, pub fn parse_property_declaration_list(context: &ParserContext,
input: &mut Parser) input: &mut Parser)
-> PropertyDeclarationBlock { -> PropertyDeclarationBlock {
let mut declarations = Vec::new();
let mut important_count = 0; let mut important_count = 0;
let parser = PropertyDeclarationParser { let parser = PropertyDeclarationParser {
context: context, context: context,
declarations: vec![],
}; };
let mut iter = DeclarationListParser::new(input, parser); let mut iter = DeclarationListParser::new(input, parser);
while let Some(declaration) = iter.next() { while let Some(declaration) = iter.next() {
match declaration { match declaration {
Ok((results, importance)) => { Ok((count, importance)) => {
if importance.important() { if importance.important() {
important_count += results.len() as u32; important_count += count;
} }
declarations.extend(results.into_iter().map(|d| (d, importance)))
} }
Err(range) => { Err(range) => {
let pos = range.start; let pos = range.start;
@ -577,7 +591,7 @@ pub fn parse_property_declaration_list(context: &ParserContext,
} }
} }
let mut block = PropertyDeclarationBlock { let mut block = PropertyDeclarationBlock {
declarations: declarations, declarations: iter.parser.declarations,
important_count: important_count, important_count: important_count,
}; };
super::deduplicate_property_declarations(&mut block); super::deduplicate_property_declarations(&mut block);

11
components/style/properties/helpers.mako.rs
View file

@ -406,6 +406,7 @@
use cssparser::Parser; use cssparser::Parser;
use parser::ParserContext; use parser::ParserContext;
use properties::{longhands, PropertyDeclaration, DeclaredValue, ShorthandId}; use properties::{longhands, PropertyDeclaration, DeclaredValue, ShorthandId};
use properties::declaration_block::Importance;
use std::fmt; use std::fmt;
use style_traits::ToCss; use style_traits::ToCss;
use super::{SerializeFlags, ALL_INHERIT, ALL_INITIAL, ALL_UNSET}; use super::{SerializeFlags, ALL_INHERIT, ALL_INITIAL, ALL_UNSET};
@ -508,7 +509,7 @@
/// `declarations` vector. /// `declarations` vector.
pub fn parse(context: &ParserContext, pub fn parse(context: &ParserContext,
input: &mut Parser, input: &mut Parser,
declarations: &mut Vec<PropertyDeclaration>) declarations: &mut Vec<(PropertyDeclaration, Importance)>)
-> Result<(), ()> { -> Result<(), ()> {
input.look_for_var_functions(); input.look_for_var_functions();
let start = input.position(); let start = input.position();
@ -519,12 +520,12 @@
let var = input.seen_var_functions(); let var = input.seen_var_functions();
if let Ok(value) = value { if let Ok(value) = value {
% for sub_property in shorthand.sub_properties: % for sub_property in shorthand.sub_properties:
declarations.push(PropertyDeclaration::${sub_property.camel_case}( declarations.push((PropertyDeclaration::${sub_property.camel_case}(
match value.${sub_property.ident} { match value.${sub_property.ident} {
Some(value) => DeclaredValue::Value(value), Some(value) => DeclaredValue::Value(value),
None => DeclaredValue::Initial, None => DeclaredValue::Initial,
} }
)); ), Importance::Normal));
% endfor % endfor
Ok(()) Ok(())
} else if var { } else if var {
@ -532,14 +533,14 @@
let (first_token_type, css) = try!( let (first_token_type, css) = try!(
::custom_properties::parse_non_custom_with_var(input)); ::custom_properties::parse_non_custom_with_var(input));
% for sub_property in shorthand.sub_properties: % for sub_property in shorthand.sub_properties:
declarations.push(PropertyDeclaration::${sub_property.camel_case}( declarations.push((PropertyDeclaration::${sub_property.camel_case}(
DeclaredValue::WithVariables { DeclaredValue::WithVariables {
css: css.clone().into_owned(), css: css.clone().into_owned(),
first_token_type: first_token_type, first_token_type: first_token_type,
base_url: context.base_url.clone(), base_url: context.base_url.clone(),
from_shorthand: Some(ShorthandId::${shorthand.camel_case}), from_shorthand: Some(ShorthandId::${shorthand.camel_case}),
} }
)); ), Importance::Normal));
% endfor % endfor
Ok(()) Ok(())
} else { } else {

23
components/style/properties/properties.mako.rs
View file

@ -998,8 +998,12 @@ impl PropertyDeclaration {
/// > The <declaration-list> inside of <keyframe-block> accepts any CSS property /// > The <declaration-list> inside of <keyframe-block> accepts any CSS property
/// > except those defined in this specification, /// > except those defined in this specification,
/// > but does accept the `animation-play-state` property and interprets it specially. /// > but does accept the `animation-play-state` property and interprets it specially.
///
/// This will not actually parse Importance values, and will always set things
/// to Importance::Normal. Parsing Importance values is the job of PropertyDeclarationParser,
/// we only set them here so that we don't have to reallocate
pub fn parse(id: PropertyId, context: &ParserContext, input: &mut Parser, pub fn parse(id: PropertyId, context: &ParserContext, input: &mut Parser,
result_list: &mut Vec<PropertyDeclaration>, result_list: &mut Vec<(PropertyDeclaration, Importance)>,
in_keyframe_block: bool) in_keyframe_block: bool)
-> PropertyDeclarationParseResult { -> PropertyDeclarationParseResult {
match id { match id {
@ -1013,7 +1017,7 @@ impl PropertyDeclaration {
Err(()) => return PropertyDeclarationParseResult::InvalidValue, Err(()) => return PropertyDeclarationParseResult::InvalidValue,
} }
}; };
result_list.push(PropertyDeclaration::Custom(name, value)); result_list.push((PropertyDeclaration::Custom(name, value), Importance::Normal));
return PropertyDeclarationParseResult::ValidOrIgnoredDeclaration; return PropertyDeclarationParseResult::ValidOrIgnoredDeclaration;
} }
PropertyId::Longhand(id) => match id { PropertyId::Longhand(id) => match id {
@ -1035,7 +1039,8 @@ impl PropertyDeclaration {
match longhands::${property.ident}::parse_declared(context, input) { match longhands::${property.ident}::parse_declared(context, input) {
Ok(value) => { Ok(value) => {
result_list.push(PropertyDeclaration::${property.camel_case}(value)); result_list.push((PropertyDeclaration::${property.camel_case}(value),
Importance::Normal));
PropertyDeclarationParseResult::ValidOrIgnoredDeclaration PropertyDeclarationParseResult::ValidOrIgnoredDeclaration
}, },
Err(()) => PropertyDeclarationParseResult::InvalidValue, Err(()) => PropertyDeclarationParseResult::InvalidValue,
@ -1065,24 +1070,24 @@ impl PropertyDeclaration {
match input.try(|i| CSSWideKeyword::parse(context, i)) { match input.try(|i| CSSWideKeyword::parse(context, i)) {
Ok(CSSWideKeyword::InheritKeyword) => { Ok(CSSWideKeyword::InheritKeyword) => {
% for sub_property in shorthand.sub_properties: % for sub_property in shorthand.sub_properties:
result_list.push( result_list.push((
PropertyDeclaration::${sub_property.camel_case}( PropertyDeclaration::${sub_property.camel_case}(
DeclaredValue::Inherit)); DeclaredValue::Inherit), Importance::Normal));
% endfor % endfor
PropertyDeclarationParseResult::ValidOrIgnoredDeclaration PropertyDeclarationParseResult::ValidOrIgnoredDeclaration
}, },
Ok(CSSWideKeyword::InitialKeyword) => { Ok(CSSWideKeyword::InitialKeyword) => {
% for sub_property in shorthand.sub_properties: % for sub_property in shorthand.sub_properties:
result_list.push( result_list.push((
PropertyDeclaration::${sub_property.camel_case}( PropertyDeclaration::${sub_property.camel_case}(
DeclaredValue::Initial)); DeclaredValue::Initial), Importance::Normal));
% endfor % endfor
PropertyDeclarationParseResult::ValidOrIgnoredDeclaration PropertyDeclarationParseResult::ValidOrIgnoredDeclaration
}, },
Ok(CSSWideKeyword::UnsetKeyword) => { Ok(CSSWideKeyword::UnsetKeyword) => {
% for sub_property in shorthand.sub_properties: % for sub_property in shorthand.sub_properties:
result_list.push(PropertyDeclaration::${sub_property.camel_case}( result_list.push((PropertyDeclaration::${sub_property.camel_case}(
DeclaredValue::Unset)); DeclaredValue::Unset), Importance::Normal));
% endfor % endfor
PropertyDeclarationParseResult::ValidOrIgnoredDeclaration PropertyDeclarationParseResult::ValidOrIgnoredDeclaration
}, },

4
ports/geckolib/glue.rs
View file

@ -724,8 +724,6 @@ pub extern "C" fn Servo_ParseProperty(property: *const nsACString, value: *const
_ => return RawServoDeclarationBlockStrong::null(), _ => return RawServoDeclarationBlockStrong::null(),
} }
let results = results.into_iter().map(|r| (r, Importance::Normal)).collect();
Arc::new(RwLock::new(PropertyDeclarationBlock { Arc::new(RwLock::new(PropertyDeclarationBlock {
declarations: results, declarations: results,
important_count: 0, important_count: 0,
@ -863,7 +861,7 @@ fn set_property(declarations: RawServoDeclarationBlockBorrowed, property_id: Pro
let mut declarations = RwLock::<PropertyDeclarationBlock>::as_arc(&declarations).write(); let mut declarations = RwLock::<PropertyDeclarationBlock>::as_arc(&declarations).write();
let importance = if is_important { Importance::Important } else { Importance::Normal }; let importance = if is_important { Importance::Important } else { Importance::Normal };
for decl in decls.into_iter() { for decl in decls.into_iter() {
declarations.set_parsed_declaration(decl, importance); declarations.set_parsed_declaration(decl.0, importance);
} }
true true
} else { } else {