Auto merge of #15953 - bholley:size_of_stylo, r=Manishearth

Make size_of tests measure stylo and reduce the size of stylo PropertyDeclarations by 16 bytes

Right now they don't, which means that we have four properties making PropertyDeclaration 16 bytes bigger than it should be.

I'm not sure if there's a better way to get these tests to run against stylo than to hoist them into the properties file, but I couldn't figure it out. This seems good enough.

<!-- 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/15953)
<!-- Reviewable:end -->

components/style/properties/data.py

@@ -211,11 +211,13 @@ class PropertiesData(object):
 
         In this situation, the `product` value is ignored while choosing
         which shorthands and longhands to generate; and instead all properties for
-        which code exists for either servo or stylo are generated.
+        which code exists for either servo or stylo are generated. Note that we skip
+        this behavior when the style crate is being built in gecko mode, because we
+        need manual glue for such properties and we don't have it.
     """
     def __init__(self, product, testing):
         self.product = product
-        self.testing = testing
+        self.testing = testing and product != "gecko"
         self.style_structs = []
         self.current_style_struct = None
         self.longhands = []

components/style/properties/longhand/inherited_svg.mako.rs

@@ -119,16 +119,19 @@ ${helpers.single_keyword("clip-rule", "nonzero evenodd",
 
 ${helpers.predefined_type("marker-start", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
+                          boxed = product == "gecko",
                           animatable="False",
                           spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")}
 
 ${helpers.predefined_type("marker-mid", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
+                          boxed = product == "gecko",
                           animatable="False",
                           spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")}
 
 ${helpers.predefined_type("marker-end", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
+                          boxed = product == "gecko",
                           animatable="False",
                           spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")}
 

components/style/properties/longhand/list.mako.rs

@@ -38,6 +38,7 @@ ${helpers.single_keyword("list-style-type", """
     spec="https://drafts.csswg.org/css-lists/#propdef-list-style-type")}
 
 ${helpers.predefined_type("list-style-image", "UrlOrNone", "Either::Second(None_)",
+                          boxed = product == "gecko",
                           initial_specified_value="Either::Second(None_)", animatable=False,
                           spec="https://drafts.csswg.org/css-lists/#propdef-list-style-image")}
 

components/style/properties/properties.mako.rs

@@ -2387,17 +2387,62 @@ macro_rules! longhand_properties_idents {
     }
 }
 
-/// Retuns all longhands SpecifiedValue sizes. This is used in unit tests.
+/// Testing function to check the size of a PropertyDeclaration. We implement
+/// this here so that the code can be used by both servo and stylo unit tests.
+/// This is important because structs can have different sizes in stylo and
+/// servo.
 #[cfg(feature = "testing")]
