Skip to content

Commit

Permalink
Use Parser::skip_whitespace in a few places to make Parser::try rewin…
Browse files Browse the repository at this point in the history
…d less.

Gecko’s CSS parsing microbenchmarks before:

```
  43.437 ±  0.391 ms    Stylo.Servo_StyleSheet_FromUTF8Bytes_Bench
  29.244 ±  0.042 ms    Stylo.Gecko_nsCSSParser_ParseSheet_Bench
 281.884 ±  0.028 ms    Stylo.Servo_DeclarationBlock_SetPropertyById_Bench
 426.242 ±  0.008 ms    Stylo.Servo_DeclarationBlock_SetPropertyById_WithInitialSpace_Bench
```

After:

```
  29.779 ±  0.254 ms    Stylo.Servo_StyleSheet_FromUTF8Bytes_Bench
  28.841 ±  0.031 ms    Stylo.Gecko_nsCSSParser_ParseSheet_Bench
 296.240 ±  4.744 ms    Stylo.Servo_DeclarationBlock_SetPropertyById_Bench
 293.855 ±  4.304 ms    Stylo.Servo_DeclarationBlock_SetPropertyById_WithInitialSpace_Bench
```
  • Loading branch information
SimonSapin committed Aug 23, 2017
1 parent 2e775ab commit 45b8cd0
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 34 deletions.
26 changes: 13 additions & 13 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion components/selectors/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ unstable = []
[dependencies]
bitflags = "0.7"
matches = "0.1"
cssparser = "0.19"
cssparser = "0.19.3"
log = "0.3"
fnv = "1.0"
phf = "0.7.18"
Expand Down
19 changes: 3 additions & 16 deletions components/selectors/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1425,14 +1425,7 @@ fn parse_negation<'i, 't, P, E, Impl>(parser: &P,
// We use a sequence because a type selector may be represented as two Components.
let mut sequence = SmallVec::<[Component<Impl>; 2]>::new();

// Consume any leading whitespace.
loop {
let before_this_token = input.state();
if !matches!(input.next_including_whitespace(), Ok(&Token::WhiteSpace(_))) {
input.reset(&before_this_token);
break
}
}
input.skip_whitespace();

// Get exactly one simple selector. The parse logic in the caller will verify
// that there are no trailing tokens after we're done.
Expand Down Expand Up @@ -1468,14 +1461,8 @@ fn parse_compound_selector<'i, 't, P, E, Impl>(
-> Result<bool, ParseError<'i, SelectorParseError<'i, E>>>
where P: Parser<'i, Impl=Impl, Error=E>, Impl: SelectorImpl
{
// Consume any leading whitespace.
loop {
let before_this_token = input.state();
if !matches!(input.next_including_whitespace(), Ok(&Token::WhiteSpace(_))) {
input.reset(&before_this_token);
break
}
}
input.skip_whitespace();

let mut empty = true;
if !parse_type_selector(parser, input, builder)? {
if let Some(url) = parser.default_namespace() {
Expand Down
2 changes: 1 addition & 1 deletion components/style/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ bitflags = "0.7"
bit-vec = "0.4.3"
byteorder = "1.0"
cfg-if = "0.1.0"
cssparser = "0.19"
cssparser = "0.19.3"
encoding = {version = "0.2", optional = true}
euclid = "0.15"
fnv = "1.0"
Expand Down
5 changes: 5 additions & 0 deletions components/style/properties/properties.mako.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1552,6 +1552,9 @@ impl PropertyDeclaration {
assert!(declarations.is_empty());
match id {
PropertyId::Custom(name) => {
// FIXME: fully implement https://github.com/w3c/csswg-drafts/issues/774
// before adding skip_whitespace here.
// This probably affects some test results.
let value = match input.try(|i| CSSWideKeyword::parse(i)) {
Ok(keyword) => DeclaredValueOwned::CSSWideKeyword(keyword),
Err(()) => match ::custom_properties::SpecifiedValue::parse(context, input) {
Expand All @@ -1564,6 +1567,7 @@ impl PropertyDeclaration {
Ok(())
}
PropertyId::Longhand(id) => {
input.skip_whitespace(); // Unnecessary for correctness, but may help try() rewind less.
input.try(|i| CSSWideKeyword::parse(i)).map(|keyword| {
PropertyDeclaration::CSSWideKeyword(id, keyword)
}).or_else(|()| {
Expand Down Expand Up @@ -1595,6 +1599,7 @@ impl PropertyDeclaration {
})
}
PropertyId::Shorthand(id) => {
input.skip_whitespace(); // Unnecessary for correctness, but may help try() rewind less.
if let Ok(keyword) = input.try(|i| CSSWideKeyword::parse(i)) {
if id == ShorthandId::All {
declarations.all_shorthand = AllShorthand::CSSWideKeyword(keyword)
Expand Down
14 changes: 11 additions & 3 deletions components/style_traits/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,16 @@ impl Separator for Space {
where
F: for<'tt> FnMut(&mut Parser<'i, 'tt>) -> Result<T, ParseError<'i, E>>
{
input.skip_whitespace(); // Unnecessary for correctness, but may help try() rewind less.
let mut results = vec![parse_one(input)?];
while let Ok(item) = input.try(&mut parse_one) {
results.push(item);
loop {
input.skip_whitespace(); // Unnecessary for correctness, but may help try() rewind less.
if let Ok(item) = input.try(&mut parse_one) {
results.push(item);
} else {
return Ok(results)
}
}
Ok(results)
}
}

Expand All @@ -266,9 +271,12 @@ impl Separator for CommaWithSpace {
where
F: for<'tt> FnMut(&mut Parser<'i, 'tt>) -> Result<T, ParseError<'i, E>>
{
input.skip_whitespace(); // Unnecessary for correctness, but may help try() rewind less.
let mut results = vec![parse_one(input)?];
loop {
input.skip_whitespace(); // Unnecessary for correctness, but may help try() rewind less.
let comma = input.try(|i| i.expect_comma()).is_ok();
input.skip_whitespace(); // Unnecessary for correctness, but may help try() rewind less.
if let Ok(item) = input.try(&mut parse_one) {
results.push(item);
} else if comma {
Expand Down

0 comments on commit 45b8cd0

Please sign in to comment.