mirror of
https://github.com/servo/servo.git
synced 2025-08-07 06:25:32 +01:00
style: Don't copy structs to write the same value to them.
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
This commit is contained in:
parent
4e174ace3b
commit
8a47c8b2e6
1 changed files with 26 additions and 5 deletions
|
@ -3137,7 +3137,8 @@ pub enum StyleStructRef<'a, T: 'static> {
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'a, T: 'a> StyleStructRef<'a, T>
|
impl<'a, T: 'a> StyleStructRef<'a, T>
|
||||||
where T: Clone,
|
where
|
||||||
|
T: Clone,
|
||||||
{
|
{
|
||||||
/// Ensure a mutable reference of this value exists, either cloning the
|
/// Ensure a mutable reference of this value exists, either cloning the
|
||||||
/// borrowed value, or returning the owned one.
|
/// 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.
|
/// Extract a unique Arc from this struct, vacating it.
|
||||||
///
|
///
|
||||||
/// The vacated state is a transient one, please put the Arc back
|
/// 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);
|
self.flags.insert(ComputedValueFlags::INHERITS_DISPLAY);
|
||||||
% endif
|
% endif
|
||||||
|
|
||||||
|
if self.${property.style_struct.ident}.ptr_eq(inherited_struct) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
self.${property.style_struct.ident}.mutate()
|
self.${property.style_struct.ident}.mutate()
|
||||||
.copy_${property.ident}_from(
|
.copy_${property.ident}_from(
|
||||||
inherited_struct,
|
inherited_struct,
|
||||||
|
@ -3407,10 +3428,10 @@ impl<'a> StyleBuilder<'a> {
|
||||||
self.modified_reset = true;
|
self.modified_reset = true;
|
||||||
% endif
|
% endif
|
||||||
|
|
||||||
// TODO(emilio): There's a maybe-worth it optimization here: We should
|
if self.${property.style_struct.ident}.ptr_eq(reset_struct) {
|
||||||
// avoid allocating a new reset struct if `reset_struct` and our struct
|
return;
|
||||||
// is the same pointer. Would remove a bunch of stupid allocations if
|
}
|
||||||
// you did something like `* { all: initial }` or what not.
|
|
||||||
self.${property.style_struct.ident}.mutate()
|
self.${property.style_struct.ident}.mutate()
|
||||||
.reset_${property.ident}(
|
.reset_${property.ident}(
|
||||||
reset_struct,
|
reset_struct,
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue