Bug 1558926 - Part 5: Store a reference to cached display item data in DisplayItemRef
☠☠ backed out by 65b1787817bd ☠ ☠
authorMiko Mynttinen <mikokm@gmail.com>
Wed, 22 Jan 2020 22:19:49 +0000
changeset 511715 974bcab6b1bf3cea22595e23d06cd9457d302299
parent 511714 1865e6d29dcfdcc1ec7adb5a5fa8fa9df183083b
child 511716 d2931c463ea8ec934de4bef5f5bf9d4ccd5b2359
push id105981
push usermikokm@gmail.com
push dateFri, 24 Jan 2020 18:05:11 +0000
treeherderautoland@974bcab6b1bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1558926
milestone74.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 1558926 - Part 5: Store a reference to cached display item data in DisplayItemRef Depends on D60760 Differential Revision: https://phabricator.services.mozilla.com/D60761
gfx/wr/webrender/src/scene.rs
gfx/wr/webrender/src/scene_building.rs
gfx/wr/webrender_api/src/display_item.rs
gfx/wr/webrender_api/src/display_list.rs
--- a/gfx/wr/webrender/src/scene.rs
+++ b/gfx/wr/webrender/src/scene.rs
@@ -1,13 +1,13 @@
 /* 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 api::{BuiltDisplayList, ColorF, DynamicProperties, Epoch, FontRenderMode};
+use api::{BuiltDisplayList, DisplayItemCache, ColorF, DynamicProperties, Epoch, FontRenderMode};
 use api::{PipelineId, PropertyBinding, PropertyBindingId, MixBlendMode, StackingContext};
 use api::units::*;
 use crate::composite::CompositorKind;
 use crate::clip::{ClipStore, ClipDataStore};
 use crate::clip_scroll_tree::{ClipScrollTree, SpatialNodeIndex};
 use crate::frame_builder::{ChasePrimitive, FrameBuilderConfig};
 use crate::hit_test::{HitTester, HitTestingScene, HitTestingSceneStats};
 use crate::internal_types::{FastHashMap, FastHashSet};
@@ -122,29 +122,28 @@ impl SceneProperties {
     pub fn float_properties(&self) -> &FastHashMap<PropertyBindingId, f32> {
         &self.float_properties
     }
 }
 
 /// A representation of the layout within the display port for a given document or iframe.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
-#[derive(Clone)]
 pub struct ScenePipeline {
     pub pipeline_id: PipelineId,
     pub viewport_size: LayoutSize,
     pub content_size: LayoutSize,
     pub background_color: Option<ColorF>,
     pub display_list: BuiltDisplayList,
+    pub display_list_cache: DisplayItemCache,
 }
 
 /// A complete representation of the layout bundling visible pipelines together.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
-#[derive(Clone)]
 pub struct Scene {
     pub root_pipeline_id: Option<PipelineId>,
     pub pipelines: FastHashMap<PipelineId, ScenePipeline>,
     pub pipeline_epochs: FastHashMap<PipelineId, Epoch>,
 }
 
 impl Scene {
     pub fn new() -> Self {
@@ -163,22 +162,30 @@ impl Scene {
         &mut self,
         pipeline_id: PipelineId,
         epoch: Epoch,
         display_list: BuiltDisplayList,
         background_color: Option<ColorF>,
         viewport_size: LayoutSize,
         content_size: LayoutSize,
     ) {
+        let pipeline = self.pipelines.remove(&pipeline_id);
+        let mut display_list_cache = pipeline.map_or(Default::default(), |p| {
+            p.display_list_cache
+        });
+
+        display_list_cache.update(&display_list);
+
         let new_pipeline = ScenePipeline {
             pipeline_id,
             viewport_size,
             content_size,
             background_color,
             display_list,
+            display_list_cache,
         };
 
         self.pipelines.insert(pipeline_id, new_pipeline);
         self.pipeline_epochs.insert(pipeline_id, epoch);
     }
 
     pub fn remove_pipeline(&mut self, pipeline_id: PipelineId) {
         if self.root_pipeline_id == Some(pipeline_id) {
--- a/gfx/wr/webrender/src/scene_building.rs
+++ b/gfx/wr/webrender/src/scene_building.rs
@@ -444,18 +444,19 @@ impl<'a> SceneBuilder<'a> {
             /* create_tile_cache = */ false,
             ROOT_SPATIAL_NODE_INDEX,
             ClipChainId::NONE,
             RasterSpace::Screen,
             /* is_backdrop_root = */ true,
             device_pixel_scale,
         );
 
