Bug 1903981 - Do scene-related memory deallocations on the scene builder 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

Author: nical

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(
@@ -605,6 +610,7 @@ impl SceneBuilderThread {
                 &self.config,
                 &mut doc.interners,
                 &mut doc.spatial_tree,
+                &mut self.recycler,
                 &doc.stats,
                 self.debug_flags,
             );

webrender/src/scene_building.rs

@@ -46,26 +46,27 @@ use api::{APZScrollGeneration, HasScrollLinkedEffect, Shadow, SpatialId, StickyF
 use api::{ClipMode, PrimitiveKeyKind, TransformStyle, YuvColorSpace, ColorRange, YuvData, TempFilterData};
 use api::{ReferenceTransformBinding, Rotation, FillRule, SpatialTreeItem, ReferenceFrameDescriptor};
 use api::FilterOpGraphPictureBufferId;
+use api::channel::{unbounded_channel, Receiver, Sender};
 use api::units::*;
 use crate::image_tiling::simplify_repeated_primitive;
 use crate::box_shadow::BLUR_SAMPLE_SCALE;
-use crate::clip::{ClipItemKey, ClipStore, ClipItemKeyKind, ClipIntern};
+use crate::clip::{ClipIntern, ClipItemKey, ClipItemKeyKind, ClipStore};
 use crate::clip::{ClipInternData, ClipNodeId, ClipLeafId};
 use crate::clip::{PolygonDataHandle, ClipTreeBuilder};
 use crate::segment::EdgeAaSegmentMask;
 use crate::spatial_tree::{SceneSpatialTree, SpatialNodeContainer, SpatialNodeIndex, get_external_scroll_offset};
-use crate::frame_builder::{FrameBuilderConfig};
+use crate::frame_builder::FrameBuilderConfig;
 use glyph_rasterizer::{FontInstance, SharedFontResources};
 use crate::hit_test::HitTestingScene;
 use crate::intern::Interner;
 use crate::internal_types::{FastHashMap, LayoutPrimitiveInfo, Filter, FilterGraphNode, FilterGraphOp, FilterGraphPictureReference, PlaneSplitterIndex, PipelineInstanceId};
 use crate::picture::{Picture3DContext, PictureCompositeMode, PicturePrimitive};
 use crate::picture::{BlitReason, OrderedPictureChild, PrimitiveList, SurfaceInfo, PictureFlags};
 use crate::picture_graph::PictureGraph;
-use crate::prim_store::{PrimitiveInstance};
+use crate::prim_store::{PrimitiveInstance, PrimitiveStoreStats};
 use crate::prim_store::{PrimitiveInstanceKind, NinePatchDescriptor, PrimitiveStore};
 use crate::prim_store::{InternablePrimitive, PictureIndex};
-use crate::prim_store::{PolygonKey};
+use crate::prim_store::PolygonKey;
 use crate::prim_store::backdrop::{BackdropCapture, BackdropRender};
 use crate::prim_store::borders::{ImageBorder, NormalBorderPrim};
 use crate::prim_store::gradient::{
@@ -79,7 +80,7 @@ use crate::prim_store::picture::{Picture, PictureCompositeKey, PictureKey};
 use crate::prim_store::text_run::TextRun;
 use crate::render_backend::SceneView;
 use crate::resource_cache::ImageRequest;
-use crate::scene::{Scene, ScenePipeline, BuiltScene, SceneStats, StackingContextHelpers};
+use crate::scene::{BuiltScene, Scene, ScenePipeline, SceneStats, StackingContextHelpers};
 use crate::scene_builder_thread::Interners;
 use crate::space::SpaceSnapper;
 use crate::spatial_node::{
@@ -539,6 +540,7 @@ impl<'a> SceneBuilder<'a> {
         frame_builder_config: &FrameBuilderConfig,
         interners: &mut Interners,
         spatial_tree: &mut SceneSpatialTree,
+        recycler: &mut SceneRecycler,
         stats: &SceneStats,
         debug_flags: DebugFlags,
     ) -> BuiltScene {
@@ -560,17 +562,17 @@ impl<'a> SceneBuilder<'a> {
             spatial_tree,
             fonts,
             config: *frame_builder_config,
-            id_to_index_mapper_stack: Vec::new(),
-            hit_testing_scene: HitTestingScene::new(&stats.hit_test_stats),
-            pending_shadow_items: VecDeque::new(),
-            sc_stack: Vec::new(),
-            containing_block_stack: Vec::new(),
-            raster_space_stack: vec![RasterSpace::Screen],
-            prim_store: PrimitiveStore::new(&stats.prim_store_stats),
-            clip_store: ClipStore::new(),
+            id_to_index_mapper_stack: mem::take(&mut recycler.id_to_index_mapper_stack),
+            hit_testing_scene: recycler.hit_testing_scene.take().unwrap_or_else(|| HitTestingScene::new(&stats.hit_test_stats)),
+            pending_shadow_items: mem::take(&mut recycler.pending_shadow_items),
+            sc_stack: mem::take(&mut recycler.sc_stack),
+            containing_block_stack: mem::take(&mut recycler.containing_block_stack),
+            raster_space_stack: mem::take(&mut recycler.raster_space_stack),
+            prim_store: mem::take(&mut recycler.prim_store),
+            clip_store: mem::take(&mut recycler.clip_store),
             interners,
             external_scroll_mapper: ScrollOffsetMapper::new(),
-            iframe_size: Vec::new(),
+            iframe_size: mem::take(&mut recycler.iframe_size),
             root_iframe_clip: None,
             quality_settings: view.quality_settings,
             tile_cache_builder: TileCacheBuilder::new(
@@ -579,14 +581,32 @@ impl<'a> SceneBuilder<'a> {
                 debug_flags,
             ),
             snap_to_device,
-            picture_graph: PictureGraph::new(),
+            picture_graph: mem::take(&mut recycler.picture_graph),
             next_plane_splitter_index: 0,
-            prim_instances: Vec::new(),
+            prim_instances: mem::take(&mut recycler.prim_instances),
             pipeline_instance_ids: FastHashMap::default(),
-            surfaces: Vec::new(),
-            clip_tree_builder: ClipTreeBuilder::new(),
+            surfaces: mem::take(&mut recycler.surfaces),
+            clip_tree_builder: recycler.clip_tree_builder.take().unwrap_or_else(|| ClipTreeBuilder::new()),
         };
 
+        // Reset
+        builder.hit_testing_scene.reset();
+        builder.prim_store.reset();
+        builder.clip_store.reset();
+        builder.picture_graph.reset();
+        builder.prim_instances.clear();
+        builder.surfaces.clear();
+        builder.sc_stack.clear();
+        builder.containing_block_stack.clear();
+        builder.id_to_index_mapper_stack.clear();
+        builder.pending_shadow_items.clear();
+        builder.iframe_size.clear();
+
+        builder.raster_space_stack.clear();
+        builder.raster_space_stack.push(RasterSpace::Screen);
+
+        builder.clip_tree_builder.begin();
+
         builder.build_all(
             root_pipeline_id,
             &root_pipeline,
@@ -617,6 +637,14 @@ impl<'a> SceneBuilder<'a> {
 
         let clip_tree = builder.clip_tree_builder.finalize();
 
+        recycler.clip_tree_builder = Some(builder.clip_tree_builder);
+        recycler.sc_stack = builder.sc_stack;
+        recycler.id_to_index_mapper_stack = builder.id_to_index_mapper_stack;
+        recycler.containing_block_stack = builder.containing_block_stack;
+        recycler.raster_space_stack = builder.raster_space_stack;
+        recycler.pending_shadow_items = builder.pending_shadow_items;
+        recycler.iframe_size = builder.iframe_size;
+
         BuiltScene {
             has_root_pipeline: scene.has_root_pipeline(),
             pipeline_epochs: scene.pipeline_epochs.clone(),
@@ -632,6 +660,7 @@ impl<'a> SceneBuilder<'a> {
             prim_instances: builder.prim_instances,
             surfaces: builder.surfaces,
             clip_tree,
+            recycler_tx: Some(recycler.tx.clone()),
         }
     }
 
@@ -4722,3 +4751,94 @@ fn read_gradient_stops(stops: ItemRange<GradientStop>) -> Vec<GradientStopKey> {
         }
     }).collect()
 }
+
+/// A helper for reusing the scene builder's memory allocations and dropping
+/// scene allocations on the scene builder thread to avoid lock contention in
+/// jemalloc. 
+pub struct SceneRecycler {
+    pub tx: Sender<BuiltScene>,
+    rx: Receiver<BuiltScene>,
+
+    // Allocations recycled from BuiltScene:
+
+    pub prim_store: PrimitiveStore,
+    pub clip_store: ClipStore,
+    pub picture_graph: PictureGraph,
+    pub prim_instances: Vec<PrimitiveInstance>,
+    pub surfaces: Vec<SurfaceInfo>,
+    pub hit_testing_scene: Option<HitTestingScene>,
+    pub clip_tree_builder: Option<ClipTreeBuilder>,
+    //Could also attempt to recycle the following:
+    //pub tile_cache_config: TileCacheConfig,
+    //pub pipeline_epochs: FastHashMap<PipelineId, Epoch>,
+    //pub tile_cache_pictures: Vec<PictureIndex>,
+
+
+    // Allocations recycled from SceneBuilder
+
+    id_to_index_mapper_stack: Vec<NodeIdToIndexMapper>,
+    sc_stack: Vec<FlattenedStackingContext>,
+    containing_block_stack: Vec<SpatialNodeIndex>,
+    raster_space_stack: Vec<RasterSpace>,
+    pending_shadow_items: VecDeque<ShadowItem>,
+    iframe_size: Vec<LayoutSize>,
+}
+
+impl SceneRecycler {
+    pub fn new() -> Self {
+        let (tx, rx) = unbounded_channel();
+        SceneRecycler {
+            tx,
+            rx,
+
+            prim_instances: Vec::new(),
+            surfaces: Vec::new(),
+            prim_store: PrimitiveStore::new(&PrimitiveStoreStats::empty()),
+            clip_store: ClipStore::new(),
+            picture_graph: PictureGraph::new(),
+            hit_testing_scene: None,
+            clip_tree_builder: None,
+
+            id_to_index_mapper_stack: Vec::new(),
+            sc_stack: Vec::new(),
+            containing_block_stack: Vec::new(),
+            raster_space_stack: Vec::new(),
+            pending_shadow_items: VecDeque::new(),
+            iframe_size: Vec::new(),
+        }
+    }
+
+    /// Do some bookkeeping of past memory allocations, retaining some of them for
+    /// reuse and dropping the rest.
+    ///
+    /// Should be called once between scene builds, ideally outside of the critical
+    /// path since deallocations can take some time.
+    #[inline(never)]
+    pub fn recycle_built_scene(&mut self) {
+        let Ok(scene) = self.rx.try_recv() else {
+            return;
+        };
+
+        self.prim_store = scene.prim_store;
+        self.clip_store = scene.clip_store;
+        // We currently retain top-level allocations but don't attempt to retain leaf
+        // allocations in the prim store and clip store. We don't have to reset it here
+        // but doing so avoids dropping the leaf allocations in the 
+        self.prim_store.reset();
+        self.clip_store.reset();
+        self.hit_testing_scene = Arc::try_unwrap(scene.hit_testing_scene).ok();
+        self.picture_graph = scene.picture_graph;
+        self.prim_instances = scene.prim_instances;
+        self.surfaces = scene.surfaces;
+        if let Some(clip_tree_builder) = &mut self.clip_tree_builder {
+            clip_tree_builder.recycle_tree(scene.clip_tree);
+        }
+
+        while let Ok(_) = self.rx.try_recv() {
+            // If for some reason more than one scene accumulated in the queue, drop
+            // the rest.
+        }
+
+        // Note: fields of the scene we don't recycle get dropped here.
+    }
+}