Auto merge of #20014 - servo:by-the-power-of-void, r=emilio

Optimise some AnimationValue methods for size 🐉🐲

This shaves 40KB off of libxul.

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

Cargo.lock

@@ -1562,6 +1562,7 @@ dependencies = [
  "smallvec 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "string_cache 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "url 1.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "webrender_api 0.57.0 (git+https://github.com/servo/webrender)",
  "xml5ever 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
@@ -2926,6 +2927,7 @@ dependencies = [
  "uluru 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "unicode-bidi 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "unicode-segmentation 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "walkdir 1.0.7 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 

components/malloc_size_of/Cargo.toml

@@ -26,3 +26,4 @@ string_cache = { version = "0.7", optional = true }
 url = { version = "1.2", optional = true }
 webrender_api = { git = "https://github.com/servo/webrender", features = ["ipc"], optional = true }
 xml5ever = { version = "0.12", optional = true }
+void = "1.0.2"

components/malloc_size_of/lib.rs

@@ -58,6 +58,7 @@ extern crate smallvec;
 extern crate string_cache;
 #[cfg(feature = "url")]
 extern crate url;
+extern crate void;
 #[cfg(feature = "webrender_api")]
 extern crate webrender_api;
 #[cfg(feature = "servo")]
@@ -67,6 +68,7 @@ use std::hash::{BuildHasher, Hash};
 use std::mem::size_of;
 use std::ops::Range;
 use std::os::raw::c_void;
+use void::Void;
 
 /// A C function that takes a pointer to a heap allocation and returns its size.
 type VoidPtrToSizeFn = unsafe extern "C" fn(ptr: *const c_void) -> usize;
@@ -746,6 +748,13 @@ impl MallocSizeOf for selectors::parser::AncestorHashes {
     }
 }
 
+impl MallocSizeOf for Void {
+    #[inline]
+    fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize {
+        void::unreachable(*self)
+    }
+}
+
 #[cfg(feature = "servo")]
 impl<Static: string_cache::StaticAtomSet> MallocSizeOf for string_cache::Atom<Static> {
     fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize {

components/style/Cargo.toml

@@ -70,6 +70,7 @@ time = "0.1"
 uluru = "0.2"
 unicode-bidi = "0.3"
 unicode-segmentation = "1.0"
+void = "1.0.2"
 
 [target.'cfg(windows)'.dependencies]
 kernel32-sys = "0.2"

components/style/lib.rs

@@ -78,6 +78,7 @@ extern crate uluru;
 extern crate unicode_bidi;
 #[allow(unused_extern_crates)]
 extern crate unicode_segmentation;
+extern crate void;
 
 #[macro_use]
 mod macros;

components/style/macros.rs

@@ -68,8 +68,8 @@ macro_rules! try_match_ident_ignore_ascii_case {
 macro_rules! define_keyword_type {
     ($name: ident, $css: expr) => {
         #[allow(missing_docs)]
-        #[derive(Animate, Clone, ComputeSquaredDistance, Copy, MallocSizeOf, PartialEq)]
+        #[derive(Animate, Clone, ComputeSquaredDistance, Copy, MallocSizeOf)]
-        #[derive(ToAnimatedZero, ToComputedValue, ToCss)]
+        #[derive(PartialEq, ToAnimatedValue, ToAnimatedZero, ToComputedValue, ToCss)]
         pub struct $name;
 
         impl fmt::Debug for $name {
@@ -86,8 +86,6 @@ macro_rules! define_keyword_type {
                 input.expect_ident_matching($css).map(|_| $name).map_err(|e| e.into())
             }
         }
-
-        impl $crate::values::animated::AnimatedValueAsComputed for $name {}
     };
 }
 

components/style/properties/data.py

@@ -229,6 +229,11 @@ class Longhand(object):
     def enabled_in_content(self):
         return self.enabled_in == "content"
 
+    def base_type(self):
+        if self.predefined_type and not self.is_vector:
+            return "::values::specified::{}".format(self.predefined_type)
+        return "longhands::{}::SpecifiedValue".format(self.ident)
+
     def specified_type(self):
         if self.predefined_type and not self.is_vector:
             ty = "::values::specified::{}".format(self.predefined_type)
@@ -281,6 +286,13 @@ class Longhand(object):
             }
         return bool(self.keyword)
 
+    def animated_type(self):
+        assert self.animatable
+        computed = "<{} as ToComputedValue>::ComputedValue".format(self.base_type())
+        if self.is_animatable_with_computed_value:
+            return computed
+        return "<{} as ToAnimatedValue>::AnimatedValue".format(computed)
+
 
 class Shorthand(object):
     def __init__(self, name, sub_properties, spec=None, servo_pref=None, gecko_pref=None,

components/style/properties/helpers/animated_properties.mako.rs

@@ -4,7 +4,10 @@
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
-<% from data import to_idl_name, SYSTEM_FONT_LONGHANDS %>
+<%
+    from data import to_idl_name, SYSTEM_FONT_LONGHANDS
+    from itertools import groupby
+%>
 
 use cssparser::Parser;
 #[cfg(feature = "gecko")] use gecko_bindings::bindings::RawServoAnimationValueMap;
@@ -24,7 +27,7 @@ use properties::{LonghandId, ShorthandId};
 use selectors::parser::SelectorParseErrorKind;
 use servo_arc::Arc;
 use smallvec::SmallVec;
-use std::cmp;
+use std::{cmp, mem, ptr};
 use std::fmt::{self, Write};
 #[cfg(feature = "gecko")] use hash::FnvHashMap;
 use style_traits::{CssWriter, ParseError, ToCss};
@@ -58,6 +61,7 @@ use values::generics::position as generic_position;
 use values::generics::svg::{SVGLength,  SvgLengthOrPercentageOrNumber, SVGPaint};
 use values::generics::svg::{SVGPaintKind, SVGStrokeDashArray, SVGOpacity};
 use values::specified::font::FontTag;
+use void::{self, Void};
 
 /// <https://drafts.csswg.org/css-transitions/#animtype-repeatable-list>
 pub trait RepeatableListAnimatable: Animate {}
@@ -343,56 +347,86 @@ unsafe impl HasSimpleFFI for AnimationValueMap {}
 /// this (is a similar path to that of PropertyDeclaration).
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "servo", derive(MallocSizeOf))]
+#[repr(u16)]
 pub enum AnimationValue {
     % for prop in data.longhands:
     % if prop.animatable:
-            /// ${prop.name}
+    /// `${prop.name}`
-            % if prop.is_animatable_with_computed_value:
+    ${prop.camel_case}(${prop.animated_type()}),
-                ${prop.camel_case}(longhands::${prop.ident}::computed_value::T),
     % else:
-                ${prop.camel_case}(<longhands::${prop.ident}::computed_value::T as ToAnimatedValue>::AnimatedValue),
+    /// `${prop.name}` (not animatable)
-            % endif
+    ${prop.camel_case}(Void),
     % endif
     % endfor
 }
 
+<%
+    animated = []
+    unanimated = []
+    for prop in data.longhands:
+        if prop.animatable:
+            animated.append(prop)
+        else:
+            unanimated.append(prop)
+%>
+
 impl AnimationValue {
     /// Returns the longhand id this animated value corresponds to.
+    #[inline]
     pub fn id(&self) -> LonghandId {
-        match *self {
+        let id = unsafe { *(self as *const _ as *const LonghandId) };
+        debug_assert_eq!(id, match *self {
             % for prop in data.longhands:
             % if prop.animatable:
             AnimationValue::${prop.camel_case}(..) => LonghandId::${prop.camel_case},
+            % else:
+            AnimationValue::${prop.camel_case}(void) => void::unreachable(void),
             % endif
             % endfor
-        }
+        });
+        id
     }
 
     /// "Uncompute" this animation value in order to be used inside the CSS
     /// cascade.
     pub fn uncompute(&self) -> PropertyDeclaration {
         use properties::longhands;
+        use self::AnimationValue::*;
+
+        use super::PropertyDeclarationVariantRepr;
+
         match *self {
-            % for prop in data.longhands:
+            <% keyfunc = lambda x: (x.base_type(), x.specified_type(), x.boxed, x.is_animatable_with_computed_value) %>
-                % if prop.animatable:
+            % for (ty, specified, boxed, computed), props in groupby(animated, key=keyfunc):
-                    AnimationValue::${prop.camel_case}(ref from) => {
+            <% props = list(props) %>
-                        PropertyDeclaration::${prop.camel_case}(
+            ${" |\n".join("{}(ref value)".format(prop.camel_case) for prop in props)} => {
-                            % if prop.boxed:
+                % if not computed:
-                            Box::new(
+                let ref value = ToAnimatedValue::from_animated_value(value.clone());
                 % endif
-                                longhands::${prop.ident}::SpecifiedValue::from_computed_value(
+                let value = ${ty}::from_computed_value(&value);
-                                % if prop.is_animatable_with_computed_value:
+                % if boxed:
-                                    from
+                let value = Box::new(value);
+                % endif
+                % if len(props) == 1:
+                PropertyDeclaration::${props[0].camel_case}(value)
                 % else:
-                                    &ToAnimatedValue::from_animated_value(from.clone())
+                unsafe {
-                                % endif
+                    let mut out = mem::uninitialized();
-                                ))
+                    ptr::write(
-                            % if prop.boxed:
+                        &mut out as *mut _ as *mut PropertyDeclarationVariantRepr<${specified}>,
-                            )
+                        PropertyDeclarationVariantRepr {
-                            % endif
+                            tag: *(self as *const _ as *const u16),
+                            value,
+                        },
+                    );
+                    out
                 }
                 % endif
+            }
             % endfor
+            ${" |\n".join("{}(void)".format(prop.camel_case) for prop in unanimated)} => {
+                void::unreachable(void)
+            }
         }
     }
 
@@ -530,65 +564,83 @@ fn animate_discrete<T: Clone>(this: &T, other: &T, procedure: Procedure) -> Resu
     }
 }
 
+#[repr(C)]
+struct AnimationValueVariantRepr<T> {
+    tag: u16,
+    value: T
+}
+
 impl Animate for AnimationValue {
     fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
-        let value = match (self, other) {
+        Ok(unsafe {
-            % for prop in data.longhands:
+            use self::AnimationValue::*;
-            % if prop.animatable:
+
-            % if prop.animation_value_type != "discrete":
+            let this_tag = *(self as *const _ as *const u16);
-            (
+            let other_tag = *(other as *const _ as *const u16);
-                &AnimationValue::${prop.camel_case}(ref this),
+            if this_tag != other_tag {
-                &AnimationValue::${prop.camel_case}(ref other),
-            ) => {
-                AnimationValue::${prop.camel_case}(
-                    this.animate(other, procedure)?,
-                )
-            },
-            % else:
-            (
-                &AnimationValue::${prop.camel_case}(ref this),
-                &AnimationValue::${prop.camel_case}(ref other),
-            ) => {
-                AnimationValue::${prop.camel_case}(
-                    animate_discrete(this, other, procedure)?
-                )
-            },
-            % endif
-            % endif
-            % endfor
-            _ => {
                 panic!("Unexpected AnimationValue::animate call");
             }
-        };
+
-        Ok(value)
+            match *self {
+                <% keyfunc = lambda x: (x.animated_type(), x.animation_value_type == "discrete") %>
+                % for (ty, discrete), props in groupby(animated, key=keyfunc):
+                ${" |\n".join("{}(ref this)".format(prop.camel_case) for prop in props)} => {
+                    let other_repr =
+                        &*(other as *const _ as *const AnimationValueVariantRepr<${ty}>);
+                    % if discrete:
+                    let value = animate_discrete(this, &other_repr.value, procedure)?;
+                    % else:
+                    let value = this.animate(&other_repr.value, procedure)?;
+                    % endif
+
+                    let mut out = mem::uninitialized();
+                    ptr::write(
+                        &mut out as *mut _ as *mut AnimationValueVariantRepr<${ty}>,
+                        AnimationValueVariantRepr {
+                            tag: this_tag,
+                            value,
+                        },
+                    );
+                    out
+                }
+                % endfor
+                ${" |\n".join("{}(void)".format(prop.camel_case) for prop in unanimated)} => {
+                    void::unreachable(void)
+                }
+            }
+        })
     }
 }
 
+<%
+    nondiscrete = []
+    for prop in animated:
+        if prop.animation_value_type != "discrete":
+            nondiscrete.append(prop)
+%>
+
 impl ComputeSquaredDistance for AnimationValue {
     fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
-        match *self {
+        unsafe {
-            % for i, prop in enumerate([p for p in data.longhands if p.animatable and p.animation_value_type == "discrete"]):
+            use self::AnimationValue::*;
-            % if i > 0:
+
-            |
+            let this_tag = *(self as *const _ as *const u16);
-            % endif
+            let other_tag = *(other as *const _ as *const u16);
-            AnimationValue::${prop.camel_case}(..)
+            if this_tag != other_tag {
-            % endfor
+                panic!("Unexpected AnimationValue::compute_squared_distance call");
-            => return Err(()),
+            }
-            _ => (),
+
+            match *self {
+                % for ty, props in groupby(nondiscrete, key=lambda x: x.animated_type()):
+                ${" |\n".join("{}(ref this)".format(prop.camel_case) for prop in props)} => {
+                    let other_repr =
+                        &*(other as *const _ as *const AnimationValueVariantRepr<${ty}>);
+
+                    this.compute_squared_distance(&other_repr.value)
                 }
-        match (self, other) {
-            % for prop in data.longhands:
-            % if prop.animatable:
-            % if prop.animation_value_type != "discrete":
-            (&AnimationValue::${prop.camel_case}(ref this), &AnimationValue::${prop.camel_case}(ref other)) => {
-                this.compute_squared_distance(other)
-            },
-            % endif
-            % endif
                 % endfor
-            _ => {
+                _ => Err(()),
-                panic!("computed values should be of the same property");
+            }
-            },
         }
     }
 }

components/style/properties/properties.mako.rs

@@ -195,14 +195,6 @@ pub mod shorthands {
     %>
 }
 
-/// A module with all the code related to animated properties.
-///
-/// This needs to be "included" by mako at least after all longhand modules,
-/// given they populate the global data.
-pub mod animated_properties {
-    <%include file="/helpers/animated_properties.mako.rs" />
-}
-
 <%
     from itertools import groupby
 
@@ -413,6 +405,14 @@ impl PropertyDeclaration {
     }
 }
 
+/// A module with all the code related to animated properties.
+///
+/// This needs to be "included" by mako at least after all longhand modules,
+/// given they populate the global data.
+pub mod animated_properties {
+    <%include file="/helpers/animated_properties.mako.rs" />
+}
+
 /// A longhand or shorthand porperty
 #[derive(Clone, Copy, Debug)]
 pub struct NonCustomPropertyId(usize);
@@ -1739,6 +1739,7 @@ impl fmt::Debug for PropertyDeclaration {
 
 impl PropertyDeclaration {
     /// Given a property declaration, return the property declaration id.
+    #[inline]
     pub fn id(&self) -> PropertyDeclarationId {
         match *self {
             PropertyDeclaration::Custom(ref declaration) => {

components/style/values/animated/mod.rs

@@ -76,9 +76,6 @@ pub trait ToAnimatedValue {
     fn from_animated_value(animated: Self::AnimatedValue) -> Self;
 }
 
-/// Marker trait for computed values with the same representation during animations.
-pub trait AnimatedValueAsComputed {}
-
 /// Returns a value similar to `self` that represents zero.
 ///
 /// This trait is derivable with `#[derive(ToAnimatedValue)]`. If a field is
@@ -243,18 +240,9 @@ where
     }
 }
 
-impl AnimatedValueAsComputed for Au {}
+macro_rules! trivial_to_animated_value {
-impl AnimatedValueAsComputed for ComputedAngle {}
+    ($ty:ty) => {
-impl AnimatedValueAsComputed for SpecifiedUrl {}
+        impl $crate::values::animated::ToAnimatedValue for $ty {
-#[cfg(feature = "servo")]
-impl AnimatedValueAsComputed for ComputedUrl {}
-impl AnimatedValueAsComputed for bool {}
-impl AnimatedValueAsComputed for f32 {}
-
-impl<T> ToAnimatedValue for T
-where
-    T: AnimatedValueAsComputed,
-{
             type AnimatedValue = Self;
 
             #[inline]
@@ -266,8 +254,18 @@ where
             fn from_animated_value(animated: Self::AnimatedValue) -> Self {
                 animated
             }
+        }
+    }
 }
 
+trivial_to_animated_value!(Au);
+trivial_to_animated_value!(ComputedAngle);
+trivial_to_animated_value!(SpecifiedUrl);
+#[cfg(feature = "servo")]
+trivial_to_animated_value!(ComputedUrl);
+trivial_to_animated_value!(bool);
+trivial_to_animated_value!(f32);
+
 impl ToAnimatedValue for ComputedNonNegativeNumber {
     type AnimatedValue = Self;