Bug 1441308 - Make WR caches document-aware r=bholley

This change makes the various WR caches segment their cached data by
document, so that documents' data are not evicted out from underneath them.

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

--HG--
extra : moz-landing-system : lando

gfx/wr/webrender/src/frame_builder.rs

@@ -502,7 +502,7 @@ impl FrameBuilder {
 
         render_tasks.write_task_data(device_pixel_scale);
 
-        resource_cache.end_frame();
+        resource_cache.end_frame(texture_cache_profile);
 
         Frame {
             window_size: self.window_size,

gfx/wr/webrender/src/render_backend.rs

@@ -148,12 +148,15 @@ impl ::std::ops::Sub<usize> for FrameId {
 pub struct FrameStamp {
     id: FrameId,
     time: SystemTime,
+    document_id: DocumentId,
 }
 
 impl Eq for FrameStamp {}
 
 impl PartialEq for FrameStamp {
     fn eq(&self, other: &Self) -> bool {
+        // We should not be checking equality unless the documents are the same
+        debug_assert!(self.document_id == other.document_id);
         self.id == other.id
     }
 }
@@ -175,11 +178,24 @@ impl FrameStamp {
         self.time
     }
 
+    /// Gets the DocumentId in this stamp.
+    pub fn document_id(&self) -> DocumentId {
+        self.document_id
+    }
+
+    pub fn is_valid(&self) -> bool {
+        // If any fields are their default values, the whole struct should equal INVALID
+        debug_assert!((self.time != UNIX_EPOCH && self.id != FrameId(0) && self.document_id != DocumentId::INVALID) ||
+                      *self == Self::INVALID);
+        self.document_id != DocumentId::INVALID
+    }
+
     /// Returns a FrameStamp corresponding to the first frame.
-    pub fn first() -> Self {
+    pub fn first(document_id: DocumentId) -> Self {
         FrameStamp {
             id: FrameId::first(),
             time: SystemTime::now(),
+            document_id: document_id,
         }
     }
 
@@ -193,6 +209,7 @@ impl FrameStamp {
     pub const INVALID: FrameStamp = FrameStamp {
         id: FrameId(0),
         time: UNIX_EPOCH,
+        document_id: DocumentId::INVALID,
     };
 }
 
@@ -332,6 +349,7 @@ struct Document {
 
 impl Document {
     pub fn new(
+        id: DocumentId,
         window_size: DeviceIntSize,
         layer: DocumentLayer,
         default_device_pixel_ratio: f32,
@@ -349,7 +367,7 @@ impl Document {
                 device_pixel_ratio: default_device_pixel_ratio,
             },
             clip_scroll_tree: ClipScrollTree::new(),
-            stamp: FrameStamp::first(),
+            stamp: FrameStamp::first(id),
             frame_builder: None,
             output_pipelines: FastHashSet::default(),
             hit_tester: None,
@@ -982,6 +1000,7 @@ impl RenderBackend {
             }
             ApiMsg::AddDocument(document_id, initial_size, layer) => {
                 let document = Document::new(
+                    document_id,
                     initial_size,
                     layer,
                     self.default_device_pixel_ratio,
@@ -1747,7 +1766,7 @@ impl RenderBackend {
                 removed_pipelines: Vec::new(),
                 view: view.clone(),
                 clip_scroll_tree: ClipScrollTree::new(),
-                stamp: FrameStamp::first(),
+                stamp: FrameStamp::first(id),
                 frame_builder: Some(FrameBuilder::empty()),
                 output_pipelines: FastHashSet::default(),
                 dynamic_properties: SceneProperties::new(),

gfx/wr/webrender/src/resource_cache.rs

@@ -1610,7 +1610,6 @@ impl ResourceCache {
             &mut self.texture_cache,
             render_tasks,
         );
-        self.texture_cache.end_frame(texture_cache_profile);
     }
 
     fn rasterize_missing_blob_images(&mut self) {
@@ -1767,9 +1766,10 @@ impl ResourceCache {
         }
     }
 
-    pub fn end_frame(&mut self) {
+    pub fn end_frame(&mut self, texture_cache_profile: &mut TextureCacheProfileCounters) {
         debug_assert_eq!(self.state, State::QueryResources);
         self.state = State::Idle;
+        self.texture_cache.end_frame(texture_cache_profile);
     }
 
     pub fn set_debug_flags(&mut self, flags: DebugFlags) {

gfx/wr/webrender/src/texture_cache.rs

@@ -2,14 +2,14 @@
  * 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 api::{DebugFlags, DeviceIntPoint, DeviceIntRect, DeviceIntSize, DirtyRect, ImageDirtyRect};
-use api::{ExternalImageType, ImageFormat};
-use api::ImageDescriptor;
+use api::{DebugFlags, DeviceIntPoint, DeviceIntRect, DeviceIntSize};
+use api::{DirtyRect, ImageDirtyRect, DocumentId, ExternalImageType, ImageFormat};
+use api::{IdNamespace, ImageDescriptor};
 use device::{TextureFilter, total_gpu_bytes_allocated};
 use freelist::{FreeList, FreeListHandle, UpsertResult, WeakFreeListHandle};
 use gpu_cache::{GpuCache, GpuCacheHandle};
 use gpu_types::{ImageSource, UvRectKind};
-use internal_types::{CacheTextureId, LayerIndex, TextureUpdateList, TextureUpdateSource};
+use internal_types::{CacheTextureId, FastHashMap, LayerIndex, TextureUpdateList, TextureUpdateSource};
 use internal_types::{TextureSource, TextureCacheAllocInfo, TextureCacheUpdate};
 use profiler::{ResourceProfileCounter, TextureCacheProfileCounters};
 use render_backend::{FrameId, FrameStamp};
@@ -282,7 +282,7 @@ impl SharedTextures {
 /// Lists of strong handles owned by the texture cache. There is only one strong
 /// handle for each entry, but unlimited weak handles. Consumers receive the weak
 /// handles, and `TextureCache` owns the strong handles internally.
-#[derive(Default)]
+#[derive(Default, Debug)]
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 struct EntryHandles {
@@ -410,6 +410,27 @@ impl EvictionThresholdBuilder {
     }
 }
 
+#[cfg_attr(feature = "capture", derive(Serialize))]
+#[cfg_attr(feature = "replay", derive(Deserialize))]
+pub struct PerDocumentData {
+    /// The last `FrameStamp` in which we expired the shared cache for
+    /// this document.
+    last_shared_cache_expiration: FrameStamp,
+
+    /// Strong handles for all entries that this document has allocated
+    /// from the shared FreeList.
+    handles: EntryHandles,
+}
+
+impl PerDocumentData {
+    pub fn new() -> Self {
+        PerDocumentData {
+            last_shared_cache_expiration: FrameStamp::INVALID,
+            handles: EntryHandles::default(),
+        }
+    }
+}
+
 /// General-purpose manager for images in GPU memory. This includes images,
 /// rasterized glyphs, rasterized blobs, cached render tasks, etc.
 ///
@@ -453,9 +474,6 @@ pub struct TextureCache {
     /// The current `FrameStamp`. Used for cache eviction policies.
     now: FrameStamp,
 
-    /// The last `FrameStamp` in which we expired the shared cache.
-    last_shared_cache_expiration: FrameStamp,
-
     /// The time at which we first reached the byte threshold for reclaiming
     /// cache memory. `None if we haven't reached the threshold.
     reached_reclaim_threshold: Option<SystemTime>,
@@ -463,8 +481,16 @@ pub struct TextureCache {
     /// Maintains the list of all current items in the texture cache.
     entries: FreeList<CacheEntry, CacheEntryMarker>,
 
-    /// Strong handles for all entries allocated from the above `FreeList`.
-    handles: EntryHandles,
+    /// Holds items that need to be maintained on a per-document basis. If we
+    /// modify this data for a document without also building a frame for that
+    /// 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,
 }
 
 impl TextureCache {
@@ -497,16 +523,16 @@ impl TextureCache {
 
         TextureCache {
             shared_textures: SharedTextures::new(),
+            reached_reclaim_threshold: None,
+            entries: FreeList::new(),
             max_texture_size,
             max_texture_layers,
             debug_flags: DebugFlags::empty(),
             next_id: CacheTextureId(1),
             pending_updates: TextureUpdateList::new(),
             now: FrameStamp::INVALID,
-            last_shared_cache_expiration: FrameStamp::INVALID,
-            reached_reclaim_threshold: None,
-            entries: FreeList::new(),
-            handles: EntryHandles::default(),
+            per_doc_data: FastHashMap::default(),
+            doc_data: PerDocumentData::new(),
         }
     }
 
@@ -516,7 +542,7 @@ impl TextureCache {
     #[allow(dead_code)]
     pub fn new_for_testing(max_texture_size: i32, max_texture_layers: usize) -> Self {
         let mut cache = Self::new(max_texture_size, max_texture_layers);
-        let mut now = FrameStamp::first();
+        let mut now = FrameStamp::first(DocumentId(IdNamespace(1), 1));
         now.advance();
         cache.begin_frame(now);
         cache
@@ -528,32 +554,43 @@ impl TextureCache {
 
     /// Clear all standalone textures in the cache.
     pub fn clear_standalone(&mut self) {
-        let standalone_entry_handles = mem::replace(
-            &mut self.handles.standalone,
-            Vec::new(),
-        );
+        debug_assert!(!self.now.is_valid());
+        let mut per_doc_data = mem::replace(&mut self.per_doc_data, FastHashMap::default());
+        for (&_, doc_data) in per_doc_data.iter_mut() {
+            let standalone_entry_handles = mem::replace(
+                &mut doc_data.handles.standalone,
+                Vec::new(),
+            );
 
-        for handle in standalone_entry_handles {
-            let entry = self.entries.free(handle);
-            entry.evict();
-            self.free(entry);
+            for handle in standalone_entry_handles {
+                let entry = self.entries.free(handle);
+                entry.evict();
+                self.free(entry);
+            }
         }
+        self.per_doc_data = per_doc_data;
     }
 
     /// Clear all shared textures in the cache.
     pub fn clear_shared(&mut self) {
-        let shared_entry_handles = mem::replace(
-            &mut self.handles.shared,
-            Vec::new(),
-        );
+        self.unset_doc_data();
+        let mut per_doc_data = mem::replace(&mut self.per_doc_data, FastHashMap::default());
+        for (&_, doc_data) in per_doc_data.iter_mut() {
+            let shared_entry_handles = mem::replace(
+                &mut doc_data.handles.shared,
+                Vec::new(),
+            );
 
-        for handle in shared_entry_handles {
-            let entry = self.entries.free(handle);
-            entry.evict();
-            self.free(entry);
+            for handle in shared_entry_handles {
+                let entry = self.entries.free(handle);
+                entry.evict();
+                self.free(entry);
+            }
         }
 
         self.shared_textures.clear(&mut self.pending_updates);
+        self.per_doc_data = per_doc_data;
+        self.set_doc_data();
     }
 
     /// Clear all entries in the texture cache. This is a fairly drastic
@@ -563,15 +600,30 @@ impl TextureCache {
         self.clear_shared();
     }
 
+    fn set_doc_data(&mut self) {
+        let document_id = self.now.document_id();
+        self.doc_data = self.per_doc_data
+                            .remove(&document_id)
+                            .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()));
+    }
+
     /// 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();
     }
 
     /// 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) {
+        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;
@@ -581,7 +633,7 @@ impl TextureCache {
         // 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.last_shared_cache_expiration.time())
+            .duration_since(self.doc_data.last_shared_cache_expiration.time())
             .unwrap_or(Duration::default());
         let do_periodic_gc = time_since_last_gc >= Duration::from_secs(5) &&
             self.shared_textures.size_in_bytes() >= RECLAIM_THRESHOLD_BYTES * 2;
@@ -598,6 +650,10 @@ impl TextureCache {
         //
         // 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 {
@@ -610,10 +666,10 @@ impl TextureCache {
                 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
@@ -633,6 +689,9 @@ impl TextureCache {
             .update_profile(&mut texture_cache_profile.pages_rgba8_linear);
         self.shared_textures.array_rgba8_nearest
             .update_profile(&mut texture_cache_profile.pages_rgba8_nearest);
+
+        self.unset_doc_data();
+        self.now = FrameStamp::INVALID;
     }
 
     // Request an item in the texture cache. All images that will
@@ -690,6 +749,8 @@ impl TextureCache {
         uv_rect_kind: UvRectKind,
         eviction: Eviction,
     ) {
+        debug_assert!(self.now.is_valid());
+
         // Determine if we need to allocate texture cache memory
         // for this item. We need to reallocate if any of the following
         // is true:
@@ -853,12 +914,13 @@ impl TextureCache {
     ///
     /// See `EvictionThreshold` for more details on policy.
     fn expire_old_entries(&mut self, kind: EntryKind, threshold: EvictionThreshold) {
+        debug_assert!(self.now.is_valid());
         // Iterate over the entries in reverse order, evicting the ones older than
         // the frame age threshold. Reverse order avoids iterator invalidation when
         // removing entries.
-        for i in (0..self.handles.select(kind).len()).rev() {
+        for i in (0..self.doc_data.handles.select(kind).len()).rev() {
             let evict = {
-                let entry = self.entries.get(&self.handles.select(kind)[i]);
+                let entry = self.entries.get(&self.doc_data.handles.select(kind)[i]);
                 match entry.eviction {
                     Eviction::Manual => false,
                     Eviction::Auto => threshold.should_evict(entry.last_access),
@@ -880,7 +942,7 @@ impl TextureCache {
                 }
             };
             if evict {
-                let handle = self.handles.select(kind).swap_remove(i);
+                let handle = self.doc_data.handles.select(kind).swap_remove(i);
                 let entry = self.entries.free(handle);
                 entry.evict();
                 self.free(entry);
@@ -892,12 +954,13 @@ impl TextureCache {
     ///
     /// Returns true if any entries were expired.
     fn maybe_expire_old_shared_entries(&mut self, threshold: EvictionThreshold) -> bool {
-        let old_len = self.handles.shared.len();
-        if self.last_shared_cache_expiration.frame_id() < self.now.frame_id() {
+        debug_assert!(self.now.is_valid());
+        let old_len = self.doc_data.handles.shared.len();
+        if self.doc_data.last_shared_cache_expiration.frame_id() < self.now.frame_id() {
             self.expire_old_entries(EntryKind::Shared, threshold);
-            self.last_shared_cache_expiration = self.now;
+            self.doc_data.last_shared_cache_expiration = self.now;
         }
-        self.handles.shared.len() != old_len
+        self.doc_data.handles.shared.len() != old_len
     }
 
     // Free a cache entry from the standalone list or shared cache.
@@ -1106,6 +1169,7 @@ impl TextureCache {
     /// Allocates a cache entry for the given parameters, and updates the
     /// provided handle to point to the new entry.
     fn allocate(&mut self, params: &CacheAllocParams, handle: &mut TextureCacheHandle) {
+        debug_assert!(self.now.is_valid());
         let new_cache_entry = self.allocate_cache_entry(params);
         let new_kind = new_cache_entry.details.kind();
 
@@ -1125,9 +1189,9 @@ impl TextureCache {
                     // shared to standalone or vice versa. This involves a linear
                     // search, but should be rare enough not to matter.
                     let (from, to) = if new_kind == EntryKind::Standalone {
-                        (&mut self.handles.shared, &mut self.handles.standalone)
+                        (&mut self.doc_data.handles.shared, &mut self.doc_data.handles.standalone)
                     } else {
-                        (&mut self.handles.standalone, &mut self.handles.shared)
+                        (&mut self.doc_data.handles.standalone, &mut self.doc_data.handles.shared)
                     };
                     let idx = from.iter().position(|h| h.weak() == *handle).unwrap();
                     to.push(from.remove(idx));
@@ -1136,7 +1200,7 @@ impl TextureCache {
             }
             UpsertResult::Inserted(new_handle) => {
                 *handle = new_handle.weak();
-                self.handles.select(new_kind).push(new_handle);
+                self.doc_data.handles.select(new_kind).push(new_handle);
             }
         }
     }

gfx/wr/webrender_api/src/api.rs

@@ -794,6 +794,10 @@ pub struct IdNamespace(pub u32);
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
 pub struct DocumentId(pub IdNamespace, pub u32);
 
+impl DocumentId {
+    pub const INVALID: DocumentId = DocumentId(IdNamespace(0), 0);
+}
+
 /// This type carries no valuable semantics for WR. However, it reflects the fact that
 /// clients (Servo) may generate pipelines by different semi-independent sources.
 /// These pipelines still belong to the same `IdNamespace` and the same `DocumentId`.