-pub fn specified_value_sizes() -> Vec<(&'static str, usize, bool)> {
+pub fn test_size_of_property_declaration() {
     use std::mem::size_of;
-    let mut sizes = vec![];
 
+    let old = 48;
+    let new = size_of::<PropertyDeclaration>();
+    if new < old {
+        panic!("Your changes have decreased the stack size of PropertyDeclaration enum from {} to {}. \
+                Good work! Please update the size in components/style/properties/properties.mako.rs.",
+                old, new)
+    } else if new > old {
+        panic!("Your changes have increased the stack size of PropertyDeclaration enum from {} to {}. \
+                These enum is present in large quantities in the style, and increasing the size \
+                may negatively affect style system performance. Please consider using `boxed=\"True\"` in \
+                the longhand If you feel that the increase is necessary, update to the new size in \
+                components/style/properties/properties.mako.rs.",
+                old, new)
+    }
+}
+
+/// Testing function to check the size of all SpecifiedValues.
+#[cfg(feature = "testing")]
+pub fn test_size_of_specified_values() {
+    use std::mem::size_of;
+    let threshold = 32;
+
+    let mut longhands = vec![];
     % for property in data.longhands:
-        sizes.push(("${property.name}",
-                    size_of::<longhands::${property.ident}::SpecifiedValue>(),
-                    ${"true" if property.boxed else "false"}));
+        longhands.push(("${property.name}",
+                       size_of::<longhands::${property.ident}::SpecifiedValue>(),
+                       ${"true" if property.boxed else "false"}));
     % endfor
 
-    sizes
+    let mut failing_messages = vec![];
+
+    for specified_value in longhands {
+        if specified_value.1 > threshold && !specified_value.2 {
+            failing_messages.push(
+                format!("Your changes have increased the size of {} SpecifiedValue to {}. The threshold is \
+                        currently {}. SpecifiedValues affect size of PropertyDeclaration enum and \
+                        increasing the size may negative affect style system performance. Please consider \
+                        using `boxed=\"True\"` in this longhand.",
+                        specified_value.0, specified_value.1, threshold));
+        } else if specified_value.1 <= threshold && specified_value.2 {
+            failing_messages.push(
+                format!("Your changes have decreased the size of {} SpecifiedValue to {}. Good work! \
+                        The threshold is currently {}. Please consider removing `boxed=\"True\"` from this longhand.",
+                        specified_value.0, specified_value.1, threshold));
+        }
+    }
+
+    if !failing_messages.is_empty() {
+        panic!("{}", failing_messages.join("\n\n"));
+    }
 }

ports/geckolib/Cargo.toml

@@ -11,6 +11,7 @@ crate-type = ["staticlib", "rlib"]
 
 [features]
 bindgen = ["style/use_bindgen"]
+testing = ["style/testing"]
 
 [dependencies]
 atomic_refcell = "0.1"

python/servo/testing_commands.py

@@ -300,7 +300,7 @@ def test_stylo(self, release=False):
         release = ["--release"] if release else []
         ret = 0
         with cd(path.join("ports", "geckolib")):
-            ret = call(["cargo", "test", "-p", "stylo_tests"] + release, env=env)
+            ret = call(["cargo", "test", "-p", "stylo_tests", "--features", "testing"] + release, env=env)
         if ret != 0:
             return ret
         with cd(path.join("ports", "geckolib")):

tests/unit/style/size_of.rs

@@ -2,51 +2,12 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use std::mem::size_of;
-use style::properties::{PropertyDeclaration, specified_value_sizes};
-
 #[test]
 fn size_of_property_declaration() {
-    let old = 48;
-    let new = size_of::<PropertyDeclaration>();
-    if new < old {
-        panic!("Your changes have decreased the stack size of PropertyDeclaration enum from {} to {}. \
-                Good work! Please update the size in tests/unit/style/size_of.rs.",
-                old, new)
-    } else if new > old {
-        panic!("Your changes have increased the stack size of PropertyDeclaration enum from {} to {}. \
-                These enum is present in large quantities in the style, and increasing the size \
-                may dramatically affect our memory footprint. Please consider using `boxed=\"True\"` in \
-                the longhand If you feel that the increase is necessary, update to the new size in \
-                tests/unit/style/size_of.rs.",
-                old, new)
-    }
+    ::style::properties::test_size_of_property_declaration();
 }
 
 #[test]
 fn size_of_specified_values() {
-    let threshold = 40;
-    let longhands = specified_value_sizes();
-
-    let mut failing_messages = vec![];
-
-    for specified_value in longhands {
-        if specified_value.1 >= threshold && !specified_value.2 {
-            failing_messages.push(
-                format!("Your changes have increased the size of {} SpecifiedValue to {}. The threshold is \
-                        currently {}. SpecifiedValues are affect size of PropertyDeclaration enum and \
-                        increasing the size may dramatically affect our memory footprint. Please consider \
-                        using `boxed=\"True\"` in this longhand.",
-                        specified_value.0, specified_value.1, threshold));
-        } else if specified_value.1 < threshold && specified_value.2 {
-            failing_messages.push(
-                format!("Your changes have decreased the size of {} SpecifiedValue to {}. Good work! \
-                        The threshold is currently {}. Please consider removing `boxed=\"True\"` from this longhand.",
-                        specified_value.0, specified_value.1, threshold));
-        }
-    }
-
-    if !failing_messages.is_empty() {
-        panic!("{}", failing_messages.join("\n\n"));
-    }
+    ::style::properties::test_size_of_specified_values();
 }

tests/unit/stylo/Cargo.toml

@@ -11,6 +11,9 @@ name = "stylo_tests"
 path = "lib.rs"
 doctest = false
 
+[features]
+testing = ["style/testing"]
+
 [dependencies]
 app_units = "0.4"
 atomic_refcell = "0.1"

tests/unit/stylo/lib.rs

@@ -19,6 +19,7 @@ extern crate servo_url;
 extern crate style_traits;
 
 mod sanity_checks;
+mod size_of;
 
 #[path = "../../../ports/geckolib/stylesheet_loader.rs"]
 mod stylesheet_loader;

tests/unit/stylo/size_of.rs

@@ -0,0 +1,13 @@
+/* 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 http://mozilla.org/MPL/2.0/. */
+
+#[test]
+fn size_of_property_declaration() {
+    ::style::properties::test_size_of_property_declaration();
+}
+
+#[test]
+fn size_of_specified_values() {
+    ::style::properties::test_size_of_specified_values();
+}