Skip to content

Commit

Permalink
Auto merge of #14353 - emilio:fix-bloom, r=SimonSapin
Browse files Browse the repository at this point in the history
Fix the bloom filter stuff.

<!-- Please describe your changes on the following line: -->

I think I got the numbers right, want to do a try run before just in case.

r? @SimonSapin

<!-- 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/14353)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Nov 28, 2016
2 parents 555aa40 + de1a3d8 commit 2289ad5
Show file tree
Hide file tree
Showing 11 changed files with 376 additions and 193 deletions.
38 changes: 7 additions & 31 deletions components/layout/traversal.rs
Expand Up @@ -18,9 +18,8 @@ use style::data::ElementData;
use style::dom::{StylingMode, TElement, TNode};
use style::selector_parser::RestyleDamage;
use style::servo::restyle_damage::{BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, REPAINT};
use style::traversal::{DomTraversalContext, put_thread_local_bloom_filter};
use style::traversal::{recalc_style_at, remove_from_bloom_filter};
use style::traversal::take_thread_local_bloom_filter;
use style::traversal::{DomTraversalContext, recalc_style_at, remove_from_bloom_filter};
use style::traversal::PerLevelTraversalData;
use util::opts;
use wrapper::{GetRawData, LayoutNodeHelpers, LayoutNodeLayoutData};

Expand Down Expand Up @@ -74,37 +73,14 @@ impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc>
}
}

fn process_preorder(&self, node: N) {
fn process_preorder(&self, node: N, data: &mut PerLevelTraversalData) {
// FIXME(pcwalton): Stop allocating here. Ideally this should just be
// done by the HTML parser.
node.initialize_data();

if node.is_text_node() {
// FIXME(bholley): Stop doing this silly work to maintain broken bloom filter
// invariants.
//
// Longer version: The bloom filter is entirely busted for parallel traversal. Because
// parallel traversal is breadth-first, each sibling rejects the bloom filter set up
// by the previous sibling (which is valid for children, not siblings) and recreates
// it. Similarly, the fixup performed in the bottom-up traversal is useless, because
// threads perform flow construction up the parent chain until they find a parent with
// other unprocessed children, at which point they bail to the work queue and find a
// different node.
//
// Nevertheless, the remove_from_bloom_filter call at the end of flow construction
// asserts that the bloom filter is valid for the current node. This breaks when we
// stop calling recalc_style_at for text nodes, because the recursive chain of
// construct_flows_at calls is no longer necessarily rooted in a call that sets up the
// thread-local bloom filter for the leaf node.
//
// The bloom filter stuff is all going to be rewritten, so we just hackily duplicate
// the bloom filter manipulation from recalc_style_at to maintain invariants.
let parent = node.parent_node().unwrap().as_element();
let bf = take_thread_local_bloom_filter(parent, self.root, self.context.shared_context());
put_thread_local_bloom_filter(bf, &node.to_unsafe(), self.context.shared_context());
} else {
if !node.is_text_node() {
let el = node.as_element().unwrap();
recalc_style_at::<_, _, Self>(&self.context, self.root, el);
recalc_style_at::<_, _, Self>(&self.context, data, el);
}
}

Expand Down Expand Up @@ -174,9 +150,9 @@ fn construct_flows_at<'a, N: LayoutNode>(context: &'a LayoutContext<'a>, root: O
if let Some(el) = node.as_element() {
el.mutate_data().unwrap().persist();
unsafe { el.unset_dirty_descendants(); }
}

remove_from_bloom_filter(context, root, node);
remove_from_bloom_filter(context, root, el);
}
}

/// The bubble-inline-sizes traversal, the first part of layout computation. This computes
Expand Down
3 changes: 2 additions & 1 deletion components/layout_thread/lib.rs
Expand Up @@ -1131,6 +1131,7 @@ impl LayoutThread {
viewport_size_changed,
data.reflow_info.goal);

let dom_depth = Some(0); // This is always the root node.
if element.styling_mode() != StylingMode::Stop {
// Recalculate CSS styles and rebuild flows and fragments.
profile(time::ProfilerCategory::LayoutStyleRecalc,
Expand All @@ -1145,7 +1146,7 @@ impl LayoutThread {
}
Some(ref mut traversal) => {
parallel::traverse_dom::<ServoLayoutNode, RecalcStyleAndConstructFlows>(
element.as_node(), &shared_layout_context, traversal);
element.as_node(), dom_depth, &shared_layout_context, traversal);
}
}
});
Expand Down
8 changes: 0 additions & 8 deletions components/script/layout_wrapper.rs
Expand Up @@ -175,14 +175,6 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
unsafe { self.get_jsmanaged().opaque() }
}

