From 97a7db1896007acd6f54fe0b42cd0104b018116f Mon Sep 17 00:00:00 2001 From: Jeff Muizelaar Date: Thu, 22 Nov 2018 17:48:24 -0500 Subject: [PATCH] Improve code quality for push_new_empty_item. LLVM's ability to eliminate memcpy's across basic blocks is bad. Because of this, we want to make sure that we avoid putting branches in code where we want the memcpy elimination to happen. https://github.com/rust-lang/rust/issues/56172 has a reduced test case of this happening. This change lifts the branch caused by unwrap() above the creation of the SpecificDisplayItem. It ends up saving a memcpy of 127 bytes along with reducing pop_reference_frame by 18 instructions. --- webrender_api/src/display_list.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/webrender_api/src/display_list.rs b/webrender_api/src/display_list.rs index a31f5ae831..8c83c61c00 100644 --- a/webrender_api/src/display_list.rs +++ b/webrender_api/src/display_list.rs @@ -1002,13 +1002,13 @@ impl DisplayListBuilder { ) } - fn push_new_empty_item(&mut self, item: SpecificDisplayItem) { + fn push_new_empty_item(&mut self, item: SpecificDisplayItem, clip_and_scroll: &ClipAndScrollInfo) { let info = LayoutPrimitiveInfo::new(LayoutRect::zero()); serialize_fast( &mut self.data, &DisplayItem { item, - clip_and_scroll: *self.clip_stack.last().unwrap(), + clip_and_scroll: *clip_and_scroll, info, } ) @@ -1279,7 +1279,8 @@ impl DisplayListBuilder { } pub fn pop_reference_frame(&mut self) { - self.push_new_empty_item(SpecificDisplayItem::PopReferenceFrame); + let clip_and_scroll = *self.clip_stack.last().unwrap(); + self.push_new_empty_item(SpecificDisplayItem::PopReferenceFrame, &clip_and_scroll); } pub fn push_stacking_context( @@ -1305,14 +1306,16 @@ impl DisplayListBuilder { } pub fn pop_stacking_context(&mut self) { - self.push_new_empty_item(SpecificDisplayItem::PopStackingContext); + let clip_and_scroll = *self.clip_stack.last().unwrap(); + self.push_new_empty_item(SpecificDisplayItem::PopStackingContext, &clip_and_scroll); } pub fn push_stops(&mut self, stops: &[GradientStop]) { if stops.is_empty() { return; } - self.push_new_empty_item(SpecificDisplayItem::SetGradientStops); + let clip_and_scroll = *self.clip_stack.last().unwrap(); + self.push_new_empty_item(SpecificDisplayItem::SetGradientStops, &clip_and_scroll); self.push_iter(stops); } @@ -1399,7 +1402,8 @@ impl DisplayListBuilder { I::IntoIter: ExactSizeIterator + Clone, { let id = self.generate_clip_chain_id(); - self.push_new_empty_item(SpecificDisplayItem::ClipChain(ClipChainItem { id, parent })); + let clip_and_scroll = *self.clip_stack.last().unwrap(); + self.push_new_empty_item(SpecificDisplayItem::ClipChain(ClipChainItem { id, parent }), &clip_and_scroll); self.push_iter(clips); id } @@ -1524,7 +1528,8 @@ impl DisplayListBuilder { } pub fn pop_all_shadows(&mut self) { - self.push_new_empty_item(SpecificDisplayItem::PopAllShadows); + let clip_and_scroll = *self.clip_stack.last().unwrap(); + self.push_new_empty_item(SpecificDisplayItem::PopAllShadows, &clip_and_scroll); } pub fn finalize(self) -> (PipelineId, LayoutSize, BuiltDisplayList) {