Bug 1520645 - WR improved vector allocation recycler r=nical
authorDzmitry Malyshau <dmalyshau@mozilla.com>
Wed, 16 Jan 2019 22:24:01 +0000
changeset 511267 bf95c0c1f962b670505ec32b12b3a49f934b5089
parent 511266 ac8281636fba946242d7da3357948443cca6aba9
child 511268 3f880d838bb0e5cb3e318dc94acd08c8387245f7
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1520645
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1520645 - WR improved vector allocation recycler r=nical The old vector recycler had a few problems: 1. shrinking to fit would be inevitably followed by re-allocation, since it's unlikely that the consequent sizes are below the immediately followed frames 2. shrinking before clearing means that the existing contents are copied over, which is a waste The new code has more complicated logic for recycling, aimed at reducing heap allocations. It's also collecting the statistics. Differential Revision: https://phabricator.services.mozilla.com/D16496
gfx/wr/webrender/src/prim_store/mod.rs
gfx/wr/webrender/src/render_backend.rs
gfx/wr/webrender/src/storage.rs
gfx/wr/webrender/src/util.rs
--- a/gfx/wr/webrender/src/prim_store/mod.rs
+++ b/gfx/wr/webrender/src/prim_store/mod.rs
@@ -45,17 +45,17 @@ use render_task::{RenderTaskCacheKeyKind
 use renderer::{MAX_VERTEX_TEXTURE_WIDTH};
 use resource_cache::{ImageProperties, ImageRequest};
 use scene::SceneProperties;
 use segment::SegmentBuilder;
 use std::{cmp, fmt, hash, ops, u32, usize, mem};
 #[cfg(debug_assertions)]
 use std::sync::atomic::{AtomicUsize, Ordering};
 use storage;
-use util::{ScaleOffset, MatrixHelpers, recycle_vec, MaxRect};
+use util::{ScaleOffset, MatrixHelpers, MaxRect, Recycler};
 use util::{pack_as_float, project_rect, raster_rect_to_device_pixels};
 use smallvec::SmallVec;
 
 pub mod borders;
 pub mod gradient;
 pub mod image;
 pub mod line_dec;
 pub mod picture;
@@ -1576,26 +1576,26 @@ impl PrimitiveScratchBuffer {
             segment_instances: SegmentInstanceStorage::new(0),
             gradient_tiles: GradientTileStorage::new(0),
             #[cfg(feature = "debug_renderer")]
             debug_items: Vec::new(),
             prim_info: Vec::new(),
         }
     }
 
