Skip to content

Commit

Permalink
Auto merge of #16014 - servo:style-ref, r=emilio
Browse files Browse the repository at this point in the history
Per-process lock for CSSOM objects

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

Before this PR, every object reflected in CSSOM is in `Arc<RwLock<_>>` to enable safe (synchronized) mutable aliasing. Acquiring all these locks has significant cost during selector matching:

* https://bugzilla.mozilla.org/show_bug.cgi?id=1311469
* https://bugzilla.mozilla.org/show_bug.cgi?id=1335941
* https://bugzilla.mozilla.org/show_bug.cgi?id=1339703

This PR introduce a mechanism to protect many objects with the same `RwLock` that only needs to be acquired once.

In Stylo, there is one such lock per process (in a `lazy_static`), used for everything.

I non-Stylo Servo, I originally intended to have one such lock per document (for author-origin stylesheets, and one per process for user-agent and user sytlesheets since they’re shared across documents, and never mutated anyway). However I failed to have the same document-specific (or pipeline-specific) `Arc` reachable from both `Document` nodes and `LayoutThread`. Recursively following callers lead me to include this `Arc` in `UnprivilegedPipelineContent`, but that needs to be serializable. So there is a second process-wide lock.

This was previously #15998, closed accidentally.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/16014)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Mar 19, 2017
2 parents fbd561b + d507413 commit bb54f0a
Show file tree
Hide file tree
Showing 73 changed files with 1,469 additions and 912 deletions.
5 changes: 1 addition & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion components/layout/block.rs
Expand Up @@ -446,7 +446,7 @@ fn translate_including_floats(cur_b: &mut Au, delta: Au, floats: &mut Floats) {
///
/// Note that flows with position 'fixed' just form a flat list as they all
/// have the Root flow as their CB.
pub struct AbsoluteAssignBSizesTraversal<'a>(pub &'a SharedStyleContext);
pub struct AbsoluteAssignBSizesTraversal<'a>(pub &'a SharedStyleContext<'a>);

