From dc5dfafbbadba076b5a95fb4b9b6b8d3c38c741b Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 21 Aug 2017 16:07:03 +0200 Subject: [PATCH] Use Parser::skip_whitespace in a few places to make Parser::try rewind less. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ``` --- Cargo.lock | 43 ++++++++++--------- components/selectors/Cargo.toml | 2 +- components/selectors/parser.rs | 19 ++------ components/style/Cargo.toml | 2 +- .../style/properties/properties.mako.rs | 8 +++- components/style_traits/values.rs | 14 ++++-- 6 files changed, 44 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 97da598d50d..5084c077c5c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -327,7 +327,7 @@ dependencies = [ "azure 0.20.1 (git+https://github.com/servo/rust-azure)", "canvas_traits 0.0.1", "compositing 0.0.1", - "cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", "gleam 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", @@ -343,7 +343,7 @@ dependencies = [ name = "canvas_traits" version = "0.0.1" dependencies = [ - "cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)", "heapsize 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "heapsize_derive 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -591,7 +591,7 @@ dependencies = [ [[package]] name = "cssparser" -version = "0.19.2" +version = "0.19.5" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "cssparser-macros 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -602,6 +602,7 @@ dependencies = [ "procedural-masquerade 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "quote 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.8 (registry+https://github.com/rust-lang/crates.io-index)", + "smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "syn 0.11.11 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1070,7 +1071,7 @@ name = "geckoservo" version = "0.0.1" dependencies = [ "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1122,7 +1123,7 @@ dependencies = [ "servo_geometry 0.0.1", "servo_url 0.0.1", "simd 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", - "smallvec 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "style 0.0.1", "style_traits 0.0.1", "time 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1138,7 +1139,7 @@ dependencies = [ name = "gfx_tests" version = "0.0.1" dependencies = [ - "cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)", "gfx 0.0.1", "ipc-channel 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "style 0.0.1", @@ -1526,7 +1527,7 @@ dependencies = [ "servo_config 0.0.1", "servo_geometry 0.0.1", "servo_url 0.0.1", - "smallvec 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "style 0.0.1", "style_traits 0.0.1", "unicode-bidi 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2191,7 +2192,7 @@ dependencies = [ "kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)", - "smallvec 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -2521,7 +2522,7 @@ dependencies = [ "caseless 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "cmake 0.1.22 (registry+https://github.com/rust-lang/crates.io-index)", "cookie 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)", "deny_public_fields 0.0.1", "devtools_traits 0.0.1", "dom_struct 0.0.1", @@ -2571,7 +2572,7 @@ dependencies = [ "servo_geometry 0.0.1", "servo_rand 0.0.1", "servo_url 0.0.1", - "smallvec 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "style 0.0.1", "style_traits 0.0.1", "swapper 0.0.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2594,7 +2595,7 @@ dependencies = [ "app_units 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "canvas_traits 0.0.1", - "cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)", "gfx_traits 0.0.1", "heapsize 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2666,7 +2667,7 @@ name = "selectors" version = "0.19.0" dependencies = [ "bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2675,7 +2676,7 @@ dependencies = [ "precomputed-hash 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "servo_arc 0.0.1", "size_of_test 0.0.1", - "smallvec 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -3005,7 +3006,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "smallvec" -version = "0.4.0" +version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "heapsize 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3065,7 +3066,7 @@ dependencies = [ "bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)", "encoding 0.2.33 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3095,7 +3096,7 @@ dependencies = [ "servo_atoms 0.0.1", "servo_config 0.0.1", "servo_url 0.0.1", - "smallvec 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "style_derive 0.0.1", "style_traits 0.0.1", "time 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3121,7 +3122,7 @@ version = "0.0.1" dependencies = [ "app_units 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)", "html5ever 0.19.0 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3143,7 +3144,7 @@ version = "0.0.1" dependencies = [ "app_units 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", "bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)", "heapsize 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "heapsize_derive 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3158,7 +3159,7 @@ name = "stylo_tests" version = "0.0.1" dependencies = [ "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)", "geckoservo 0.0.1", @@ -3746,7 +3747,7 @@ dependencies = [ "checksum core-foundation-sys 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "41115a6aa5d3e1e5ef98148373f25971d1fad53818553f216495f9e67e90a624" "checksum core-graphics 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "a9f841e9637adec70838c537cae52cb4c751cc6514ad05669b51d107c2021c79" "checksum core-text 6.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "16ce16d9ed00181016c11ff48e561314bec92bfbce9fe48f319366618d4e5de6" -"checksum cssparser 0.19.2 (registry+https://github.com/rust-lang/crates.io-index)" = "1f9442c00898020a56c9485d64c9c8f14ae30ba45be89d15846046593383467f" +"checksum cssparser 0.19.5 (registry+https://github.com/rust-lang/crates.io-index)" = "dc476dc0960774aa1cabfd0044de7d4585a8f2f8a3ef72e6d9d1e16c1e2492b1" "checksum cssparser-macros 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "079adec4af52bb5275eadd004292028c79eb3c5f5b4ee8086a36d4197032f6df" "checksum darling 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9861a8495606435477df581bc858ccf15a3469747edf175b94a4704fd9aaedac" "checksum darling_core 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1486a8b00b45062c997f767738178b43219133dd0c8c826cb811e60563810821" @@ -3929,7 +3930,7 @@ dependencies = [ "checksum siphasher 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "0df90a788073e8d0235a67e50441d47db7c8ad9debd91cbf43736a2a92d36537" "checksum skeptic 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "dd7d8dc1315094150052d0ab767840376335a98ac66ef313ff911cdf439a5b69" "checksum slab 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "17b4fcaed89ab08ef143da37bc52adbcc04d4a69014f4c1208d6b51f0c47bc23" -"checksum smallvec 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "2e40af10aafe98b4d8294ae8388d8a5cd0707c65d364872efe72d063ec44bee0" +"checksum smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "8fcd03faf178110ab0334d74ca9631d77f94c8c11cc77fcb59538abf0025695d" "checksum stable_deref_trait 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "15132e0e364248108c5e2c02e3ab539be8d6f5d52a01ca9bbf27ed657316f02b" "checksum string_cache 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "413fc7852aeeb5472f1986ef755f561ddf0c789d3d796e65f0b6fe293ecd4ef8" "checksum string_cache_codegen 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "479cde50c3539481f33906a387f2bd17c8e87cb848c35b6021d41fb81ff9b4d7" diff --git a/components/selectors/Cargo.toml b/components/selectors/Cargo.toml index 465365434c2..16ed24a31ce 100644 --- a/components/selectors/Cargo.toml +++ b/components/selectors/Cargo.toml @@ -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" diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 1948523c0b3..bbc1dc4a246 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -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; 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. @@ -1468,14 +1461,8 @@ fn parse_compound_selector<'i, 't, P, E, Impl>( -> Result>> 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() { diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index 68afeef84f9..8105639b446 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -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" diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 60022596fb4..f21e33437a0 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -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) { @@ -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. @@ -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) @@ -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| { diff --git a/components/style_traits/values.rs b/components/style_traits/values.rs index 4888753c9ef..5e4c093fd73 100644 --- a/components/style_traits/values.rs +++ b/components/style_traits/values.rs @@ -246,11 +246,16 @@ impl Separator for Space { where F: for<'tt> FnMut(&mut Parser<'i, 'tt>) -> Result> { + 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) } } @@ -266,9 +271,12 @@ impl Separator for CommaWithSpace { where F: for<'tt> FnMut(&mut Parser<'i, 'tt>) -> Result> { + 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 {