style: Inline one child in the rule tree.

It is indeed the most common case according to a bit of measurement.

A non-atypical example from GitHub for example:

> Rule tree stats:
>  0 - 340
>  1 - 1403
>  2 - 28
>  3 - 8
>  4 - 2
>  6 - 1
>  7 - 3
>  8 - 2
>  12 - 2
>  14 - 1
>  41 - 1
>  45 - 1
>  67 - 1
>  68 - 1

Differential Revision: https://phabricator.services.mozilla.com/D33351
This commit is contained in:
Emilio Cobos Álvarez 2019-06-03 16:37:45 +00:00
parent a40fd0bd5d
commit 9721bd7d0d
No known key found for this signature in database
GPG key ID: E1152D0994E4BF8A

View file

@ -83,20 +83,16 @@ impl MallocSizeOf for RuleTree {
while let Some(node) = stack.pop() { while let Some(node) = stack.pop() {
n += unsafe { ops.malloc_size_of(node.ptr()) }; n += unsafe { ops.malloc_size_of(node.ptr()) };
stack.extend(unsafe { unsafe {
(*node.ptr()) (*node.ptr()).children.read().each(|c| stack.push(c.clone()));
.children }
.read()
.iter()
.map(|(_k, v)| v.clone())
});
} }
n n
} }
} }
#[derive(Debug, Eq, Hash, PartialEq)] #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
struct ChildKey(CascadeLevel, ptr::NonNull<()>); struct ChildKey(CascadeLevel, ptr::NonNull<()>);
unsafe impl Send for ChildKey {} unsafe impl Send for ChildKey {}
@ -424,7 +420,7 @@ impl RuleTree {
while let Some(node) = stack.pop() { while let Some(node) = stack.pop() {
let children = node.get().children.read(); let children = node.get().children.read();
*children_count.entry(children.len()).or_insert(0) += 1; *children_count.entry(children.len()).or_insert(0) += 1;
stack.extend(children.iter().map(|(_k, v)| v.upgrade())) children.each(|c| stack.push(c.upgrade()));
} }
trace!("Rule tree stats:"); trace!("Rule tree stats:");
@ -755,6 +751,149 @@ impl CascadeLevel {
} }
} }
/// The children of a single rule node.
///
/// We optimize the case of no kids and a single child, since they're by far the
/// most common case and it'd cause a bunch of bloat for no reason.
///
/// The children remove themselves when they go away, which means that it's ok
/// for us to store weak pointers to them.
enum RuleNodeChildren {
/// There are no kids.
Empty,
/// There's just one kid. This is an extremely common case, so we don't
/// bother allocating a map for it.
One(WeakRuleNode),
/// At least at one point in time there was more than one kid (that is to
/// say, we don't bother re-allocating if children are removed dynamically).
Map(Box<FxHashMap<ChildKey, WeakRuleNode>>),
}
impl Default for RuleNodeChildren {
fn default() -> Self {
RuleNodeChildren::Empty
}
}
impl RuleNodeChildren {
/// Executes a given function for each of the children.
fn each(&self, mut f: impl FnMut(&WeakRuleNode)) {
match *self {
RuleNodeChildren::Empty => {},
RuleNodeChildren::One(ref child) => f(child),
RuleNodeChildren::Map(ref map) => {
for (_key, kid) in map.iter() {
f(kid)
}
},
}
}
fn len(&self) -> usize {
match *self {
RuleNodeChildren::Empty => 0,
RuleNodeChildren::One(..) => 1,
RuleNodeChildren::Map(ref map) => map.len(),
}
}
fn is_empty(&self) -> bool {
self.len() == 0
}
fn get(&self, key: &ChildKey) -> Option<&WeakRuleNode> {
match *self {
RuleNodeChildren::Empty => return None,
RuleNodeChildren::One(ref kid) => {
// We're read-locked, so no need to do refcount stuff, since the
// child is only removed from the main thread, _and_ it'd need
// to write-lock us anyway.
if unsafe { (*kid.ptr()).key() } == *key {
Some(kid)
} else {
None
}
},
RuleNodeChildren::Map(ref map) => map.get(&key),
}
}
fn get_or_insert_with(
&mut self,
key: ChildKey,
get_new_child: impl FnOnce() -> StrongRuleNode,
) -> StrongRuleNode {
let existing_child_key = match *self {
RuleNodeChildren::Empty => {
let new = get_new_child();
debug_assert_eq!(new.get().key(), key);
*self = RuleNodeChildren::One(new.downgrade());
return new;
},
RuleNodeChildren::One(ref weak) => unsafe {
// We're locked necessarily, so it's fine to look at our
// weak-child without refcount-traffic.
let existing_child_key = (*weak.ptr()).key();
if existing_child_key == key {
return weak.upgrade();
}
existing_child_key
},
RuleNodeChildren::Map(ref mut map) => {
return match map.entry(key) {
hash::map::Entry::Occupied(ref occupied) => occupied.get().upgrade(),
hash::map::Entry::Vacant(vacant) => {
let new = get_new_child();
debug_assert_eq!(new.get().key(), key);
vacant.insert(new.downgrade());
new
},
};
},
};
let existing_child = match mem::replace(self, RuleNodeChildren::Empty) {
RuleNodeChildren::One(o) => o,
_ => unreachable!(),
};
// Two rule-nodes are still a not-totally-uncommon thing, so
// avoid over-allocating entries.
//
// TODO(emilio): Maybe just inline two kids too?
let mut children = Box::new(FxHashMap::with_capacity_and_hasher(2, Default::default()));
children.insert(existing_child_key, existing_child);
let new = get_new_child();
debug_assert_eq!(new.get().key(), key);
children.insert(key, new.downgrade());
*self = RuleNodeChildren::Map(children);
new
}
fn remove(&mut self, key: &ChildKey) -> Option<WeakRuleNode> {
match *self {
RuleNodeChildren::Empty => return None,
RuleNodeChildren::One(ref one) => {
if unsafe { (*one.ptr()).key() } != *key {
return None;
}
},
RuleNodeChildren::Map(ref mut multiple) => {
return multiple.remove(key);
},
}
match mem::replace(self, RuleNodeChildren::Empty) {
RuleNodeChildren::One(o) => Some(o),
_ => unreachable!(),
}
}
}
/// A node in the rule tree. /// A node in the rule tree.
pub struct RuleNode { pub struct RuleNode {
/// The root node. Only the root has no root pointer, for obvious reasons. /// The root node. Only the root has no root pointer, for obvious reasons.
@ -780,7 +919,7 @@ pub struct RuleNode {
/// The children of a given rule node. Children remove themselves from here /// The children of a given rule node. Children remove themselves from here
/// when they go away. /// when they go away.
children: RwLock<FxHashMap<ChildKey, WeakRuleNode>>, children: RwLock<RuleNodeChildren>,
/// The next item in the rule tree free list, that starts on the root node. /// The next item in the rule tree free list, that starts on the root node.
/// ///
@ -871,8 +1010,14 @@ impl RuleNode {
} }
} }
fn key(&self) -> Option<ChildKey> { fn key(&self) -> ChildKey {
Some(ChildKey(self.level, self.source.as_ref()?.key())) ChildKey(
self.level,
self.source
.as_ref()
.expect("Called key() on the root node")
.key(),
)
} }
fn is_root(&self) -> bool { fn is_root(&self) -> bool {
@ -896,7 +1041,7 @@ impl RuleNode {
); );
if let Some(parent) = self.parent.as_ref() { if let Some(parent) = self.parent.as_ref() {
let weak = parent.get().children.write().remove(&self.key().unwrap()); let weak = parent.get().children.write().remove(&self.key());
assert_eq!(weak.unwrap().ptr() as *const _, self as *const _); assert_eq!(weak.unwrap().ptr() as *const _, self as *const _);
} }
} }
@ -933,12 +1078,12 @@ impl RuleNode {
} }
let _ = write!(writer, "\n"); let _ = write!(writer, "\n");
for (_, child) in self.children.read().iter() { self.children.read().each(|child| {
child child
.upgrade() .upgrade()
.get() .get()
.dump(guards, writer, indent + INDENT_INCREMENT); .dump(guards, writer, indent + INDENT_INCREMENT);
} });
} }
} }
@ -1000,21 +1145,14 @@ impl StrongRuleNode {
return child.upgrade(); return child.upgrade();
} }
match RwLockUpgradableReadGuard::upgrade(read_guard).entry(key) { RwLockUpgradableReadGuard::upgrade(read_guard).get_or_insert_with(key, move || {
hash::map::Entry::Occupied(ref occupied) => occupied.get().upgrade(), StrongRuleNode::new(Box::new(RuleNode::new(
hash::map::Entry::Vacant(vacant) => { root,
let new_node = StrongRuleNode::new(Box::new(RuleNode::new( self.clone(),
root, source.clone(),
self.clone(), level,
source.clone(), )))
level, })
)));
vacant.insert(new_node.downgrade());
new_node
},
}
} }
/// Raw pointer to the RuleNode /// Raw pointer to the RuleNode