Bug 1538710 - Move texture_cache cleanup to beginning of all frames r=bholley
☠☠ backed out by 2acb3832e7e5 ☠ ☠
authorDoug Thayer <dothayer@mozilla.com>
Mon, 15 Apr 2019 18:08:47 +0000
changeset 469551 afa5cc2c6032
parent 469550 ab2083ff97f4
child 469552 1a529f967061
push id35874
push userccoroiu@mozilla.com
push dateTue, 16 Apr 2019 04:04:58 +0000
treeherdermozilla-central@be3f40425b52 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1538710
milestone68.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 1538710 - Move texture_cache cleanup to beginning of all frames r=bholley ... and ensure that, if we do cleanup, we generate frames for every document. Differential Revision: https://phabricator.services.mozilla.com/D25133
gfx/wr/webrender/src/render_backend.rs
gfx/wr/webrender/src/resource_cache.rs
gfx/wr/webrender/src/texture_cache.rs
--- a/gfx/wr/webrender/src/render_backend.rs
+++ b/gfx/wr/webrender/src/render_backend.rs
@@ -858,16 +858,22 @@ impl RenderBackend {
         }
 
         while keep_going {
             profile_scope!("handle_msg");
 
             while let Ok(msg) = self.scene_rx.try_recv() {
                 match msg {
                     SceneBuilderResult::Transactions(mut txns, result_tx) => {
+                        self.resource_cache.before_frames(SystemTime::now());
+                        self.maybe_force_nop_documents(
+                            &mut frame_counter,
+                            &mut profile_counters,
+                            |document_id| txns.iter().any(|txn| txn.document_id == document_id));
+
                         for mut txn in txns.drain(..) {
                             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(
@@ -910,16 +916,17 @@ impl RenderBackend {
                                 txn.notifications.take(),
                                 txn.render_frame,
                                 txn.invalidate_rendered_frame,
                                 &mut frame_counter,
                                 &mut profile_counters,
                                 has_built_scene,
                             );
                         }
+                        self.resource_cache.after_frames();
                     },
                     SceneBuilderResult::FlushComplete(tx) => {
                         tx.send(()).ok();
                     }
                     SceneBuilderResult::ExternalEvent(evt) => {
                         self.notifier.external_event(evt);
                     }
                     SceneBuilderResult::ClearNamespace(id) => {
@@ -1269,42 +1276,82 @@ impl RenderBackend {
                     txn.request_scene_build = Some(SceneRequest {
                         view: doc.view.clone(),
                         font_instances: self.resource_cache.get_font_instances(),
                         output_pipelines: doc.output_pipelines.clone(),
                     });
                 }
             }
         } else {
+            self.resource_cache.before_frames(SystemTime::now());
+            self.maybe_force_nop_documents(
+                frame_counter,
+                profile_counters,
+                |document_id| txns.iter().any(|txn| txn.document_id == document_id));
+
             for mut txn in txns {
                 self.update_document(
                     txn.document_id,
                     txn.resource_updates.take(),
                     None,
                     txn.frame_ops.take(),
                     txn.notifications.take(),
                     txn.render_frame,
                     txn.invalidate_rendered_frame,
                     frame_counter,
                     profile_counters,
                     false
                 );                
             }
+
+            self.resource_cache.after_frames();
             return;
         }
 
         let tx = if use_high_priority {
             &self.scene_tx
         } else {
             &self.low_priority_scene_tx
         };
 
         tx.send(SceneBuilderRequest::Transactions(txns)).unwrap();
     }
 
+    /// In certain cases, resources shared by multiple documents have to run
+    /// maintenance operations, like cleaning up unused cache items. In those
+    /// cases, we are forced to build frames for all documents, however we
+    /// may not have a transaction ready for every document - this method
+    /// calls update_document with the details of a fake, nop transaction just
+    /// to force a frame build.
+    fn maybe_force_nop_documents<F>(&mut self,
+                                    frame_counter: &mut u32,
+                                    profile_counters: &mut BackendProfileCounters,
+                                    document_already_present: F) where
+        F: Fn(DocumentId) -> bool {
+        if self.resource_cache.requires_frame_build() {
+            let nop_documents : Vec<DocumentId> = self.documents.keys()
+                .cloned()
+                .filter(|key| !document_already_present(*key))
+                .collect();
+            for &document_id in &nop_documents {
+                self.update_document(
+                    document_id,
+                    Vec::default(),
+                    None,
+                    Vec::default(),
+                    Vec::default(),
+                    false,
+                    false,
+                    frame_counter,
+                    profile_counters,
+                    false);
+            }
+        }
+    }
+
     fn update_document(
         &mut self,
         document_id: DocumentId,
         resource_updates: Vec<ResourceUpdate>,
         interner_updates: Option<InternerUpdates>,
         mut frame_ops: Vec<FrameMsg>,
         mut notifications: Vec<NotificationRequest>,
         mut render_frame: bool,
@@ -1363,17 +1410,18 @@ impl RenderBackend {
         if !doc.can_render() {
             // TODO: this happens if we are building the first scene asynchronously and
             // scroll at the same time. we should keep track of the fact that we skipped
             // composition here and do it as soon as we receive the scene.
             render_frame = false;
         }
 
         // Avoid re-building the frame if the current built frame is still valid.
-        let build_frame = render_frame && !doc.frame_is_valid;
+        let build_frame = (render_frame && !doc.frame_is_valid) ||
+            self.resource_cache.requires_frame_build();
 
         // Request composite is true when we want to composite frame even when
         // there is no frame update. This happens when video frame is updated under
         // external image with NativeTexture or when platform requested to composite frame.
         if invalidate_rendered_frame {
             doc.rendered_frame_is_valid = false;
         }
 
--- a/gfx/wr/webrender/src/resource_cache.rs
+++ b/gfx/wr/webrender/src/resource_cache.rs
@@ -37,16 +37,17 @@ use std::collections::hash_map::IterMut;
 use std::collections::VecDeque;
 use std::{cmp, mem};
 use std::fmt::Debug;
 use std::hash::Hash;
 use std::os::raw::c_void;
 #[cfg(any(feature = "capture", feature = "replay"))]
 use std::path::PathBuf;
 use std::sync::{Arc, RwLock};
+use std::time::SystemTime;
 use texture_cache::{TextureCache, TextureCacheHandle, Eviction};
 use util::drain_filter;
 
 const DEFAULT_TILE_SIZE: TileSize = 512;
 
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 pub struct GlyphFetchResult {
@@ -1563,16 +1564,28 @@ impl ResourceCache {
             ImageProperties {
                 descriptor: image_template.descriptor,
                 external_image,
                 tiling: image_template.tiling,
             }
         })
     }
 
+    pub fn before_frames(&mut self, time: SystemTime) {
+        self.texture_cache.before_frames(time);
+    }
+
+    pub fn after_frames(&mut self) {
+        self.texture_cache.after_frames();
+    }
+
+    pub fn requires_frame_build(&self) -> bool {
+        self.texture_cache.requires_frame_build()
+    }
+
     pub fn begin_frame(&mut self, stamp: FrameStamp) {
         debug_assert_eq!(self.state, State::Idle);
         self.state = State::AddResources;
         self.texture_cache.begin_frame(stamp);
         self.cached_glyphs.begin_frame(&self.texture_cache, &self.cached_render_tasks, &mut self.glyph_rasterizer);
         self.cached_render_tasks.begin_frame(&mut self.texture_cache);
         self.current_frame_id = stamp.frame_id();
 
--- a/gfx/wr/webrender/src/texture_cache.rs
+++ b/gfx/wr/webrender/src/texture_cache.rs
@@ -29,16 +29,20 @@ const PICTURE_TEXTURE_ADD_SLICES: usize 
 
 /// The chosen image format for picture tiles.
 const PICTURE_TILE_FORMAT: ImageFormat = ImageFormat::BGRA8;
 
 /// The number of pixels in a region. Derived from the above.
 const TEXTURE_REGION_PIXELS: usize =
     (TEXTURE_REGION_DIMENSIONS as usize) * (TEXTURE_REGION_DIMENSIONS as usize);
 
+// The minimum number of bytes that we must be able to reclaim in order
+// to justify clearing the entire shared cache in order to shrink it.
+const RECLAIM_THRESHOLD_BYTES: usize = 5 * 1024 * 1024;
+
 /// Items in the texture cache can either be standalone textures,
 /// or a sub-rect inside the shared cache.
 #[derive(Debug)]
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 enum EntryDetails {
     Standalone,
     Picture {
@@ -507,16 +511,20 @@ pub struct TextureCache {
     /// document, then we might end up erroneously evicting items out from
     /// under that document.
     per_doc_data: FastHashMap<DocumentId, PerDocumentData>,
 
     /// The current document's data. This is moved out of per_doc_data in
     /// begin_frame and moved back in end_frame to solve borrow checker issues.
     /// We should try removing this when we require a rustc with NLL.
     doc_data: PerDocumentData,
+
+    /// This indicates that we performed a cleanup operation which requires all
+    /// documents to build a frame.
+    require_frame_build: bool,
 }
 
 impl TextureCache {
     pub fn new(
         max_texture_size: i32,
         mut max_texture_layers: usize,
         picture_tile_size: Option<DeviceIntSize>,
         initial_size: FramebufferIntSize,
@@ -576,16 +584,17 @@ impl TextureCache {
             max_texture_size,
             max_texture_layers,
             debug_flags: DebugFlags::empty(),
             next_id: CacheTextureId(2),
             pending_updates,
             now: FrameStamp::INVALID,
             per_doc_data: FastHashMap::default(),
             doc_data: PerDocumentData::new(),
+            require_frame_build: false,
         }
     }
 
     /// Creates a TextureCache and sets it up with a valid `FrameStamp`, which
     /// is useful for avoiding panics when instantiating the `TextureCache`
     /// directly from unit test code.
     #[cfg(test)]
     pub fn new_for_testing(max_texture_size: i32, max_texture_layers: usize) -> Self {
@@ -618,16 +627,17 @@ impl TextureCache {
 
             for handle in entry_handles {
                 let entry = self.entries.free(handle);
                 entry.evict();
                 self.free(&entry);
             }
         }
         self.per_doc_data = per_doc_data;
+        self.require_frame_build = true;
     }
 
     fn clear_standalone(&mut self) {
         debug_assert!(!self.now.is_valid());
         self.clear_kind(EntryKind::Standalone);
     }
 
     fn clear_picture(&mut self) {
@@ -664,31 +674,68 @@ impl TextureCache {
                             .unwrap_or_else(|| PerDocumentData::new());
     }
 
     fn unset_doc_data(&mut self) {
         self.per_doc_data.insert(self.now.document_id(),
                                  mem::replace(&mut self.doc_data, PerDocumentData::new()));
     }
 
+    pub fn before_frames(&mut self, time: SystemTime) {
+        self.maybe_reclaim_shared_memory(time);
+    }
+
+    pub fn after_frames(&mut self) {
+        self.require_frame_build = false;
+    }
+
+    pub fn requires_frame_build(&self) -> bool {
+        return self.require_frame_build;
+    }
+
     /// Called at the beginning of each frame.
     pub fn begin_frame(&mut self, stamp: FrameStamp) {
         debug_assert!(!self.now.is_valid());
         self.now = stamp;
         self.set_doc_data();
-        self.maybe_reclaim_shared_cache_memory();
+        self.maybe_do_periodic_gc();
     }
 
-    /// Called at the beginning of each frame to periodically GC and reclaim
-    /// storage if the cache has grown too large.
-    fn maybe_reclaim_shared_cache_memory(&mut self) {
+    fn maybe_reclaim_shared_memory(&mut self, time: SystemTime) {
+        // If we've had a sufficient number of unused layers for a sufficiently
+        // long time, just blow the whole cache away to shrink it.
+        //
+        // We could do this more intelligently with a resize+blit, but that would
+        // add complexity for a rare case.
+        //
+        // This function must be called before the first begin_frame() for a group
+        // of documents, otherwise documents could end up ignoring the
+        // self.require_frame_build flag which is set if we end up calling
+        // clear_shared.
+        debug_assert!(!self.now.is_valid());
+        if self.shared_textures.empty_region_bytes() >= RECLAIM_THRESHOLD_BYTES {
+            self.reached_reclaim_threshold.get_or_insert(time);
+        } else {
+            self.reached_reclaim_threshold = None;
+        }
+        if let Some(t) = self.reached_reclaim_threshold {
+            let dur = time.duration_since(t).unwrap_or(Duration::default());
+            if dur >= Duration::from_secs(5) {
+                self.clear_shared();
+                self.reached_reclaim_threshold = None;
+            }
+        }
+    }
+
+    /// Called at the beginning of each frame to periodically GC by expiring
+    /// old shared entries. If necessary, the shared memory opened up as a
+    /// result of expiring these entries will be reclaimed before the next
+    /// group of document frames.
+    fn maybe_do_periodic_gc(&mut self) {
         debug_assert!(self.now.is_valid());
-        // The minimum number of bytes that we must be able to reclaim in order
-        // to justify clearing the entire shared cache in order to shrink it.
-        const RECLAIM_THRESHOLD_BYTES: usize = 5 * 1024 * 1024;
 
         // Normally the shared cache only gets GCed when we fail to allocate.
         // However, we also perform a periodic, conservative GC to ensure that
         // we recover unused memory in bounded time, rather than having it
         // depend on allocation patterns of subsequent content.
         let time_since_last_gc = self.now.time()
             .duration_since(self.doc_data.last_shared_cache_expiration.time())
             .unwrap_or(Duration::default());
@@ -696,38 +743,16 @@ impl TextureCache {
             self.shared_textures.size_in_bytes() >= RECLAIM_THRESHOLD_BYTES * 2;
         if do_periodic_gc {
             let threshold = EvictionThresholdBuilder::new(self.now)
                 .max_frames(1)
                 .max_time_s(10)
                 .build();
             self.maybe_expire_old_shared_entries(threshold);
         }
-
-        // If we've had a sufficient number of unused layers for a sufficiently
-        // long time, just blow the whole cache away to shrink it.
-        //
-        // We could do this more intelligently with a resize+blit, but that would
-        // add complexity for a rare case.
-        //
-        // This block of code is broken with multiple documents, and should be
-        // moved out into a section that runs before building any frames in a
-        // group of documents.
-        if self.shared_textures.empty_region_bytes() >= RECLAIM_THRESHOLD_BYTES {
-            self.reached_reclaim_threshold.get_or_insert(self.now.time());
-        } else {
-            self.reached_reclaim_threshold = None;
-        }
-        if let Some(t) = self.reached_reclaim_threshold {
-            let dur = self.now.time().duration_since(t).unwrap_or(Duration::default());
-            if dur >= Duration::from_secs(5) {
-                self.clear_shared();
-                self.reached_reclaim_threshold = None;
-            }
-        }
     }
 
     pub fn end_frame(&mut self, texture_cache_profile: &mut TextureCacheProfileCounters) {
         debug_assert!(self.now.is_valid());
         // Expire standalone entries.
         //
         // Most of the time, standalone cache entries correspond to images whose
         // width or height is greater than the region size in the shared cache, i.e.