-    pub fn recycle(&mut self) {
-        recycle_vec(&mut self.clip_mask_instances);
-        recycle_vec(&mut self.prim_info);
-        self.glyph_keys.recycle();
-        self.border_cache_handles.recycle();
-        self.segments.recycle();
-        self.segment_instances.recycle();
-        self.gradient_tiles.recycle();
+    pub fn recycle(&mut self, recycler: &mut Recycler) {
+        recycler.recycle_vec(&mut self.clip_mask_instances);
+        recycler.recycle_vec(&mut self.prim_info);
+        self.glyph_keys.recycle(recycler);
+        self.border_cache_handles.recycle(recycler);
+        self.segments.recycle(recycler);
+        self.segment_instances.recycle(recycler);
+        self.gradient_tiles.recycle(recycler);
         #[cfg(feature = "debug_renderer")]
-        recycle_vec(&mut self.debug_items);
+        recycler.recycle_vec(&mut self.debug_items);
     }
 
     pub fn begin_frame(&mut self) {
         // Clear the clip mask tasks for the beginning of the frame. Append
         // a single kind representing no clip mask, at the ClipTaskIndex::INVALID
         // location.
         self.clip_mask_instances.clear();
         self.clip_mask_instances.push(ClipMaskKind::None);
--- a/gfx/wr/webrender/src/render_backend.rs
+++ b/gfx/wr/webrender/src/render_backend.rs
@@ -59,17 +59,17 @@ use std::path::PathBuf;
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::mem::replace;
 use std::sync::mpsc::{channel, Sender, Receiver};
 use std::time::{UNIX_EPOCH, SystemTime};
 use std::u32;
 #[cfg(feature = "replay")]
 use tiling::Frame;
 use time::precise_time_ns;
-use util::drain_filter;
+use util::{Recycler, drain_filter};
 
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 #[derive(Clone)]
 pub struct DocumentView {
     pub window_size: DeviceIntSize,
     pub inner_rect: DeviceIntRect,
     pub layer: DocumentLayer,
@@ -588,16 +588,17 @@ impl Document {
 
     pub fn get_scroll_node_state(&self) -> Vec<ScrollNodeState> {
         self.clip_scroll_tree.get_scroll_node_state()
     }
 
     pub fn new_async_scene_ready(
         &mut self,
         mut built_scene: BuiltScene,
+        recycler: &mut Recycler,
     ) {
         self.scene = built_scene.scene;
         self.frame_is_valid = false;
         self.hit_tester_is_valid = false;
 
         // Give the old frame builder a chance to destroy any resources.
         // Right now, all this does is build a hash map of any cached
         // surface tiles, that can be provided to the next frame builder.
@@ -610,17 +611,17 @@ impl Document {
         }
 
         // Provide any cached tiles from the previous frame builder to
         // the newly built one.
         built_scene.frame_builder.set_retained_tiles(retained_tiles);
 
         self.frame_builder = Some(built_scene.frame_builder);
 
-        self.scratch.recycle();
+        self.scratch.recycle(recycler);
 
         let old_scrolling_states = self.clip_scroll_tree.drain();
         self.clip_scroll_tree = built_scene.clip_scroll_tree;
         self.clip_scroll_tree.finalize_and_apply_pending_scroll_offsets(old_scrolling_states);
     }
 }
 
 struct DocumentOps {
@@ -672,16 +673,18 @@ pub struct RenderBackend {
     documents: FastHashMap<DocumentId, Document>,
 
     notifier: Box<RenderNotifier>,
     recorder: Option<Box<ApiRecordingReceiver>>,
     sampler: Option<Box<AsyncPropertySampler + Send>>,
     size_of_ops: Option<MallocSizeOfOps>,
     debug_flags: DebugFlags,
     namespace_alloc_by_client: bool,
+
+    recycler: Recycler,
 }
 
 impl RenderBackend {
     pub fn new(
         api_rx: MsgReceiver<ApiMsg>,
         payload_rx: Receiver<Payload>,
         result_tx: Sender<ResultMsg>,
         scene_tx: Sender<SceneBuilderRequest>,
@@ -711,16 +714,17 @@ impl RenderBackend {
             frame_config,
             documents: FastHashMap::default(),
             notifier,
             recorder,
             sampler,
             size_of_ops,
             debug_flags,
             namespace_alloc_by_client,
+            recycler: Recycler::new(),
         }
     }
 
     fn process_scene_msg(
         &mut self,
         document_id: DocumentId,
         message: SceneMsg,
         frame_counter: u32,
@@ -844,16 +848,17 @@ impl RenderBackend {
                         let has_built_scene = txn.built_scene.is_some();
                         if let Some(doc) = self.documents.get_mut(&txn.document_id) {
 
                             doc.removed_pipelines.append(&mut txn.removed_pipelines);
 
                             if let Some(mut built_scene) = txn.built_scene.take() {
                                 doc.new_async_scene_ready(
                                     built_scene,
+                                    &mut self.recycler,
                                 );
                             }
 
                             if let Some(tx) = result_tx {
                                 let (resume_tx, resume_rx) = channel();
                                 tx.send(SceneSwapResult::Complete(resume_tx)).unwrap();
                                 // Block until the post-swap hook has completed on
                                 // the scene builder thread. We need to do this before
@@ -1145,16 +1150,17 @@ impl RenderBackend {
                         ResultMsg::DebugCommand(option)
                     }
                     _ => ResultMsg::DebugCommand(option),
                 };
                 self.result_tx.send(msg).unwrap();
                 self.notifier.wake_up();
             }
             ApiMsg::ShutDown => {
+                info!("Recycling stats: {:?}", self.recycler);
                 return false;
             }
             ApiMsg::UpdateDocument(document_id, transaction_msg) => {
                 self.prepare_transaction(
                     document_id,
                     transaction_msg,
                     frame_counter,
                     profile_counters,
--- a/gfx/wr/webrender/src/storage.rs
+++ b/gfx/wr/webrender/src/storage.rs
@@ -1,14 +1,14 @@
 /* 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/. */
 
 use std::{iter::Extend, ops, marker::PhantomData, u32};
-use util::recycle_vec;
+use util::Recycler;
 
 #[derive(Debug, Hash)]
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 pub struct Index<T>(u32, PhantomData<T>);
 
 // We explicitly implement Copy + Clone instead of using #[derive(Copy, Clone)]
 // because we don't want to require that T implements Clone + Copy.
@@ -84,18 +84,18 @@ impl<T> Storage<T> {
     }
 
     pub fn push(&mut self, t: T) -> Index<T> {
         let index = self.data.len();
         self.data.push(t);
         Index(index as u32, PhantomData)
     }
 
-    pub fn recycle(&mut self) {
-        recycle_vec(&mut self.data);
+    pub fn recycle(&mut self, recycler: &mut Recycler) {
+        recycler.recycle_vec(&mut self.data);
     }
 
     pub fn extend<II: IntoIterator<Item=T>>(&mut self, iter: II) -> Range<T> {
         let start = Index::new(self.data.len());
         self.data.extend(iter);
         let end = Index::new(self.data.len());
         Range { start, end }
     }
--- a/gfx/wr/webrender/src/util.rs
+++ b/gfx/wr/webrender/src/util.rs
@@ -871,26 +871,53 @@ where
         if filter(&mut vec[i]) {
             action(vec.remove(i));
         } else {
             i += 1;
         }
     }
 }
 
-/// Clear a vector for re-use, while retaining the backing memory buffer. May shrink the buffer
-/// if it's currently much larger than was actually used.
-pub fn recycle_vec<T>(vec: &mut Vec<T>) {
-    if vec.capacity() > 2 * vec.len() {
-        // Reduce capacity of the buffer if it is a lot larger than it needs to be. This prevents
-        // a frame with exceptionally large allocations to cause subsequent frames to retain
-        // more memory than they need.
-        vec.shrink_to_fit();
+
+#[derive(Debug)]
+pub struct Recycler {
+    pub num_allocations: usize,
+}
+
+impl Recycler {
+    /// Maximum extra capacity that a recycled vector is allowed to have. If the actual capacity
+    /// is larger, we re-allocate the vector storage with lower capacity.
+    const MAX_EXTRA_CAPACITY_PERCENT: usize = 200;
+    /// Minimum extra capacity to keep when re-allocating the vector storage.
+    const MIN_EXTRA_CAPACITY_PERCENT: usize = 20;
+    /// Minimum sensible vector length to consider for re-allocation.
+    const MIN_VECTOR_LENGTH: usize = 16;
+
+    pub fn new() -> Self {
+        Recycler {
+            num_allocations: 0,
+        }
     }
-    vec.clear();
+
+    /// Clear a vector for re-use, while retaining the backing memory buffer. May shrink the buffer
+    /// if it's currently much larger than was actually used.
+    pub fn recycle_vec<T>(&mut self, vec: &mut Vec<T>) {
+        let extra_capacity = (vec.capacity() - vec.len()) * 100 / vec.len().max(Self::MIN_VECTOR_LENGTH);
+
+        if extra_capacity > Self::MAX_EXTRA_CAPACITY_PERCENT {
+            // Reduce capacity of the buffer if it is a lot larger than it needs to be. This prevents
+            // a frame with exceptionally large allocations to cause subsequent frames to retain
+            // more memory than they need.
+            //TODO: use `shrink_to` when it's stable
+            *vec = Vec::with_capacity(vec.len() + vec.len() * Self::MIN_EXTRA_CAPACITY_PERCENT / 100);
+            self.num_allocations += 1;
+        } else {
+            vec.clear();
+        }
+    }
 }
 
 /// A specialized array container for comparing equality between the current
 /// contents and the new contents, incrementally. As each item is added, the
 /// container maintains track of whether this is the same as last time items
 /// were added, or if the contents have diverged. After each reset, the memory
 /// of the vec is retained, which means that memory allocation is rare.
 #[derive(Debug)]