+        let cache = &root_pipeline.display_list_cache;
         builder.build_items(
-            &mut root_pipeline.display_list.iter(),
+            &mut root_pipeline.display_list.iter_with_cache(cache),
             root_pipeline.pipeline_id,
             true,
         );
 
         builder.pop_stacking_context();
 
         debug_assert!(builder.sc_stack.is_empty());
 
@@ -682,40 +683,64 @@ impl<'a> SceneBuilder<'a> {
 
     fn build_items(
         &mut self,
         traversal: &mut BuiltDisplayListIter<'a>,
         pipeline_id: PipelineId,
         apply_pipeline_clip: bool,
     ) {
         loop {
-            let subtraversal = {
-                let item = match traversal.next() {
-                    Some(item) => item,
-                    None => break,
-                };
-
-                match item.item() {
-                    DisplayItem::PopReferenceFrame |
-                    DisplayItem::PopStackingContext => return,
-                    _ => (),
+            let item = match traversal.next() {
+                Some(item) => item,
+                None => break,
+            };
+
+            let subtraversal = match item.item() {
+                DisplayItem::PushStackingContext(ref info) => {
+                    let space = self.get_space(&info.spatial_id);
+                    let mut subtraversal = item.sub_iter();
+                    self.build_stacking_context(
+                        &mut subtraversal,
+                        pipeline_id,
+                        &info.stacking_context,
+                        space,
+                        info.origin,
+                        item.filters(),
+                        &item.filter_datas(),
+                        item.filter_primitives(),
+                        info.prim_flags,
+                        apply_pipeline_clip,
+                    );
+                    Some(subtraversal)
                 }
-
-                self.build_item(
-                    item,
-                    pipeline_id,
-                    apply_pipeline_clip,
-                )
+                DisplayItem::PushReferenceFrame(ref info) => {
+                    let parent_space = self.get_space(&info.parent_spatial_id);
+                    let mut subtraversal = item.sub_iter();
+                    self.build_reference_frame(
+                        &mut subtraversal,
+                        pipeline_id,
+                        parent_space,
+                        info.origin,
+                        &info.reference_frame,
+                        apply_pipeline_clip,
+                    );
+                    Some(subtraversal)
+                }
+                DisplayItem::PopReferenceFrame |
+                DisplayItem::PopStackingContext => return,
+                _ => None,
             };
 
             // If build_item created a sub-traversal, we need `traversal` to have the
             // same state as the completed subtraversal, so we reinitialize it here.
             if let Some(mut subtraversal) = subtraversal {
                 subtraversal.merge_debug_stats_from(traversal);
                 *traversal = subtraversal;
+            } else {
+                self.build_item(item, pipeline_id, apply_pipeline_clip);
             }
         }
 
         // TODO: factor this out to be part of capture
         if cfg!(feature = "display_list_stats") {
             let stats = traversal.debug_stats();
             let total_bytes: usize = stats.iter().map(|(_, stats)| stats.num_bytes).sum();
             println!("item, total count, total bytes, % of DL bytes, bytes per item");
@@ -953,18 +978,20 @@ impl<'a> SceneBuilder<'a> {
             &content_size,
             ScrollSensitivity::ScriptAndInputEvents,
             ScrollFrameKind::PipelineRoot,
             LayoutVector2D::zero(),
         );
 
         self.rf_mapper.push_scope();
         self.iframe_depth += 1;
+
+        let cache = &pipeline.display_list_cache;
         self.build_items(
-            &mut pipeline.display_list.iter(),
+            &mut pipeline.display_list.iter_with_cache(cache),
             pipeline.pipeline_id,
             true,
         );
         self.iframe_depth -= 1;
         self.rf_mapper.pop_scope();
 
         self.pipeline_clip_chain_stack.pop();
     }
@@ -1056,20 +1083,20 @@ impl<'a> SceneBuilder<'a> {
             target_spatial_node,
             &self.clip_scroll_tree
         );
         snap_to_device.snap_rect(rect)
     }
 
     fn build_item<'b>(
         &'b mut self,
-        item: DisplayItemRef<'a, 'b>,
+        item: DisplayItemRef,
         pipeline_id: PipelineId,
         apply_pipeline_clip: bool,
-    ) -> Option<BuiltDisplayListIter<'a>> {
+    ) {
         match *item.item() {
             DisplayItem::Image(ref info) => {
                 let (layout, _, clip_and_scroll) = self.process_common_properties_with_bounds(
                     &info.common,
                     &info.bounds,
                     apply_pipeline_clip,
                 );
 
@@ -1294,46 +1321,16 @@ impl<'a> SceneBuilder<'a> {
 
                 self.add_border(
                     clip_and_scroll,
                     &layout,
                     info,
                     item.gradient_stops(),
                 );
             }
-            DisplayItem::PushStackingContext(ref info) => {
-                let space = self.get_space(&info.spatial_id);
-                let mut subtraversal = item.sub_iter();
-                self.build_stacking_context(
-                    &mut subtraversal,
-                    pipeline_id,
-                    &info.stacking_context,
-                    space,
-                    info.origin,
-                    item.filters(),
-                    item.filter_datas(),
-                    item.filter_primitives(),
-                    info.prim_flags,
-                    apply_pipeline_clip,
-                );
-                return Some(subtraversal);
-            }
-            DisplayItem::PushReferenceFrame(ref info) => {
-                let parent_space = self.get_space(&info.parent_spatial_id);
-                let mut subtraversal = item.sub_iter();
-                self.build_reference_frame(
-                    &mut subtraversal,
-                    pipeline_id,
-                    parent_space,
-                    info.origin,
-                    &info.reference_frame,
-                    apply_pipeline_clip,
-                );
-                return Some(subtraversal);
-            }
             DisplayItem::Iframe(ref info) => {
                 let space = self.get_space(&info.space_and_clip.spatial_id);
                 self.build_iframe(
                     info,
                     space,
                 );
             }
             DisplayItem::Clip(ref info) => {
@@ -1451,35 +1448,41 @@ impl<'a> SceneBuilder<'a> {
             }
 
             // Do nothing; these are dummy items for the display list parser
             DisplayItem::SetGradientStops |
             DisplayItem::SetFilterOps |
             DisplayItem::SetFilterData |
             DisplayItem::SetFilterPrimitives => {}
 
+            // Special items that are handled in the parent method
+            DisplayItem::PushStackingContext(..) |
+            DisplayItem::PushReferenceFrame(..) |
             DisplayItem::PopReferenceFrame |
             DisplayItem::PopStackingContext => {
                 unreachable!("Should have returned in parent method.")
             }
+
+            DisplayItem::ReuseItem(..) => {
+                unreachable!("Iterator logic error")
+            }
+
             DisplayItem::PushShadow(info) => {
                 let clip_and_scroll = self.get_clip_and_scroll(
                     &info.space_and_clip.clip_id,
                     &info.space_and_clip.spatial_id,
                     apply_pipeline_clip
                 );
 
                 self.push_shadow(info.shadow, clip_and_scroll, info.should_inflate);
             }
             DisplayItem::PopAllShadows => {
                 self.pop_all_shadows();
             }
         }
-
-        None
     }
 
     // Given a list of clip sources, a positioning node and
     // a parent clip chain, return a new clip chain entry.
     // If the supplied list of clip sources is empty, then
     // just return the parent clip chain id directly.
     fn build_clip_chain(
         &mut self,
--- a/gfx/wr/webrender_api/src/display_item.rs
+++ b/gfx/wr/webrender_api/src/display_item.rs
@@ -158,16 +158,18 @@ pub enum DisplayItem {
     SetFilterOps,
     SetFilterData,
     SetFilterPrimitives,
 
     // These marker items terminate a scope introduced by a previous item.
     PopReferenceFrame,
     PopStackingContext,
     PopAllShadows,
+
+    ReuseItem(ItemKey),
 }
 
 /// This is a "complete" version of the DisplayItem, with all implicit trailing
 /// arrays included, for debug serialization (captures).
 #[cfg(any(feature = "serialize", feature = "deserialize"))]
 #[cfg_attr(feature = "serialize", derive(Serialize))]
 #[cfg_attr(feature = "deserialize", derive(Deserialize))]
 pub enum DebugDisplayItem {
@@ -198,16 +200,18 @@ pub enum DebugDisplayItem {
     SetGradientStops(Vec<GradientStop>),
     SetFilterOps(Vec<FilterOp>),
     SetFilterData(FilterData),
     SetFilterPrimitives(Vec<FilterPrimitive>),
 
     PopReferenceFrame,
     PopStackingContext,
     PopAllShadows,
+
+    ReuseItem(ItemKey),
 }
 
 #[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize, PeekPoke)]
 pub struct ClipDisplayItem {
     pub id: ClipId,
     pub parent_space_and_clip: SpaceAndClipInfo,
     pub clip_rect: LayoutRect,
     pub image_mask: Option<ImageMask>,
@@ -1471,16 +1475,17 @@ impl DisplayItem {
             DisplayItem::PushStackingContext(..) => "push_stacking_context",
             DisplayItem::SetFilterOps => "set_filter_ops",
             DisplayItem::SetFilterData => "set_filter_data",
             DisplayItem::SetFilterPrimitives => "set_filter_primitives",
             DisplayItem::RadialGradient(..) => "radial_gradient",
             DisplayItem::Rectangle(..) => "rectangle",
             DisplayItem::ScrollFrame(..) => "scroll_frame",
             DisplayItem::SetGradientStops => "set_gradient_stops",
+            DisplayItem::ReuseItem(..) => "reuse_item",
             DisplayItem::StickyFrame(..) => "sticky_frame",
             DisplayItem::Text(..) => "text",
             DisplayItem::YuvImage(..) => "yuv_image",
             DisplayItem::BackdropFilter(..) => "backdrop_filter",
         }
     }
 }
 
--- a/gfx/wr/webrender_api/src/display_list.rs
+++ b/gfx/wr/webrender_api/src/display_list.rs
@@ -218,16 +218,68 @@ pub struct ItemStats {
     /// How many instances of this kind of item we deserialized
     pub total_count: usize,
     /// How many bytes we processed for this kind of item
     pub num_bytes: usize,
 }
 
 pub struct DisplayItemRef<'a: 'b, 'b> {
     iter: &'b BuiltDisplayListIter<'a>,
+    cached_item: Option<&'a CachedDisplayItem>,
+}
+
+// Some of these might just become ItemRanges
+impl<'a, 'b> DisplayItemRef<'a, 'b> {
+    fn cached_or_iter_data<T>(
+        &self,
+        data: ItemRange<'a, T>
+    ) -> ItemRange<'a, T> {
+        self.cached_item.map_or(data, |i| i.data_as_item_range())
+    }
+
+    pub fn display_list(&self) -> &BuiltDisplayList {
+        self.iter.display_list()
+    }
+
+    // Creates a new iterator where this element's iterator is, to hack around borrowck.
+    pub fn sub_iter(&self) -> BuiltDisplayListIter<'a> {
+        BuiltDisplayListIter::new(self.iter.list, self.iter.data, self.iter.cache)
+    }
+
+    pub fn item(&self) -> &di::DisplayItem {
+        self.cached_item.map_or(&self.iter.cur_item, |i| i.item())
+    }
+
+    pub fn clip_chain_items(&self) -> ItemRange<di::ClipId> {
+        self.iter.cur_clip_chain_items
+    }
+
+    pub fn complex_clip(&self) -> ItemRange<di::ComplexClipRegion> {
+        self.iter.cur_complex_clip
+    }
+
+    pub fn glyphs(&self) -> ItemRange<GlyphInstance> {
+        self.cached_or_iter_data(self.iter.cur_glyphs)
+    }
+
+    pub fn gradient_stops(&self) -> ItemRange<di::GradientStop> {
+        self.cached_or_iter_data(self.iter.cur_stops)
+    }
+
+    pub fn filters(&self) -> ItemRange<di::FilterOp> {
+        self.iter.cur_filters
+    }
+
+    pub fn filter_datas(&self) -> &Vec<TempFilterData> {
+        &self.iter.cur_filter_data
+    }
+
+    pub fn filter_primitives(&self) -> ItemRange<di::FilterPrimitive> {
+        self.iter.cur_filter_primitives
+    }
 }
 
 #[derive(PartialEq)]
 enum Peek {
     StartPeeking,
     IsPeeking,
     NotPeeking,
 }
@@ -452,17 +504,28 @@ impl<'a> BuiltDisplayListIter<'a> {
             }
             _ => { /* do nothing */ }
         }
 
         Some(self.as_ref())
     }
 
     pub fn as_ref<'b>(&'b self) -> DisplayItemRef<'a, 'b> {
-        DisplayItemRef { iter: self }
+        let cached_item = match self.cur_item {
+            di::DisplayItem::ReuseItem(key) => {
+                debug_assert!(self.cache.is_some(), "Cache marker without cache!");
+                self.cache.and_then(|c| c.get_item(key))
+            }
+            _ => None
+        };
+
+        DisplayItemRef {
+            iter: self,
+            cached_item
+        }
     }
 
     pub fn skip_current_stacking_context(&mut self) {
         let mut depth = 0;
         while let Some(item) = self.next() {
             match *item.item() {
                 di::DisplayItem::PushStackingContext(..) => depth += 1,
                 di::DisplayItem::PopStackingContext if depth == 0 => return,
@@ -512,60 +575,16 @@ impl<'a> BuiltDisplayListIter<'a> {
     fn log_item_stats(&mut self) {
         self.debug_stats.log_item(self.data, &self.cur_item);
     }
 
     #[cfg(not(feature = "display_list_stats"))]
     fn log_item_stats(&mut self) { /* no-op */ }
 }
 
-// Some of these might just become ItemRanges
-impl<'a, 'b> DisplayItemRef<'a, 'b> {
-    pub fn item(&self) -> &di::DisplayItem {
-        &self.iter.cur_item
-    }
-
-    pub fn complex_clip(&self) -> ItemRange<di::ComplexClipRegion> {
-        self.iter.cur_complex_clip
-    }
-
-    pub fn gradient_stops(&self) -> ItemRange<di::GradientStop> {
-        self.iter.cur_stops
-    }
-
-    pub fn glyphs(&self) -> ItemRange<GlyphInstance> {
-        self.iter.cur_glyphs
-    }
-
-    pub fn filters(&self) -> ItemRange<di::FilterOp> {
-        self.iter.cur_filters
-    }
-
-    pub fn filter_datas(&self) -> &Vec<TempFilterData> {
-        &self.iter.cur_filter_data
-    }
-
-    pub fn filter_primitives(&self) -> ItemRange<di::FilterPrimitive> {
-        self.iter.cur_filter_primitives
-    }
-
-    pub fn clip_chain_items(&self) -> ItemRange<di::ClipId> {
-        self.iter.cur_clip_chain_items
-    }
-
-    pub fn display_list(&self) -> &BuiltDisplayList {
-        self.iter.display_list()
-    }
-
-    // Creates a new iterator where this element's iterator is, to hack around borrowck.
-    pub fn sub_iter(&self) -> BuiltDisplayListIter<'a> {
-        BuiltDisplayListIter::new_with_list_and_data(self.iter.list, self.iter.data)
-    }
-}
-
 impl<'a, T> AuxIter<'a, T> {
     pub fn new(item: T, mut data: &'a [u8]) -> Self {
         let mut size = 0usize;
         if !data.is_empty() {
             data = peek_from_slice(data, &mut size);
         };
 
         AuxIter {
@@ -669,16 +688,17 @@ impl Serialize for BuiltDisplayList {
                 Real::PushReferenceFrame(v) => Debug::PushReferenceFrame(v),
                 Real::PushStackingContext(v) => Debug::PushStackingContext(v),
                 Real::PushShadow(v) => Debug::PushShadow(v),
                 Real::BackdropFilter(v) => Debug::BackdropFilter(v),
 
                 Real::PopReferenceFrame => Debug::PopReferenceFrame,
                 Real::PopStackingContext => Debug::PopStackingContext,
                 Real::PopAllShadows => Debug::PopAllShadows,
+                Real::ReuseItem(k) => Debug::ReuseItem(k),
             };
             seq.serialize_element(&serial_di)?
         }
         seq.end()
     }
 }
 
 // The purpose of this implementation is to deserialize
@@ -772,16 +792,17 @@ impl<'de> Deserialize<'de> for BuiltDisp
                 Debug::RadialGradient(v) => Real::RadialGradient(v),
                 Debug::PushStackingContext(v) => Real::PushStackingContext(v),
                 Debug::PushShadow(v) => Real::PushShadow(v),
                 Debug::BackdropFilter(v) => Real::BackdropFilter(v),
 
                 Debug::PopStackingContext => Real::PopStackingContext,
                 Debug::PopReferenceFrame => Real::PopReferenceFrame,
                 Debug::PopAllShadows => Real::PopAllShadows,
+                Debug::ReuseItem(k) => Real::ReuseItem(k),
             };
             poke_into_vec(&item, &mut data);
             // the aux data is serialized after the item, hence the temporary
             data.extend(temp.drain(..));
         }
 
         // Add `DisplayItem::max_size` zone of zeroes to the end of display list
         // so there is at least this amount available in the display list during