Skip to content

Commit

Permalink
Improve code quality for push_new_empty_item.
Browse files Browse the repository at this point in the history
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.
rust-lang/rust#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.
  • Loading branch information
jrmuizel committed Nov 22, 2018
1 parent 1ad9b79 commit 97a7db1
Showing 1 changed file with 12 additions and 7 deletions.
19 changes: 12 additions & 7 deletions webrender_api/src/display_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
)
Expand Down Expand Up @@ -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(
Expand All @@ -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);
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 97a7db1

Please sign in to comment.