From f48b95e2e3879d38314ab84999f0c17055a660d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 14 Aug 2023 22:05:25 +0200 Subject: [PATCH] style: Move size of tests to compile-time tests in the style crate Same reasoning as the previous commit. Differential Revision: https://phabricator.services.mozilla.com/D146104 --- components/size_of_test/Cargo.toml | 10 ------ components/size_of_test/lib.rs | 34 ------------------- components/style/applicable_declarations.rs | 3 ++ components/style/data.rs | 6 ++++ components/style/gecko/selector_parser.rs | 6 ++++ components/style/macros.rs | 8 +++++ .../style/properties/properties.mako.rs | 12 +++++++ components/style/rule_tree/core.rs | 8 +++-- components/style/rule_tree/mod.rs | 2 +- components/style/stylist.rs | 6 ++++ components/style/values/computed/image.rs | 3 ++ components/style/values/specified/image.rs | 3 ++ tests/unit/style/lib.rs | 1 - 13 files changed, 53 insertions(+), 49 deletions(-) delete mode 100644 components/size_of_test/Cargo.toml delete mode 100644 components/size_of_test/lib.rs diff --git a/components/size_of_test/Cargo.toml b/components/size_of_test/Cargo.toml deleted file mode 100644 index 4776581cc52..00000000000 --- a/components/size_of_test/Cargo.toml +++ /dev/null @@ -1,10 +0,0 @@ -[package] -name = "size_of_test" -version = "0.0.1" -authors = ["The Servo Project Developers"] -license = "MPL-2.0" -edition = "2018" -publish = false - -[lib] -path = "lib.rs" diff --git a/components/size_of_test/lib.rs b/components/size_of_test/lib.rs deleted file mode 100644 index 4e2452232f3..00000000000 --- a/components/size_of_test/lib.rs +++ /dev/null @@ -1,34 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ - -#[macro_export] -macro_rules! size_of_test { - ($testname: ident, $t: ty, $expected_size: expr) => { - #[test] - fn $testname() { - let new = ::std::mem::size_of::<$t>(); - let old = $expected_size; - if new < old { - panic!( - "Your changes have decreased the stack size of {} from {} to {}. \ - Good work! Please update the expected size in {}.", - stringify!($t), - old, - new, - file!() - ) - } else if new > old { - panic!( - "Your changes have increased the stack size of {} from {} to {}. \ - Please consider choosing a design which avoids this increase. \ - If you feel that the increase is necessary, update the size in {}.", - stringify!($t), - old, - new, - file!() - ) - } - } - }; -} diff --git a/components/style/applicable_declarations.rs b/components/style/applicable_declarations.rs index 5849e7c2250..3a55060cc87 100644 --- a/components/style/applicable_declarations.rs +++ b/components/style/applicable_declarations.rs @@ -208,3 +208,6 @@ impl ApplicableDeclarationBlock { (self.source, self.cascade_priority) } } + +// Size of this struct determines sorting and selector-matching performance. +size_of_test!(ApplicableDeclarationBlock, 24); diff --git a/components/style/data.rs b/components/style/data.rs index 758adf57a68..2fa0b39c24d 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -159,6 +159,9 @@ pub struct ElementStyles { pub pseudos: EagerPseudoStyles, } +// There's one of these per rendered elements so it better be small. +size_of_test!(ElementStyles, 16); + impl ElementStyles { /// Returns the primary style. pub fn get_primary(&self) -> Option<&Arc> { @@ -249,6 +252,9 @@ pub struct ElementData { pub flags: ElementDataFlags, } +// There's one of these per rendered elements so it better be small. +size_of_test!(ElementData, 24); + /// The kind of restyle that a single element should do. #[derive(Debug)] pub enum RestyleKind { diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index 336df501325..7cfb01e2794 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -445,3 +445,9 @@ unsafe impl HasFFI for SelectorList { } unsafe impl HasSimpleFFI for SelectorList {} unsafe impl HasBoxFFI for SelectorList {} + +// Selector and component sizes are important for matching performance. +size_of_test!(selectors::parser::Selector, 8); +size_of_test!(selectors::parser::Component, 24); +size_of_test!(PseudoElement, 16); +size_of_test!(NonTSPseudoClass, 16); diff --git a/components/style/macros.rs b/components/style/macros.rs index e27a554acf4..3084d33dd3d 100644 --- a/components/style/macros.rs +++ b/components/style/macros.rs @@ -128,3 +128,11 @@ macro_rules! local_name { $crate::values::AtomIdent(atom!($s)) }; } + +/// Asserts the size of a type at compile time. +macro_rules! size_of_test { + ($t: ty, $expected_size: expr) => { + #[cfg(target_pointer_width = "64")] + const_assert_eq!(std::mem::size_of::<$t>(), $expected_size); + }; +} diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 979d6718226..1bc210fd6ff 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -267,6 +267,9 @@ pub enum PropertyDeclaration { % endfor } +// There's one of these for each parsed declaration so it better be small. +size_of_test!(PropertyDeclaration, 32); + #[repr(C)] struct PropertyDeclarationVariantRepr { tag: u16, @@ -2604,6 +2607,10 @@ pub struct SourcePropertyDeclaration { all_shorthand: AllShorthand, } +// 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!(SourcePropertyDeclaration, 600); + impl SourcePropertyDeclaration { /// Create one. It’s big, try not to move it around. #[inline] @@ -4234,6 +4241,11 @@ macro_rules! longhand_properties_idents { } } +// Large pages generate tens of thousands of ComputedValues. +size_of_test!(ComputedValues, 232); +// FFI relies on this. +size_of_test!(Option>, 8); + // There are two reasons for this test to fail: // // * Your changes made a specified value type for a given property go diff --git a/components/style/rule_tree/core.rs b/components/style/rule_tree/core.rs index e4632ffa711..85584a0e224 100644 --- a/components/style/rule_tree/core.rs +++ b/components/style/rule_tree/core.rs @@ -203,9 +203,6 @@ impl RuleTree { /// where it likely did not result from a rigorous performance analysis.) const RULE_TREE_GC_INTERVAL: usize = 300; -/// Used for some size assertions. -pub const RULE_NODE_SIZE: usize = std::mem::size_of::(); - /// A node in the rule tree. struct RuleNode { /// The root node. Only the root has no root pointer, for obvious reasons. @@ -768,3 +765,8 @@ impl hash::Hash for StrongRuleNode { (&*self.p as *const RuleNode).hash(state) } } + +// Large pages generate thousands of RuleNode objects. +size_of_test!(RuleNode, 80); +// StrongRuleNode should be pointer-sized even inside an option. +size_of_test!(Option, 8); diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index c2339ee9907..01510e62070 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -20,7 +20,7 @@ mod map; mod source; mod unsafe_box; -pub use self::core::{RuleTree, StrongRuleNode, RULE_NODE_SIZE}; +pub use self::core::{RuleTree, StrongRuleNode}; pub use self::level::{CascadeLevel, ShadowCascadeOrder}; pub use self::source::StyleSource; diff --git a/components/style/stylist.rs b/components/style/stylist.rs index ebee4db0a79..b420e902ebc 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -3115,6 +3115,12 @@ impl Rule { } } +// The size of this is critical to performance on the bloom-basic +// microbenchmark. +// When iterating over a large Rule array, we want to be able to fast-reject +// selectors (with the inline hashes) with as few cache misses as possible. +size_of_test!(Rule, 40); + /// A function to be able to test the revalidation stuff. pub fn needs_revalidation_for_testing(s: &Selector) -> bool { let mut attribute_dependencies = Default::default(); diff --git a/components/style/values/computed/image.rs b/components/style/values/computed/image.rs index 49145b0bfd8..c12cbd1a2a6 100644 --- a/components/style/values/computed/image.rs +++ b/components/style/values/computed/image.rs @@ -31,6 +31,9 @@ pub use specified::ImageRendering; pub type Image = generic::GenericImage; +// Images should remain small, see https://github.com/servo/servo/pull/18430 +size_of_test!(Image, 16); + /// Computed values for a CSS gradient. /// pub type Gradient = generic::GenericGradient< diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index ec516b24e72..12dd5afdf86 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -39,6 +39,9 @@ use style_traits::{SpecifiedValueInfo, StyleParseErrorKind, ToCss}; pub type Image = generic::Image; +// Images should remain small, see https://github.com/servo/servo/pull/18430 +size_of_test!(Image, 16); + /// Specified values for a CSS gradient. /// pub type Gradient = generic::Gradient< diff --git a/tests/unit/style/lib.rs b/tests/unit/style/lib.rs index 519255b9209..110e2e8ed6d 100644 --- a/tests/unit/style/lib.rs +++ b/tests/unit/style/lib.rs @@ -17,7 +17,6 @@ extern crate servo_arc; extern crate servo_atoms; extern crate servo_url; #[macro_use] -extern crate size_of_test; extern crate style; extern crate style_traits; extern crate test;