impl<'a> PreorderFlowTraversal for AbsoluteAssignBSizesTraversal<'a> {
#[inline]
Expand Down
27 changes: 14 additions & 13 deletions components/layout/construct.rs
Expand Up @@ -311,7 +311,7 @@ impl InlineFragmentsAccumulator {
/// An object that knows how to create flows.
pub struct FlowConstructor<'a, N: ThreadSafeLayoutNode> {
/// The layout context.
pub layout_context: &'a LayoutContext,
pub layout_context: &'a LayoutContext<'a>,
/// Satisfy the compiler about the unused parameters, which we use to improve the ergonomics of
/// the ensuing impl {} by removing the need to parameterize all the methods individually.
phantom2: PhantomData<N>,
Expand All @@ -320,7 +320,7 @@ pub struct FlowConstructor<'a, N: ThreadSafeLayoutNode> {
impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
FlowConstructor<'a, ConcreteThreadSafeLayoutNode> {
/// Creates a new flow constructor.
pub fn new(layout_context: &'a LayoutContext) -> Self {
pub fn new(layout_context: &'a LayoutContext<'a>) -> Self {
FlowConstructor {
layout_context: layout_context,
phantom2: PhantomData,
Expand Down Expand Up @@ -660,10 +660,9 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>

let mut style = node.style(self.style_context());
if node_is_input_or_text_area {
style = self.style_context()
.stylist
.style_for_anonymous_box(&PseudoElement::ServoInputText,
&style)
let context = self.style_context();
style = context.stylist.style_for_anonymous_box(
&context.guards, &PseudoElement::ServoInputText, &style)
}

self.create_fragments_for_node_text_content(&mut initial_fragments, node, &style)
Expand Down Expand Up @@ -1096,11 +1095,14 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
-> ConstructionResult {
let mut legalizer = Legalizer::new();

let table_style = node.style(self.style_context());
let wrapper_style = self.style_context()
.stylist
.style_for_anonymous_box(&PseudoElement::ServoTableWrapper,
&table_style);
let table_style;
let wrapper_style;
{
let context = self.style_context();
table_style = node.style(context);
wrapper_style = context.stylist.style_for_anonymous_box(
&context.guards, &PseudoElement::ServoTableWrapper, &table_style);
}
let wrapper_fragment =
Fragment::from_opaque_node_and_style(node.opaque(),
PseudoElementType::Normal,
Expand Down Expand Up @@ -2080,8 +2082,7 @@ impl Legalizer {
let reference_block = reference.as_block();
let mut new_style = reference_block.fragment.style.clone();
for pseudo in pseudos {
new_style = context.stylist.style_for_anonymous_box(pseudo,
&new_style)
new_style = context.stylist.style_for_anonymous_box(&context.guards, pseudo, &new_style)
}
let fragment = reference_block.fragment
.create_similar_anonymous_fragment(new_style,
Expand Down
8 changes: 4 additions & 4 deletions components/layout/context.rs
Expand Up @@ -75,9 +75,9 @@ pub fn heap_size_of_persistent_local_context() -> usize {
}

/// Layout information shared among all workers. This must be thread-safe.
pub struct LayoutContext {
pub struct LayoutContext<'a> {
/// Bits shared by the layout and style system.
pub style_context: SharedStyleContext,
pub style_context: SharedStyleContext<'a>,

/// The shared image cache thread.
pub image_cache_thread: Mutex<ImageCacheThread>,
Expand All @@ -95,7 +95,7 @@ pub struct LayoutContext {
pub pending_images: Option<Mutex<Vec<PendingImage>>>
}

impl Drop for LayoutContext {
impl<'a> Drop for LayoutContext<'a> {
fn drop(&mut self) {
if !thread::panicking() {
if let Some(ref pending_images) = self.pending_images {
Expand All @@ -105,7 +105,7 @@ impl Drop for LayoutContext {
}
}

impl LayoutContext {
impl<'a> LayoutContext<'a> {
#[inline(always)]
pub fn shared_context(&self) -> &SharedStyleContext {
&self.style_context
Expand Down
2 changes: 1 addition & 1 deletion components/layout/display_list_builder.rs
Expand Up @@ -121,7 +121,7 @@ fn get_cyclic<T>(arr: &[T], index: usize) -> &T {
}

pub struct DisplayListBuildState<'a> {
pub layout_context: &'a LayoutContext,
pub layout_context: &'a LayoutContext<'a>,
pub root_stacking_context: StackingContext,
pub items: HashMap<StackingContextId, Vec<DisplayItem>>,
pub stacking_context_children: HashMap<StackingContextId, Vec<StackingContext>>,
Expand Down
2 changes: 1 addition & 1 deletion components/layout/generated_content.rs
Expand Up @@ -97,7 +97,7 @@ static KATAKANA_IROHA: [char; 47] = [
/// The generated content resolution traversal.
pub struct ResolveGeneratedContent<'a> {
/// The layout context.
layout_context: &'a LayoutContext,
layout_context: &'a LayoutContext<'a>,
/// The counter representing an ordered list item.
list_item: Counter,
/// Named CSS counters.
Expand Down
24 changes: 12 additions & 12 deletions components/layout/traversal.rs
Expand Up @@ -22,20 +22,20 @@ use style::traversal::PerLevelTraversalData;
use wrapper::{GetRawData, LayoutNodeHelpers, LayoutNodeLayoutData};
use wrapper::ThreadSafeLayoutNodeHelpers;

pub struct RecalcStyleAndConstructFlows {
context: LayoutContext,
pub struct RecalcStyleAndConstructFlows<'a> {
context: LayoutContext<'a>,
driver: TraversalDriver,
}

impl RecalcStyleAndConstructFlows {
pub fn layout_context(&self) -> &LayoutContext {
impl<'a> RecalcStyleAndConstructFlows<'a> {
pub fn layout_context(&self) -> &LayoutContext<'a> {
&self.context
}
}

impl RecalcStyleAndConstructFlows {
impl<'a> RecalcStyleAndConstructFlows<'a> {
/// Creates a traversal context, taking ownership of the shared layout context.
pub fn new(context: LayoutContext, driver: TraversalDriver) -> Self {
pub fn new(context: LayoutContext<'a>, driver: TraversalDriver) -> Self {
RecalcStyleAndConstructFlows {
context: context,
driver: driver,
Expand All @@ -44,13 +44,13 @@ impl RecalcStyleAndConstructFlows {

/// Consumes this traversal context, returning ownership of the shared layout
/// context to the caller.
pub fn destroy(self) -> LayoutContext {
pub fn destroy(self) -> LayoutContext<'a> {
self.context
}
}

#[allow(unsafe_code)]
impl<E> DomTraversal<E> for RecalcStyleAndConstructFlows
impl<'a, E> DomTraversal<E> for RecalcStyleAndConstructFlows<'a>
where E: TElement,
E::ConcreteNode: LayoutNode,
{
Expand Down Expand Up @@ -152,7 +152,7 @@ fn construct_flows_at<N>(context: &LayoutContext,
/// The bubble-inline-sizes traversal, the first part of layout computation. This computes
/// preferred and intrinsic inline-sizes and bubbles them up the tree.
pub struct BubbleISizes<'a> {
pub layout_context: &'a LayoutContext,
pub layout_context: &'a LayoutContext<'a>,
}

impl<'a> PostorderFlowTraversal for BubbleISizes<'a> {
Expand All @@ -171,7 +171,7 @@ impl<'a> PostorderFlowTraversal for BubbleISizes<'a> {
/// The assign-inline-sizes traversal. In Gecko this corresponds to `Reflow`.
#[derive(Copy, Clone)]
pub struct AssignISizes<'a> {
pub layout_context: &'a LayoutContext,
pub layout_context: &'a LayoutContext<'a>,
}

impl<'a> PreorderFlowTraversal for AssignISizes<'a> {
Expand All @@ -191,7 +191,7 @@ impl<'a> PreorderFlowTraversal for AssignISizes<'a> {
/// positions. In Gecko this corresponds to `Reflow`.
#[derive(Copy, Clone)]
pub struct AssignBSizes<'a> {
pub layout_context: &'a LayoutContext,
pub layout_context: &'a LayoutContext<'a>,
}

impl<'a> PostorderFlowTraversal for AssignBSizes<'a> {
Expand Down Expand Up @@ -220,7 +220,7 @@ impl<'a> PostorderFlowTraversal for AssignBSizes<'a> {

#[derive(Copy, Clone)]
pub struct ComputeAbsolutePositions<'a> {
pub layout_context: &'a LayoutContext,
pub layout_context: &'a LayoutContext<'a>,
}

impl<'a> PreorderFlowTraversal for ComputeAbsolutePositions<'a> {
Expand Down

0 comments on commit bb54f0a

Please sign in to comment.