mirror of
https://github.com/servo/servo.git
synced 2025-08-06 06:00:15 +01:00
Avoid returning / passing around a huge ParsedDeclaration type
This enum type used to contain the result of parsing one CSS source declaration (`name: value;`) and expanding shorthands. Enum types are as big as the biggest of their variant (plus discriminant), which was quite big because some shorthands expand to many longhand properties. This type was returned through many functions and methods, wrapped and rewrapped in `Result` with different error types. This presumably caused significant `memmove` traffic. Instead, we now allocate an `ArrayVec` on the stack and pass `&mut` references to it for various functions to push into it. This type is also very big, but we never move it. We still use an intermediate data structure because we sometimes decide after shorthand expansion that a declaration is invalid after all and that we’re gonna drop it. Only later do we push to a `PropertyDeclarationBlock`, with an entire `ArrayVec` or nothing. In future work we can try to avoid a large stack-allocated array, and instead writing directly to the heap allocation of the `Vec` inside `PropertyDeclarationBlock`. However this is tricky: we need to preserve this "all or nothing" aspect of parsing one source declaration, and at the same time we want to make it as little error-prone as possible for the various call sites. `PropertyDeclarationBlock` curently does property deduplication incrementally: as each `PropertyDeclaration` is pushed, we check if an existing declaration of the same property exists and if so overwrite it. To get rid of the stack allocated array we’d need to somehow deduplicate separately after pushing multiple `PropertyDeclaration`.
This commit is contained in:
parent
e4f241389e
commit
d2be5239f5
12 changed files with 356 additions and 303 deletions
|
@ -202,14 +202,52 @@ impl PropertyDeclarationBlock {
|
|||
}
|
||||
|
||||
/// Adds or overrides the declaration for a given property in this block,
|
||||
/// except if an existing declaration for the same property is more important.
|
||||
/// **except** if an existing declaration for the same property is more important.
|
||||
pub fn extend(&mut self, drain: SourcePropertyDeclarationDrain, importance: Importance) {
|
||||
self.extend_common(drain, importance, false);
|
||||
}
|
||||
|
||||
/// Adds or overrides the declaration for a given property in this block,
|
||||
/// **even** if an existing declaration for the same property is more important.
|
||||
///
|
||||
/// Return whether anything changed.
|
||||
pub fn extend_reset(&mut self, drain: SourcePropertyDeclarationDrain,
|
||||
importance: Importance) -> bool {
|
||||
self.extend_common(drain, importance, true)
|
||||
}
|
||||
|
||||
fn extend_common(&mut self, mut drain: SourcePropertyDeclarationDrain,
|
||||
importance: Importance, overwrite_more_important: bool) -> bool {
|
||||
let mut changed = false;
|
||||
for decl in &mut drain.declarations {
|
||||
changed |= self.push_common(decl, importance, overwrite_more_important);
|
||||
}
|
||||
match drain.all_shorthand {
|
||||
AllShorthand::NotSet => {}
|
||||
AllShorthand::CSSWideKeyword(keyword) => {
|
||||
for &id in ShorthandId::All.longhands() {
|
||||
let decl = PropertyDeclaration::CSSWideKeyword(id, keyword);
|
||||
changed |= self.push_common(decl, importance, overwrite_more_important);
|
||||
}
|
||||
}
|
||||
AllShorthand::WithVariables(unparsed) => {
|
||||
for &id in ShorthandId::All.longhands() {
|
||||
let decl = PropertyDeclaration::WithVariables(id, unparsed.clone());
|
||||
changed |= self.push_common(decl, importance, overwrite_more_important);
|
||||
}
|
||||
}
|
||||
}
|
||||
changed
|
||||
}
|
||||
|
||||
/// Adds or overrides the declaration for a given property in this block,
|
||||
/// **except** if an existing declaration for the same property is more important.
|
||||
pub fn push(&mut self, declaration: PropertyDeclaration, importance: Importance) {
|
||||
self.push_common(declaration, importance, false);
|
||||
}
|
||||
|
||||
/// Implementation detail of push and ParsedDeclaration::expand*
|
||||
pub fn push_common(&mut self, declaration: PropertyDeclaration, importance: Importance,
|
||||
overwrite_more_important: bool) -> bool {
|
||||
fn push_common(&mut self, declaration: PropertyDeclaration, importance: Importance,
|
||||
overwrite_more_important: bool) -> bool {
|
||||
let definitely_new = if let PropertyDeclarationId::Longhand(id) = declaration.id() {
|
||||
!self.longhands.contains(id)
|
||||
} else {
|
||||
|
@ -657,15 +695,15 @@ pub fn parse_style_attribute(input: &str,
|
|||
/// Parse a given property declaration. Can result in multiple
|
||||
/// `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,
|
||||
input: &str,
|
||||
url_data: &UrlExtraData,
|
||||
error_reporter: &ParseErrorReporter,
|
||||
parsing_mode: ParsingMode,
|
||||
quirks_mode: QuirksMode)
|
||||
-> Result<ParsedDeclaration, ()> {
|
||||
/// This does not attempt to parse !important at all.
|
||||
pub fn parse_one_declaration_into(declarations: &mut SourcePropertyDeclaration,
|
||||
id: PropertyId,
|
||||
input: &str,
|
||||
url_data: &UrlExtraData,
|
||||
error_reporter: &ParseErrorReporter,
|
||||
parsing_mode: ParsingMode,
|
||||
quirks_mode: QuirksMode)
|
||||
-> Result<(), ()> {
|
||||
let context = ParserContext::new(Origin::Author,
|
||||
url_data,
|
||||
error_reporter,
|
||||
|
@ -673,7 +711,7 @@ pub fn parse_one_declaration(id: PropertyId,
|
|||
parsing_mode,
|
||||
quirks_mode);
|
||||
Parser::new(input).parse_entirely(|parser| {
|
||||
ParsedDeclaration::parse(id, &context, parser)
|
||||
PropertyDeclaration::parse_into(declarations, id, &context, parser)
|
||||
.map_err(|_| ())
|
||||
})
|
||||
}
|
||||
|
@ -681,24 +719,23 @@ pub fn parse_one_declaration(id: PropertyId,
|
|||
/// A struct to parse property declarations.
|
||||
struct PropertyDeclarationParser<'a, 'b: 'a> {
|
||||
context: &'a ParserContext<'b>,
|
||||
declarations: &'a mut SourcePropertyDeclaration,
|
||||
}
|
||||
|
||||
|
||||
/// Default methods reject all at rules.
|
||||
impl<'a, 'b> AtRuleParser for PropertyDeclarationParser<'a, 'b> {
|
||||
type Prelude = ();
|
||||
type AtRule = (ParsedDeclaration, Importance);
|
||||
type AtRule = Importance;
|
||||
}
|
||||
|
||||
|
||||
impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
|
||||
type Declaration = (ParsedDeclaration, Importance);
|
||||
type Declaration = Importance;
|
||||
|
||||
fn parse_value(&mut self, name: &str, input: &mut Parser)
|
||||
-> Result<(ParsedDeclaration, Importance), ()> {
|
||||
fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<Importance, ()> {
|
||||
let id = try!(PropertyId::parse(name.into()));
|
||||
let parsed = input.parse_until_before(Delimiter::Bang, |input| {
|
||||
ParsedDeclaration::parse(id, self.context, input)
|
||||
input.parse_until_before(Delimiter::Bang, |input| {
|
||||
PropertyDeclaration::parse_into(self.declarations, id, self.context, input)
|
||||
.map_err(|_| ())
|
||||
})?;
|
||||
let importance = match input.try(parse_important) {
|
||||
|
@ -706,10 +743,8 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
|
|||
Err(()) => Importance::Normal,
|
||||
};
|
||||
// In case there is still unparsed text in the declaration, we should roll back.
|
||||
if !input.is_exhausted() {
|
||||
return Err(())
|
||||
}
|
||||
Ok((parsed, importance))
|
||||
input.expect_exhausted()?;
|
||||
Ok(importance)
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -719,15 +754,20 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
|
|||
pub fn parse_property_declaration_list(context: &ParserContext,
|
||||
input: &mut Parser)
|
||||
-> PropertyDeclarationBlock {
|
||||
let mut declarations = SourcePropertyDeclaration::new();
|
||||
let mut block = PropertyDeclarationBlock::new();
|
||||
let parser = PropertyDeclarationParser {
|
||||
context: context,
|
||||
declarations: &mut declarations,
|
||||
};
|
||||
let mut iter = DeclarationListParser::new(input, parser);
|
||||
while let Some(declaration) = iter.next() {
|
||||
match declaration {
|
||||
Ok((parsed, importance)) => parsed.expand_push_into(&mut block, importance),
|
||||
Ok(importance) => {
|
||||
block.extend(iter.parser.declarations.drain(), importance);
|
||||
}
|
||||
Err(range) => {
|
||||
iter.parser.declarations.clear();
|
||||
let pos = range.start;
|
||||
let message = format!("Unsupported property declaration: '{}'",
|
||||
iter.input.slice(range));
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue