Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-11 08:25:32 +01:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #16954 - servo:arrayvec, r=emilio

Browse source 
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`.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16954)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-05-19 18:37:14 -05:00 • committed by GitHub
commit 60682cf81f
26 changed files with 488 additions and 386 deletions
Download patch file Download diff file Expand all files Collapse all files

68
tests/unit/style/parsing/border.rs
View file

@ -4,22 +4,34 @@
use parsing::parse;
use style::parser::Parse;
use style::properties::MaybeBoxed;
use style::properties::longhands::{border_image_outset, border_image_repeat, border_image_slice};
use style::properties::longhands::{border_image_source, border_image_width};
use style::properties::shorthands::border_image;
use style_traits::ToCss;
macro_rules! assert_longhand {
($parsed_shorthand: expr, $prop: ident, $value_string: expr) => {
assert_eq!($parsed_shorthand.$prop, parse_longhand!($prop, $value_string).maybe_boxed())
}
}
macro_rules! assert_initial {
($parsed_shorthand: expr, $prop: ident) => {
assert_eq!($parsed_shorthand.$prop, $prop::get_initial_specified_value().maybe_boxed())
}
}
#[test]
fn border_image_shorthand_should_parse_when_all_properties_specified() {
let input = "linear-gradient(red, blue) 30 30% 45 fill / 20px 40px / 10px round stretch";
let result = parse(border_image::parse_value, input).unwrap();
assert_eq!(result.border_image_source,
parse_longhand!(border_image_source, "linear-gradient(red, blue)"));
assert_eq!(result.border_image_slice, parse_longhand!(border_image_slice, "30 30% 45 fill"));
assert_eq!(result.border_image_width, parse_longhand!(border_image_width, "20px 40px"));
assert_eq!(result.border_image_outset, parse_longhand!(border_image_outset, "10px"));
assert_eq!(result.border_image_repeat, parse_longhand!(border_image_repeat, "round stretch"));
assert_longhand!(result, border_image_source, "linear-gradient(red, blue)");
assert_longhand!(result, border_image_slice, "30 30% 45 fill");
assert_longhand!(result, border_image_width, "20px 40px");
assert_longhand!(result, border_image_outset, "10px");
assert_longhand!(result, border_image_repeat, "round stretch");
}
#[test]
@ -27,12 +39,11 @@ fn border_image_shorthand_should_parse_without_width() {
let input = "linear-gradient(red, blue) 30 30% 45 fill / / 10px round stretch";
let result = parse(border_image::parse_value, input).unwrap();
assert_eq!(result.border_image_source,
parse_longhand!(border_image_source, "linear-gradient(red, blue)"));
assert_eq!(result.border_image_slice, parse_longhand!(border_image_slice, "30 30% 45 fill"));
assert_eq!(result.border_image_outset, parse_longhand!(border_image_outset, "10px"));
assert_eq!(result.border_image_repeat, parse_longhand!(border_image_repeat, "round stretch"));
assert_eq!(result.border_image_width, border_image_width::get_initial_specified_value());
assert_longhand!(result, border_image_source, "linear-gradient(red, blue)");
assert_longhand!(result, border_image_slice, "30 30% 45 fill");
assert_longhand!(result, border_image_outset, "10px");
assert_longhand!(result, border_image_repeat, "round stretch");
assert_initial!(result, border_image_width);
}
#[test]
@ -40,12 +51,11 @@ fn border_image_shorthand_should_parse_without_outset() {
let input = "linear-gradient(red, blue) 30 30% 45 fill / 20px 40px round";
let result = parse(border_image::parse_value, input).unwrap();
assert_eq!(result.border_image_source,
parse_longhand!(border_image_source, "linear-gradient(red, blue)"));
assert_eq!(result.border_image_slice, parse_longhand!(border_image_slice, "30 30% 45 fill"));
assert_eq!(result.border_image_width, parse_longhand!(border_image_width, "20px 40px"));
assert_eq!(result.border_image_repeat, parse_longhand!(border_image_repeat, "round"));
assert_eq!(result.border_image_outset, border_image_outset::get_initial_specified_value());
assert_longhand!(result, border_image_source, "linear-gradient(red, blue)");
assert_longhand!(result, border_image_slice, "30 30% 45 fill");
assert_longhand!(result, border_image_width, "20px 40px");
assert_longhand!(result, border_image_repeat, "round");
assert_initial!(result, border_image_outset);
}
#[test]
@ -53,24 +63,22 @@ fn border_image_shorthand_should_parse_without_width_or_outset() {
let input = "linear-gradient(red, blue) 30 30% 45 fill round";
let result = parse(border_image::parse_value, input).unwrap();
assert_eq!(result.border_image_source,
parse_longhand!(border_image_source, "linear-gradient(red, blue)"));
assert_eq!(result.border_image_slice, parse_longhand!(border_image_slice, "30 30% 45 fill"));
assert_eq!(result.border_image_repeat, parse_longhand!(border_image_repeat, "round"));
assert_eq!(result.border_image_width, border_image_width::get_initial_specified_value());
assert_eq!(result.border_image_outset, border_image_outset::get_initial_specified_value());
assert_longhand!(result, border_image_source, "linear-gradient(red, blue)");
assert_longhand!(result, border_image_slice, "30 30% 45 fill");
assert_longhand!(result, border_image_repeat, "round");
assert_initial!(result, border_image_width);
assert_initial!(result, border_image_outset);
}
#[test]
fn border_image_shorthand_should_parse_with_just_source() {
let result = parse(border_image::parse_value, "linear-gradient(red, blue)").unwrap();
assert_eq!(result.border_image_source,
parse_longhand!(border_image_source, "linear-gradient(red, blue)"));
assert_eq!(result.border_image_slice, border_image_slice::get_initial_specified_value());
assert_eq!(result.border_image_width, border_image_width::get_initial_specified_value());
assert_eq!(result.border_image_outset, border_image_outset::get_initial_specified_value());
assert_eq!(result.border_image_repeat, border_image_repeat::get_initial_specified_value());
assert_longhand!(result, border_image_source, "linear-gradient(red, blue)");
assert_initial!(result, border_image_slice);
assert_initial!(result, border_image_width);
assert_initial!(result, border_image_outset);
assert_initial!(result, border_image_repeat);
}
#[test]

4
tests/unit/style/size_of.rs
View file

@ -6,6 +6,10 @@ use style::properties;
size_of_test!(test_size_of_property_declaration, properties::PropertyDeclaration, 32);
// This is huge, but we allocate it on the stack and then never move it,
// we only pass `&mut SourcePropertyDeclaration` references around.
size_of_test!(test_size_of_parsed_declaration, properties::SourcePropertyDeclaration, 576);
#[test]
fn size_of_specified_values() {
::style::properties::test_size_of_specified_values();

4
tests/unit/stylo/size_of.rs
View file

@ -21,6 +21,10 @@ fn size_of_selectors_dummy_types() {
size_of_test!(test_size_of_property_declaration, style::properties::PropertyDeclaration, 32);
// This is huge, but we allocate it on the stack and then never move it,
// we only pass `&mut SourcePropertyDeclaration` references around.
size_of_test!(test_size_of_parsed_declaration, style::properties::SourcePropertyDeclaration, 704);
#[test]
fn size_of_specified_values() {
::style::properties::test_size_of_specified_values();