Bug 1498943 - Don't copy structs to write the same value to them. r=heycam

This makes us not allocate useless style structs when you're doing something
like resetting an already-reset property, or inheriting an already-inherited
property.

Seemed simple enough that I think we should do it. In practice we don't even
should pay an extra branch because I expect the compiler to be smart enough and
merge it with the one in the mutate() call.

Differential Revision: https://phabricator.services.mozilla.com/D8685

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2018-10-15 03:04:44 +00:00
parent f0cdf769cb
commit fae0e8e687

View File

@ -3137,7 +3137,8 @@ pub enum StyleStructRef<'a, T: 'static> {
}
impl<'a, T: 'a> StyleStructRef<'a, T>
where T: Clone,
where
T: Clone,
{
/// Ensure a mutable reference of this value exists, either cloning the
/// borrowed value, or returning the owned one.
@ -3153,6 +3154,22 @@ impl<'a, T: 'a> StyleStructRef<'a, T>
}
}
/// Whether this is pointer-equal to the struct we're going to copy the
/// value from.
///
/// This is used to avoid allocations when people write stuff like `font:
/// inherit` or such `all: initial`.
#[inline]
pub fn ptr_eq(&self, struct_to_copy_from: &T) -> bool {
match *self {
StyleStructRef::Owned(..) => false,
StyleStructRef::Borrowed(arc) => {
&**arc as *const T == struct_to_copy_from as *const T
}
StyleStructRef::Vacated => panic!("Accessed vacated style struct")
}
}
/// Extract a unique Arc from this struct, vacating it.
///
/// The vacated state is a transient one, please put the Arc back
@ -3388,6 +3405,10 @@ impl<'a> StyleBuilder<'a> {
self.flags.insert(ComputedValueFlags::INHERITS_DISPLAY);
% endif
if self.${property.style_struct.ident}.ptr_eq(inherited_struct) {
return;
}
self.${property.style_struct.ident}.mutate()
.copy_${property.ident}_from(
inherited_struct,
@ -3407,10 +3428,10 @@ impl<'a> StyleBuilder<'a> {
self.modified_reset = true;
% endif
// TODO(emilio): There's a maybe-worth it optimization here: We should
// avoid allocating a new reset struct if `reset_struct` and our struct
// is the same pointer. Would remove a bunch of stupid allocations if
// you did something like `* { all: initial }` or what not.
if self.${property.style_struct.ident}.ptr_eq(reset_struct) {
return;
}
self.${property.style_struct.ident}.mutate()
.reset_${property.ident}(
reset_struct,