fn layout_parent_element(self, reflow_root: OpaqueNode) -> Option<ServoLayoutElement<'ln>> {
if self.opaque() == reflow_root {
None
} else {
self.parent_node().and_then(|x| x.as_element())
}
}

fn debug_id(self) -> usize {
self.opaque().0
}
Expand Down
235 changes: 235 additions & 0 deletions components/style/bloom.rs
@@ -0,0 +1,235 @@
/* 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/. */

//! The style bloom filter is used as an optimization when matching deep
//! descendant selectors.

use dom::{TNode, TElement, UnsafeNode};
use matching::MatchMethods;
use selectors::bloom::BloomFilter;

pub struct StyleBloom {
/// The bloom filter per se.
filter: Box<BloomFilter>,

/// The stack of elements that this bloom filter contains. These unsafe
/// nodes are guaranteed to be elements.
///
/// Note that the use we do for them is safe, since the data we access from
/// them is completely read-only during restyling.
elements: Vec<UnsafeNode>,

/// A monotonic counter incremented which each reflow in order to invalidate
/// the bloom filter if appropriate.
generation: u32,
}

impl StyleBloom {
pub fn new(generation: u32) -> Self {
StyleBloom {
filter: Box::new(BloomFilter::new()),
elements: vec![],
generation: generation,
}
}

pub fn filter(&self) -> &BloomFilter {
&*self.filter
}

pub fn generation(&self) -> u32 {
self.generation
}

pub fn maybe_pop<E>(&mut self, element: E)
where E: TElement + MatchMethods
{
if self.elements.last() == Some(&element.as_node().to_unsafe()) {
self.pop::<E>().unwrap();
}
}

/// Push an element to the bloom filter, knowing that it's a child of the
/// last element parent.
pub fn push<E>(&mut self, element: E)
where E: TElement + MatchMethods,
{
if cfg!(debug_assertions) {
if self.elements.is_empty() {
assert!(element.parent_element().is_none());
}
}
element.insert_into_bloom_filter(&mut *self.filter);
self.elements.push(element.as_node().to_unsafe());
}

/// Pop the last element in the bloom filter and return it.
fn pop<E>(&mut self) -> Option<E>
where E: TElement + MatchMethods,
{
let popped =
self.elements.pop().map(|unsafe_node| {
let parent = unsafe {
E::ConcreteNode::from_unsafe(&unsafe_node)
};
parent.as_element().unwrap()
});
if let Some(popped) = popped {
popped.remove_from_bloom_filter(&mut self.filter);
}

popped
}

fn clear(&mut self) {
self.filter.clear();
self.elements.clear();
}

fn rebuild<E>(&mut self, mut element: E) -> usize
where E: TElement + MatchMethods,
{
self.clear();

while let Some(parent) = element.parent_element() {
parent.insert_into_bloom_filter(&mut *self.filter);
self.elements.push(parent.as_node().to_unsafe());
element = parent;
}

// Put them in the order we expect, from root to `element`'s parent.
self.elements.reverse();
return self.elements.len();
}

/// In debug builds, asserts that all the parents of `element` are in the
/// bloom filter.
pub fn assert_complete<E>(&self, mut element: E)
where E: TElement,
{
if cfg!(debug_assertions) {
let mut checked = 0;
while let Some(parent) = element.parent_element() {
assert_eq!(parent.as_node().to_unsafe(),
self.elements[self.elements.len() - 1 - checked]);
element = parent;
checked += 1;
}
assert_eq!(checked, self.elements.len());
}
}

/// Insert the parents of an element in the bloom filter, trying to recover
/// the filter if the last element inserted doesn't match.
///
/// Gets the element depth in the dom, to make it efficient, or if not
/// provided always rebuilds the filter from scratch.
///
/// Returns the new bloom filter depth.
pub fn insert_parents_recovering<E>(&mut self,
element: E,
element_depth: Option<usize>,
generation: u32)
-> usize
where E: TElement,
{
// Easy case, we're in a different restyle, or we're empty.
if self.generation != generation || self.elements.is_empty() {
self.generation = generation;
return self.rebuild(element);
}

let parent_element = match element.parent_element() {
Some(parent) => parent,
None => {
// Yay, another easy case.
self.clear();
return 0;
}
};

let unsafe_parent = parent_element.as_node().to_unsafe();
if self.elements.last() == Some(&unsafe_parent) {
// Ta da, cache hit, we're all done.
return self.elements.len();
}

let element_depth = match element_depth {
Some(depth) => depth,
// If we don't know the depth of `element`, we'd rather don't try
// fixing up the bloom filter, since it's quadratic.
None => {
return self.rebuild(element);
}
};

// We should've early exited above.
debug_assert!(element_depth != 0,
"We should have already cleared the bloom filter");
debug_assert!(!self.elements.is_empty(),
"How! We should've just rebuilt!");

// Now the fun begins: We have the depth of the dom and the depth of the
// last element inserted in the filter, let's try to find a common
// parent.
//
// The current depth, that is, the depth of the last element inserted in
// the bloom filter, is the number of elements _minus one_, that is: if
// there's one element, it must be the root -> depth zero.
let mut current_depth = self.elements.len() - 1;

// If the filter represents an element too deep in the dom, we need to
// pop ancestors.
while current_depth >= element_depth - 1 {
self.pop::<E>().expect("Emilio is bad at math");
current_depth -= 1;
}

// Now let's try to find a common parent in the bloom filter chain,
// starting with parent_element.
let mut common_parent = parent_element;
let mut common_parent_depth = element_depth - 1;

// Let's collect the parents we are going to need to insert once we've
// found the common one.
let mut parents_to_insert = vec![];

// If the bloom filter still doesn't have enough elements, the common
// parent is up in the dom.
while common_parent_depth > current_depth {
// TODO(emilio): Seems like we could insert parents here, then
// reverse the slice.
parents_to_insert.push(common_parent);
common_parent =
common_parent.parent_element().expect("We were lied");
common_parent_depth -= 1;
}

// Now the two depths are the same.
debug_assert_eq!(common_parent_depth, current_depth);

// Happy case: The parents match, we only need to push the ancestors
// we've collected and we'll never enter in this loop.
//
// Not-so-happy case: Parent's don't match, so we need to keep going up
// until we find a common ancestor.
while *self.elements.last().unwrap() != common_parent.as_node().to_unsafe() {
parents_to_insert.push(common_parent);
common_parent =
common_parent.parent_element().expect("We were lied again?");
self.pop::<E>().unwrap();
}

// Now the parents match, so insert the stack of elements we have been
// collecting so far.
for parent in parents_to_insert.into_iter().rev() {
self.push(parent);
}

debug_assert_eq!(self.elements.len(), element_depth);

// We're done! Easy.
return self.elements.len();
}
}
8 changes: 2 additions & 6 deletions components/style/dom.rs
Expand Up @@ -122,12 +122,8 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo {
/// Converts self into an `OpaqueNode`.
fn opaque(&self) -> OpaqueNode;

fn layout_parent_element(self, reflow_root: OpaqueNode) -> Option<Self::ConcreteElement> {
if self.opaque() == reflow_root {
None
} else {
self.parent_node().and_then(|n| n.as_element())
}
fn parent_element(&self) -> Option<Self::ConcreteElement> {
self.parent_node().and_then(|n| n.as_element())
}

fn debug_id(self) -> usize;
Expand Down
6 changes: 3 additions & 3 deletions components/style/gecko/traversal.rs
Expand Up @@ -9,7 +9,7 @@ use dom::{NodeInfo, OpaqueNode, StylingMode, TElement, TNode};
use gecko::context::StandaloneStyleContext;
use gecko::wrapper::{GeckoElement, GeckoNode};
use std::mem;
use traversal::{DomTraversalContext, recalc_style_at};
use traversal::{DomTraversalContext, PerLevelTraversalData, recalc_style_at};

pub struct RecalcStyleOnly<'lc> {
context: StandaloneStyleContext<'lc>,
Expand All @@ -29,10 +29,10 @@ impl<'lc, 'ln> DomTraversalContext<GeckoNode<'ln>> for RecalcStyleOnly<'lc> {
}
}

fn process_preorder(&self, node: GeckoNode<'ln>) {
fn process_preorder(&self, node: GeckoNode<'ln>, data: &mut PerLevelTraversalData) {
if node.is_element() && (!self.context.shared_context().skip_root || node.opaque() != self.root) {
let el = node.as_element().unwrap();
recalc_style_at::<_, _, Self>(&self.context, self.root, el);
recalc_style_at::<_, _, Self>(&self.context, data, el);
}
}

Expand Down
1 change: 1 addition & 0 deletions components/style/lib.rs
Expand Up @@ -92,6 +92,7 @@ pub mod animation;
pub mod atomic_refcell;
pub mod attr;
pub mod bezier;
pub mod bloom;
pub mod cache;
pub mod cascade_info;
pub mod context;
Expand Down

0 comments on commit 2289ad5

Please sign in to comment.