Bug 1903981 - Do scene-related memory deallocations on the scene buil…

…der thread. r=gw

The scene is sent back to the scene builder thread where the SceneRecycler will reuse some allocations and drop the rest. Deallocating the memory on the scene builder thread where it was allocated removes lock contention in jemalloc and as a bonus gives an opportunity to recycle some of it. This patch picks only the lowest hanging fruits when it comes to recycling allocations from the scene and the scene builder.

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

webrender/src/clip.rs

@@ -187,6 +187,20 @@ impl ClipTree {
         }
     }
 
+    pub fn reset(&mut self) {
+        self.nodes.clear();
+        self.nodes.push(ClipTreeNode {
+            handle: ClipDataHandle::INVALID,
+            children: Vec::new(),
+            parent: ClipNodeId::NONE,
+        });
+
+        self.leaves.clear();
+
+        self.clip_root_stack.clear();
+        self.clip_root_stack.push(ClipNodeId::NONE);
+    }
+
     /// Add a set of clips to the provided tree node id, reusing existing
     /// nodes in the tree where possible
     fn add_impl(
@@ -430,6 +444,24 @@ impl ClipTreeBuilder {
         }
     }
 
+    pub fn begin(&mut self) {
+        self.clip_map.clear();
+        self.clip_chain_map.clear();
+        self.clip_chains.clear();
+        self.clip_stack.clear();
+        self.clip_stack.push(ClipStackEntry {
+            clip_node_id: ClipNodeId::NONE,
+            last_clip_chain_cache: None,
+            seen_clips: FastHashSet::default(),
+        });
+        self.tree.reset();
+        self.clip_handles_buffer.clear();
+    }
+
+    pub fn recycle_tree(&mut self, tree: ClipTree) {
+        self.tree = tree;
+    }
+
     /// Define a new rect clip
     pub fn define_rect_clip(
         &mut self,
@@ -685,8 +717,15 @@ impl ClipTreeBuilder {
     }
 
     /// Finalize building and return the clip-tree
-    pub fn finalize(self) -> ClipTree {
-        self.tree
+    pub fn finalize(&mut self) -> ClipTree {
+        // Note: After this, the builder's clip tree does not hold allocations and
+        // is not in valid state. `ClipTreeBuilder::begin()` must be called before
+        // building can happen again.
+        std::mem::replace(&mut self.tree, ClipTree {
+            nodes: Vec::new(),
+            leaves: Vec::new(),
+            clip_root_stack: Vec::new(),
+        })
     }
 
     /// Get a clip node by id
@@ -1251,6 +1290,14 @@ impl ClipStore {
         }
     }
 
+    pub fn reset(&mut self) {
+        self.clip_node_instances.clear();
+        self.mask_tiles.clear();
+        self.active_clip_node_info.clear();
+        self.active_local_clip_rect = None;
+        self.active_pic_coverage_rect = PictureRect::max_rect();
+    }
+
     pub fn get_instance_from_range(
         &self,
         node_range: &ClipNodeRange,
@@ -1550,6 +1597,12 @@ impl ClipStore {
     }
 }
 
+impl Default for ClipStore {
+    fn default() -> Self {
+        ClipStore::new()
+    }
+}
+
 // The ClipItemKey is a hashable representation of the contents
 // of a clip item. It is used during interning to de-duplicate
 // clip nodes between frames and display lists. This allows quick

webrender/src/hit_test.rs

@@ -179,6 +179,11 @@ impl HitTestingScene {
         }
     }
 
