From 214a48c9072123116b7ea269bf9cf763eb5bf1d7 Mon Sep 17 00:00:00 2001 From: Jeff Muizelaar Date: Tue, 3 Oct 2017 15:38:38 -0400 Subject: [PATCH] Use an unsafe serializer. This gives a noticable impact on serialization performance in Gecko. wr_dp_push_text() goes from 35.6% of nsDisplayText::CreateWebRenderCommands down to 24%. The generated code is still pretty bad but hopefully adding proper noalias information to rust will fix that. --- webrender_api/src/display_list.rs | 88 +++++++++++++++++++++++++++---- 1 file changed, 77 insertions(+), 11 deletions(-) diff --git a/webrender_api/src/display_list.rs b/webrender_api/src/display_list.rs index 91d87203b5..7af3c8a2fd 100644 --- a/webrender_api/src/display_list.rs +++ b/webrender_api/src/display_list.rs @@ -17,6 +17,8 @@ use YuvImageDisplayItem; use bincode; use serde::{Deserialize, Serialize, Serializer}; use serde::ser::{SerializeMap, SerializeSeq}; +use std::io::Write; +use std::{io, ptr}; use std::marker::PhantomData; use time::precise_time_ns; @@ -483,6 +485,72 @@ impl<'a, 'b> Serialize for DisplayItemRef<'a, 'b> { } } +// This is a replacement for bincode::serialize_into(&vec) +// The default implementation Write for Vec will basically +// call extend_from_slice(). Serde ends up calling that for every +// field of a struct that we're serializing. extend_from_slice() +// does not get inlined and thus we end up calling a generic memcpy() +// implementation. If we instead reserve enough room for the serialized +// struct in the Vec ahead of time we can rely on that and use +// the following UnsafeVecWriter to write into the vec without +// any checks. This writer assumes that size returned by the +// serialize function will not change between calls to serialize_into: +// +// For example, the following struct will cause memory unsafety when +// used with UnsafeVecWriter. +// +// struct S { +// first: Cell, +// } +// +// impl Serialize for S { +// fn serialize(&self, serializer: S) -> Result +// where S: Serializer +// { +// if self.first.get() { +// self.first.set(false); +// ().serialize(serializer) +// } else { +// 0.serialize(serializer) +// } +// } +// } +// + +struct UnsafeVecWriter<'a>(&'a mut Vec); + +impl<'a> Write for UnsafeVecWriter<'a> { + fn write(&mut self, buf: &[u8]) -> io::Result { + unsafe { + let old_len = self.0.len(); + self.0.set_len(old_len + buf.len()); + debug_assert!(self.0.len() <= self.0.capacity()); + ptr::copy_nonoverlapping(buf.as_ptr(), self.0.as_mut_ptr().offset(old_len as isize), buf.len()); + } + Ok(buf.len()) + } + fn flush(&mut self) -> io::Result<()> { Ok(()) } +} + +struct SizeCounter(usize); + +impl<'a> Write for SizeCounter { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0 += buf.len(); + Ok(buf.len()) + } + fn flush(&mut self) -> io::Result<()> { Ok(()) } +} + +fn serialize_fast(vec: &mut Vec, e: &T) { + // manually counting the size is faster than vec.reserve(bincode::serialized_size(&e) as usize) for some reason + let mut size = SizeCounter(0); + bincode::serialize_into(&mut size,e , bincode::Infinite).unwrap(); + vec.reserve(size.0); + + bincode::serialize_into(&mut UnsafeVecWriter(vec), e, bincode::Infinite).unwrap(); +} + #[derive(Clone)] pub struct DisplayListBuilder { pub data: Vec, @@ -541,28 +609,26 @@ impl DisplayListBuilder { } fn push_item(&mut self, item: SpecificDisplayItem, info: &LayoutPrimitiveInfo) { - bincode::serialize_into( + serialize_fast( &mut self.data, &DisplayItem { item, clip_and_scroll: *self.clip_stack.last().unwrap(), info: *info, }, - bincode::Infinite, - ).unwrap(); + ) } fn push_new_empty_item(&mut self, item: SpecificDisplayItem) { let info = LayoutPrimitiveInfo::new(LayoutRect::zero()); - bincode::serialize_into( + serialize_fast( &mut self.data, &DisplayItem { item, clip_and_scroll: *self.clip_stack.last().unwrap(), info, - }, - bincode::Infinite, - ).unwrap(); + } + ) } fn push_iter(&mut self, iter: I) @@ -575,10 +641,10 @@ impl DisplayListBuilder { let len = iter.len(); let mut count = 0; - bincode::serialize_into(&mut self.data, &len, bincode::Infinite).unwrap(); + serialize_fast(&mut self.data, &len); for elem in iter { count += 1; - bincode::serialize_into(&mut self.data, &elem, bincode::Infinite).unwrap(); + serialize_fast(&mut self.data, &elem); } debug_assert_eq!(len, count); @@ -1103,8 +1169,8 @@ impl DisplayListBuilder { // Append glyph data to the end for ((font_key, color), sub_glyphs) in glyphs { - bincode::serialize_into(&mut self.data, &font_key, bincode::Infinite).unwrap(); - bincode::serialize_into(&mut self.data, &color, bincode::Infinite).unwrap(); + serialize_fast(&mut self.data, &font_key); + serialize_fast(&mut self.data, &color); self.push_iter(sub_glyphs); }