Skip to content

Commit

Permalink
servo: Merge #18171 - Use Parser::skip_whitespace in a few places to …
Browse files Browse the repository at this point in the history
…make Parser::try rewind less (from servo:skip_whitespace); r=emilio

**Do not merge yet.** This pulls in unrelated cssparser changes which add a dependency to `dota`, and we’d like to resolve dtolnay/dtoa#9 before landing dtoa in mozilla-central. Also, the dependency change may require manually revendoring in mozilla-central.

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
```

Source-Repo: https://github.com/servo/servo
Source-Revision: 8812422bfa504633a7011b0b002e9a2682c7d045

UltraBlame original commit: 75a20ebcd6e8c43b2f0c332b6d5ce947ba1f5ad7
  • Loading branch information
marco-c committed Oct 1, 2019
1 parent fec0088 commit 63a7c94
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 44 deletions.
43 changes: 22 additions & 21 deletions servo/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 servo/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 servo/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,

let mut sequence = SmallVec::<[Component<Impl>; 2]>::new();


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();



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
{

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 servo/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
8 changes: 6 additions & 2 deletions servo/components/style/properties/properties.mako.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1547,8 +1547,12 @@ impl PropertyDeclaration {
id: PropertyId, context: &ParserContext, input: &mut Parser<'i, 't>)
-> Result<(), PropertyDeclarationParseError<'i>> {
assert!(declarations.is_empty());
let start = input.state();
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 @@ -1561,11 +1565,11 @@ 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(|()| {
input.look_for_var_functions();
let start = input.state();
input.parse_entirely(|input| id.parse_value(context, input))
.or_else(|err| {
while let Ok(_) = input.next() {} // Look for var() after the error.
Expand All @@ -1592,6 +1596,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 All @@ -1603,7 +1608,6 @@ impl PropertyDeclaration {
Ok(())
} else {
input.look_for_var_functions();
let start = input.state();
// Not using parse_entirely here: each ${shorthand.ident}::parse_into function
// needs to do so *before* pushing to `declarations`.
id.parse_into(declarations, context, input).or_else(|err| {
Expand Down
14 changes: 11 additions & 3 deletions servo/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();
let mut results = vec![parse_one(input)?];
while let Ok(item) = input.try(&mut parse_one) {
results.push(item);
loop {
input.skip_whitespace();
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();
let mut results = vec![parse_one(input)?];
loop {
input.skip_whitespace();
let comma = input.try(|i| i.expect_comma()).is_ok();
input.skip_whitespace();
if let Ok(item) = input.try(&mut parse_one) {
results.push(item);
} else if comma {
Expand Down

0 comments on commit 63a7c94

Please sign in to comment.