+    pub fn reset(&mut self) {
+        self.clip_nodes.clear();
+        self.items.clear();
+    }
+
     /// Get stats about the current scene allocation sizes.
     pub fn get_stats(&self) -> HitTestingSceneStats {
         HitTestingSceneStats {

webrender/src/picture_graph.rs

@@ -16,6 +16,7 @@ pub struct PictureInfo {
     pub parent: Option<PictureIndex>,
 }
 
+#[derive(Default)]
 /// A graph of picture dependencies, allowing updates to be processed without recursion
 /// by building a list of passes.
 pub struct PictureGraph {
@@ -26,11 +27,13 @@ pub struct PictureGraph {
 
 impl PictureGraph {
     pub fn new() -> Self {
-        PictureGraph {
-            roots: Vec::new(),
-            pic_info: Vec::new(),
-            update_passes: Vec::new(),
-        }
+        PictureGraph::default()
+    }
+
+    pub fn reset(&mut self) {
+        self.roots.clear();
+        self.pic_info.clear();
+        self.update_passes.clear();
     }
 
     /// Add a root picture to the graph

webrender/src/prim_store/mod.rs

@@ -1471,6 +1471,14 @@ impl PrimitiveStore {
         }
     }
 
+    pub fn reset(&mut self) {
+        self.pictures.clear();
+        self.text_runs.clear();
+        self.images.clear();
+        self.color_bindings.clear();
+        self.linear_gradients.clear();
+    }
+
     pub fn get_stats(&self) -> PrimitiveStoreStats {
         PrimitiveStoreStats {
             picture_count: self.pictures.len(),
@@ -1489,6 +1497,12 @@ impl PrimitiveStore {
     }
 }
 
+impl Default for PrimitiveStore {
+    fn default() -> Self {
+        PrimitiveStore::new(&PrimitiveStoreStats::empty())
+    }
+}
+
 /// Trait for primitives that are directly internable.
 /// see SceneBuilder::add_primitive<P>
 pub trait InternablePrimitive: intern::Internable<InternData = ()> + Sized {

webrender/src/render_backend.rs

@@ -667,7 +667,10 @@ impl Document {
             resource_cache,
         );
 
-        self.scene = built_scene;
+
+        let old_scene = std::mem::replace(&mut self.scene, built_scene);
+        old_scene.recycle();
+
         self.scratch.recycle(recycler);
     }
 }

webrender/src/scene.rs

@@ -5,12 +5,13 @@
 use api::{BuiltDisplayList, DisplayListWithCache, ColorF, DynamicProperties, Epoch, FontRenderMode};
 use api::{PipelineId, PropertyBinding, PropertyBindingId, PropertyValue, MixBlendMode, StackingContext};
 use api::units::*;
+use api::channel::Sender;
 use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
 use crate::render_api::MemoryReport;
 use crate::composite::CompositorKind;
 use crate::clip::{ClipStore, ClipTree};
 use crate::spatial_tree::SpatialTree;
-use crate::frame_builder::{FrameBuilderConfig};
+use crate::frame_builder::FrameBuilderConfig;
 use crate::hit_test::{HitTester, HitTestingScene, HitTestingSceneStats};
 use crate::internal_types::FastHashMap;
 use crate::picture::SurfaceInfo;
@@ -284,6 +285,12 @@ pub struct BuiltScene {
     pub prim_instances: Vec<PrimitiveInstance>,
     pub surfaces: Vec<SurfaceInfo>,
     pub clip_tree: ClipTree,
+
+    /// Deallocating memory outside of the thread that allocated it causes lock
+    /// contention in jemalloc. To avoid this we send the built scene back to
+    /// the scene builder thread when we don't need it anymore, and in the process,
+    /// also reuse some allocations.
+    pub recycler_tx: Option<Sender<BuiltScene>>,
 }
 
 impl BuiltScene {
@@ -302,6 +309,7 @@ impl BuiltScene {
             prim_instances: Vec::new(),
             surfaces: Vec::new(),
             clip_tree: ClipTree::new(),
+            recycler_tx: None,
             config: FrameBuilderConfig {
                 default_font_render_mode: FontRenderMode::Mono,
                 dual_source_blending_is_supported: false,
@@ -326,6 +334,14 @@ impl BuiltScene {
         }
     }
 
+    /// Send the scene back to the scene builder thread so that recycling/deallocations
+    /// can happen there.
+    pub fn recycle(mut self) {
+        if let Some(tx) = self.recycler_tx.take() {
+            let _ = tx.send(self);
+        }
+    }
+
     /// Get the memory usage statistics to pre-allocate for the next scene.
     pub fn get_stats(&self) -> SceneStats {
         SceneStats {

webrender/src/scene_builder_thread.rs

@@ -13,7 +13,7 @@ use crate::box_shadow::BoxShadow;
 #[cfg(feature = "capture")]
 use crate::capture::CaptureConfig;
 use crate::frame_builder::FrameBuilderConfig;
-use crate::scene_building::SceneBuilder;
+use crate::scene_building::{SceneBuilder, SceneRecycler};
 use crate::clip::{ClipIntern, PolygonIntern};
 use crate::filterdata::FilterDataIntern;
 use glyph_rasterizer::SharedFontResources;
@@ -30,7 +30,7 @@ use crate::prim_store::text_run::TextRun;
 use crate::profiler::{self, TransactionProfile};
 use crate::render_backend::SceneView;
 use crate::renderer::{FullFrameStats, PipelineInfo};
-use crate::scene::{Scene, BuiltScene, SceneStats};
+use crate::scene::{BuiltScene, Scene, SceneStats};
 use crate::spatial_tree::{SceneSpatialTree, SpatialTreeUpdates};
 use crate::telemetry::Telemetry;
 use crate::SceneBuilderHooks;
@@ -243,6 +243,7 @@ pub struct SceneBuilderThread {
     #[cfg(feature = "capture")]
     capture_config: Option<CaptureConfig>,
     debug_flags: DebugFlags,
+    recycler: SceneRecycler,
 }
 
 pub struct SceneBuilderThreadChannels {
@@ -288,6 +289,7 @@ impl SceneBuilderThread {
             #[cfg(feature = "capture")]
             capture_config: None,
             debug_flags: DebugFlags::default(),
+            recycler: SceneRecycler::new(),
         }
     }
 
@@ -326,6 +328,9 @@ impl SceneBuilderThread {
                         _ => {},
                     }
                     self.forward_built_transactions(built_txns);
+
+                    // Now that we off the critical path, do some memory bookkeeping.
+                    self.recycler.recycle_built_scene();
                 }
                 Ok(SceneBuilderRequest::AddDocument(document_id, initial_size)) => {
                     let old = self.documents.insert(document_id, Document::new(
@@ -440,6 +445,7 @@ impl SceneBuilderThread {
                     &self.config,
                     &mut item.interners,
                     &mut item.spatial_tree,
+                    &mut self.recycler,
                     &SceneStats::empty(),
                     self.debug_flags,
                 ));
@@ -605,6 +611,7 @@ impl SceneBuilderThread {
                 &self.config,
                 &mut doc.interners,
                 &mut doc.spatial_tree,
+                &mut self.recycler,
                 &doc.stats,
                 self.debug_flags,
             );