auto merge of #3772 : pcwalton/servo/slim-down-fragment, r=metajack

16% performance improvement in layout (!)

r? @metajack (or whoever)
This commit is contained in:
bors-servo 2014-10-23 12:18:34 -06:00
commit 012a80cc18
8 changed files with 128 additions and 88 deletions

View file

@ -238,12 +238,12 @@ impl<'a> FlowConstructor<'a> {
Some(url) => {
// FIXME(pcwalton): The fact that image fragments store the cache within them makes
// little sense to me.
ImageFragment(ImageFragmentInfo::new(node,
url,
self.layout_context
.shared
.image_cache
.clone()))
ImageFragment(box ImageFragmentInfo::new(node,
url,
self.layout_context
.shared
.image_cache
.clone()))
}
}
}
@ -258,7 +258,7 @@ impl<'a> FlowConstructor<'a> {
-> SpecificFragmentInfo {
match node.type_id() {
Some(ElementNodeTypeId(HTMLIFrameElementTypeId)) => {
IframeFragment(IframeFragmentInfo::new(node))
IframeFragment(box IframeFragmentInfo::new(node))
}
Some(ElementNodeTypeId(HTMLImageElementTypeId)) => {
self.build_fragment_info_for_image(node, node.image_url())

View file

@ -502,7 +502,7 @@ impl FragmentDisplayListBuilding for Fragment {
if opts::get().show_debug_fragment_borders {
self.build_debug_borders_around_text_fragments(display_list,
flow_origin,
text_fragment,
&**text_fragment,
clip_rect);
}
}
@ -559,7 +559,7 @@ impl FragmentDisplayListBuilding for Fragment {
// the iframe is actually going to be displayed.
match self.specific {
IframeFragment(ref iframe_fragment) => {
self.finalize_position_and_size_of_iframe(iframe_fragment,
self.finalize_position_and_size_of_iframe(&**iframe_fragment,
absolute_fragment_bounds.origin,
layout_context)
}

View file

@ -70,7 +70,11 @@ use url::Url;
/// Different types of fragments may also contain custom data; for example, text fragments contain
/// text.
///
/// FIXME(#2260, pcwalton): This can be slimmed down some.
/// Do not add fields to this structure unless they're really really mega necessary! Fragments get
/// moved around a lot and thus their size impacts performance of layout quite a bit.
///
/// FIXME(#2260, pcwalton): This can be slimmed down some by (at least) moving `inline_context`
/// to be on `InlineFlow` only.
#[deriving(Clone)]
pub struct Fragment {
/// An opaque reference to the DOM node that this `Fragment` originates from.
@ -79,9 +83,6 @@ pub struct Fragment {
/// The CSS style of this fragment.
pub style: Arc<ComputedValues>,
/// How damaged this fragment is since last reflow.
pub restyle_damage: RestyleDamage,
/// The position of this fragment relative to its owning flow.
/// The size includes padding and border, but not margin.
pub border_box: LogicalRect<Au>,
@ -96,18 +97,16 @@ pub struct Fragment {
/// Info specific to the kind of fragment. Keep this enum small.
pub specific: SpecificFragmentInfo,
/// New-line chracter(\n)'s positions(relative, not absolute)
///
/// FIXME(#2260, pcwalton): This is very inefficient; remove.
pub new_line_pos: Vec<CharIndex>,
/// Holds the style context information for fragments
/// that are part of an inline formatting context.
pub inline_context: Option<InlineFragmentContext>,
/// A debug ID that is consistent for the life of
/// this fragment (via transform etc).
pub debug_id: uint,
pub debug_id: u16,
/// How damaged this fragment is since last reflow.
pub restyle_damage: RestyleDamage,
}
impl<E, S: Encoder<E>> Encodable<S, E> for Fragment {
@ -120,14 +119,14 @@ impl<E, S: Encoder<E>> Encodable<S, E> for Fragment {
}
}
/// Info specific to the kind of fragment. Keep this enum small.
/// Info specific to the kind of fragment.
///
/// FIXME(pcwalton): We have completely failed at keeping this enum small.
/// Keep this enum small. As in, no more than one word. Or pcwalton will yell at you.
#[deriving(Clone)]
pub enum SpecificFragmentInfo {
GenericFragment,
IframeFragment(IframeFragmentInfo),
ImageFragment(ImageFragmentInfo),
IframeFragment(Box<IframeFragmentInfo>),
ImageFragment(Box<ImageFragmentInfo>),
/// A hypothetical box (see CSS 2.1 § 10.3.7) for an absolutely-positioned block that was
/// declared with `display: inline;`.
@ -135,7 +134,7 @@ pub enum SpecificFragmentInfo {
InlineBlockFragment(InlineBlockFragmentInfo),
InputFragment,
ScannedTextFragment(ScannedTextFragmentInfo),
ScannedTextFragment(Box<ScannedTextFragmentInfo>),
TableFragment,
TableCellFragment,
TableColumnFragment(TableColumnFragmentInfo),
@ -368,6 +367,12 @@ pub struct ScannedTextFragmentInfo {
/// The range within the above text run that this represents.
pub range: Range<CharIndex>,
/// The positions of newlines within this scanned text fragment.
///
/// FIXME(#2260, pcwalton): Can't this go somewhere else, like in the text run or something?
/// Or can we just remove it?
pub new_line_pos: Vec<CharIndex>,
/// The new_line_pos is eaten during line breaking. If we need to re-merge
/// fragments, it will have to be restored.
pub original_new_line_pos: Option<Vec<CharIndex>>,
@ -378,11 +383,15 @@ pub struct ScannedTextFragmentInfo {
impl ScannedTextFragmentInfo {
/// Creates the information specific to a scanned text fragment from a range and a text run.
pub fn new(run: Arc<Box<TextRun>>, range: Range<CharIndex>, content_size: LogicalSize<Au>)
pub fn new(run: Arc<Box<TextRun>>,
range: Range<CharIndex>,
new_line_positions: Vec<CharIndex>,
content_size: LogicalSize<Au>)
-> ScannedTextFragmentInfo {
ScannedTextFragmentInfo {
run: run,
range: range,
new_line_pos: new_line_positions,
original_new_line_pos: None,
content_size: content_size,
}
@ -406,12 +415,15 @@ impl SplitInfo {
}
}
/// Data for an unscanned text fragment. Unscanned text fragments are the results of flow construction that
/// have not yet had their inline-size determined.
/// Data for an unscanned text fragment. Unscanned text fragments are the results of flow
/// construction that have not yet had their inline-size determined.
#[deriving(Clone)]
pub struct UnscannedTextFragmentInfo {
/// The text inside the fragment.
pub text: String,
///
/// FIXME(pcwalton): Is there something more clever we can do here that avoids the double
/// indirection while not penalizing all fragments?
pub text: Box<String>,
}
impl UnscannedTextFragmentInfo {
@ -419,7 +431,7 @@ impl UnscannedTextFragmentInfo {
pub fn new(node: &ThreadSafeLayoutNode) -> UnscannedTextFragmentInfo {
// FIXME(pcwalton): Don't copy text; atomically reference count it instead.
UnscannedTextFragmentInfo {
text: node.text(),
text: box node.text(),
}
}
@ -427,7 +439,7 @@ impl UnscannedTextFragmentInfo {
#[inline]
pub fn from_text(text: String) -> UnscannedTextFragmentInfo {
UnscannedTextFragmentInfo {
text: text,
text: box text,
}
}
}
@ -436,7 +448,7 @@ impl UnscannedTextFragmentInfo {
#[deriving(Clone)]
pub struct TableColumnFragmentInfo {
/// the number of columns a <col> element should span
pub span: Option<int>,
pub span: int,
}
impl TableColumnFragmentInfo {
@ -447,7 +459,7 @@ impl TableColumnFragmentInfo {
element.get_attr(&ns!(""), &atom!("span")).and_then(|string| {
let n: Option<int> = FromStr::from_str(string);
n
})
}).unwrap_or(0)
};
TableColumnFragmentInfo {
span: span,
@ -476,7 +488,6 @@ impl Fragment {
border_padding: LogicalMargin::zero(writing_mode),
margin: LogicalMargin::zero(writing_mode),
specific: constructor.build_specific_fragment_info_for_node(node),
new_line_pos: vec!(),
inline_context: None,
debug_id: layout_debug::generate_unique_debug_id(),
}
@ -495,7 +506,6 @@ impl Fragment {
border_padding: LogicalMargin::zero(writing_mode),
margin: LogicalMargin::zero(writing_mode),
specific: specific,
new_line_pos: vec!(),
inline_context: None,
debug_id: layout_debug::generate_unique_debug_id(),
}
@ -525,7 +535,6 @@ impl Fragment {
border_padding: LogicalMargin::zero(writing_mode),
margin: LogicalMargin::zero(writing_mode),
specific: specific,
new_line_pos: vec!(),
inline_context: None,
debug_id: layout_debug::generate_unique_debug_id(),
}
@ -546,7 +555,6 @@ impl Fragment {
border_padding: LogicalMargin::zero(writing_mode),
margin: LogicalMargin::zero(writing_mode),
specific: specific,
new_line_pos: vec!(),
inline_context: None,
debug_id: layout_debug::generate_unique_debug_id(),
}
@ -562,8 +570,8 @@ impl Fragment {
pub fn save_new_line_pos(&mut self) {
match &mut self.specific {
&ScannedTextFragment(ref mut info) => {
if !self.new_line_pos.is_empty() {
info.original_new_line_pos = Some(self.new_line_pos.clone());
if !info.new_line_pos.is_empty() {
info.original_new_line_pos = Some(info.new_line_pos.clone());
}
}
_ => {}
@ -575,7 +583,7 @@ impl Fragment {
&ScannedTextFragment(ref mut info) => {
match info.original_new_line_pos.take() {
None => {}
Some(new_line_pos) => self.new_line_pos = new_line_pos,
Some(new_line_pos) => info.new_line_pos = new_line_pos,
}
return
}
@ -585,15 +593,17 @@ impl Fragment {
/// Returns a debug ID of this fragment. This ID should not be considered stable across
/// multiple layouts or fragment manipulations.
pub fn debug_id(&self) -> uint {
pub fn debug_id(&self) -> u16 {
self.debug_id
}
/// Transforms this fragment into another fragment of the given type, with the given size,
/// preserving all the other data.
pub fn transform(&self, size: LogicalSize<Au>, mut info: ScannedTextFragmentInfo) -> Fragment {
let new_border_box =
LogicalRect::from_point_size(self.style.writing_mode, self.border_box.start, size);
pub fn transform(&self, size: LogicalSize<Au>, mut info: Box<ScannedTextFragmentInfo>)
-> Fragment {
let new_border_box = LogicalRect::from_point_size(self.style.writing_mode,
self.border_box.start,
size);
info.content_size = size.clone();
@ -605,7 +615,6 @@ impl Fragment {
border_padding: self.border_padding,
margin: self.margin,
specific: ScannedTextFragment(info),
new_line_pos: self.new_line_pos.clone(),
inline_context: self.inline_context.clone(),
debug_id: self.debug_id,
}
@ -918,6 +927,22 @@ impl Fragment {
self.is_scanned_text_fragment()
}
/// Returns the newline positions of this fragment, if it's a scanned text fragment.
pub fn newline_positions(&self) -> Option<&Vec<CharIndex>> {
match self.specific {
ScannedTextFragment(ref info) => Some(&info.new_line_pos),
_ => None,
}
}
/// Returns the newline positions of this fragment, if it's a scanned text fragment.
pub fn newline_positions_mut(&mut self) -> Option<&mut Vec<CharIndex>> {
match self.specific {
ScannedTextFragment(ref mut info) => Some(&mut info.new_line_pos),
_ => None,
}
}
/// Returns true if and only if this is a scanned text fragment.
fn is_scanned_text_fragment(&self) -> bool {
match self.specific {
@ -1049,19 +1074,22 @@ impl Fragment {
fail!("Inline blocks or inline absolute hypothetical fragments do not get split")
}
ScannedTextFragment(ref text_fragment_info) => {
let mut new_line_pos = self.new_line_pos.clone();
let mut new_line_pos = text_fragment_info.new_line_pos.clone();
let cur_new_line_pos = new_line_pos.remove(0).unwrap();
let inline_start_range = Range::new(text_fragment_info.range.begin(), cur_new_line_pos);
let inline_end_range = Range::new(text_fragment_info.range.begin() + cur_new_line_pos + CharIndex(1),
text_fragment_info.range.length() - (cur_new_line_pos + CharIndex(1)));
let inline_start_range = Range::new(text_fragment_info.range.begin(),
cur_new_line_pos);
let inline_end_range = Range::new(
text_fragment_info.range.begin() + cur_new_line_pos + CharIndex(1),
text_fragment_info.range.length() - (cur_new_line_pos + CharIndex(1)));
// Left fragment is for inline-start text of first founded new-line character.
let inline_start_fragment = SplitInfo::new(inline_start_range, text_fragment_info);
let inline_start_fragment = SplitInfo::new(inline_start_range,
&**text_fragment_info);
// Right fragment is for inline-end text of first founded new-line character.
let inline_end_fragment = if inline_end_range.length() > CharIndex(0) {
Some(SplitInfo::new(inline_end_range, text_fragment_info))
Some(SplitInfo::new(inline_end_range, &**text_fragment_info))
} else {
None
};
@ -1078,24 +1106,30 @@ impl Fragment {
/// Otherwise the information pertaining to the split is returned. The inline-start
/// and inline-end split information are both optional due to the possibility of
/// them being whitespace.
//
// TODO(bjz): The text run should be removed in the future, but it is currently needed for
// the current method of fragment splitting in the `inline::try_append_*` functions.
pub fn find_split_info_for_inline_size(&self, start: CharIndex, max_inline_size: Au, starts_line: bool)
-> Option<(Option<SplitInfo>, Option<SplitInfo>, Arc<Box<TextRun>> /* TODO(bjz): remove */)> {
pub fn find_split_info_for_inline_size(&self,
start: CharIndex,
max_inline_size: Au,
starts_line: bool)
-> Option<(Option<SplitInfo>,
Option<SplitInfo>,
Arc<Box<TextRun>>)> {
match self.specific {
GenericFragment | IframeFragment(_) | ImageFragment(_) | TableFragment | TableCellFragment |
TableRowFragment | TableWrapperFragment | InlineBlockFragment(_) | InputFragment |
InlineAbsoluteHypotheticalFragment(_) => None,
GenericFragment | IframeFragment(_) | ImageFragment(_) | TableFragment |
TableCellFragment | TableRowFragment | TableWrapperFragment | InlineBlockFragment(_) |
InputFragment | InlineAbsoluteHypotheticalFragment(_) => None,
TableColumnFragment(_) => fail!("Table column fragments do not have inline_size"),
UnscannedTextFragment(_) => fail!("Unscanned text fragments should have been scanned by now!"),
UnscannedTextFragment(_) => {
fail!("Unscanned text fragments should have been scanned by now!")
}
ScannedTextFragment(ref text_fragment_info) => {
let mut pieces_processed_count: uint = 0;
let mut remaining_inline_size: Au = max_inline_size;
let mut inline_start_range = Range::new(text_fragment_info.range.begin() + start, CharIndex(0));
let mut inline_start_range = Range::new(text_fragment_info.range.begin() + start,
CharIndex(0));
let mut inline_end_range: Option<Range<CharIndex>> = None;
debug!("split_to_inline_size: splitting text fragment (strlen={}, range={}, avail_inline_size={})",
debug!("split_to_inline_size: splitting text fragment \
(strlen={}, range={}, avail_inline_size={})",
text_fragment_info.run.text.len(),
text_fragment_info.range,
max_inline_size);
@ -1151,12 +1185,12 @@ impl Fragment {
None
} else {
let inline_start = if inline_start_is_some {
Some(SplitInfo::new(inline_start_range, text_fragment_info))
Some(SplitInfo::new(inline_start_range, &**text_fragment_info))
} else {
None
};
let inline_end = inline_end_range.map(|inline_end_range| {
SplitInfo::new(inline_end_range, text_fragment_info)
SplitInfo::new(inline_end_range, &**text_fragment_info)
});
Some((inline_start, inline_end, text_fragment_info.run.clone()))

View file

@ -8,7 +8,7 @@ use style::ComputedValues;
bitflags! {
#[doc = "Individual layout actions that may be necessary after restyling."]
flags RestyleDamage: int {
flags RestyleDamage: u8 {
#[doc = "Repaint the node itself."]
#[doc = "Currently unused; need to decide how this propagates."]
static Repaint = 0x01,

View file

@ -391,7 +391,11 @@ impl LineBreaker {
}
fn try_append_to_line_by_new_line(&mut self, in_fragment: Fragment) -> bool {
if in_fragment.new_line_pos.len() == 0 {
let no_newline_positions = match in_fragment.newline_positions() {
None => true,
Some(ref positions) => positions.is_empty(),
};
if no_newline_positions {
debug!("LineBreaker: Did not find a new-line character, so pushing the fragment to \
the line without splitting.");
self.push_fragment_to_line(in_fragment);
@ -406,13 +410,14 @@ impl LineBreaker {
let writing_mode = self.floats.writing_mode;
let split_fragment = |split: SplitInfo| {
let info =
ScannedTextFragmentInfo::new(
run.clone(),
split.range,
in_fragment.border_box.size);
let size = LogicalSize::new(
writing_mode, split.inline_size, in_fragment.border_box.size.block);
let info = box ScannedTextFragmentInfo::new(run.clone(),
split.range,
(*in_fragment.newline_positions()
.unwrap()).clone(),
in_fragment.border_box.size);
let size = LogicalSize::new(writing_mode,
split.inline_size,
in_fragment.border_box.size.block);
in_fragment.transform(size, info)
};
@ -420,14 +425,14 @@ impl LineBreaker {
to the line.");
let mut inline_start = split_fragment(inline_start);
inline_start.save_new_line_pos();
inline_start.new_line_pos = vec![];
*inline_start.newline_positions_mut().unwrap() = vec![];
self.push_fragment_to_line(inline_start);
for inline_end in inline_end.into_iter() {
debug!("LineBreaker: Deferring the fragment to the inline_end of the new-line \
character to the line.");
let mut inline_end = split_fragment(inline_end);
inline_end.new_line_pos.remove(0);
inline_end.newline_positions_mut().unwrap().remove(0);
self.work_list.push_front(inline_end);
}
@ -497,11 +502,10 @@ impl LineBreaker {
line_is_empty);
match split.map(|(inline_start, inline_end, run)| {
let split_fragment = |split: SplitInfo| {
let info =
ScannedTextFragmentInfo::new(
run.clone(),
split.range,
in_fragment.border_box.size);
let info = box ScannedTextFragmentInfo::new(run.clone(),
split.range,
Vec::new(),
in_fragment.border_box.size);
let size = LogicalSize::new(self.floats.writing_mode,
split.inline_size,
in_fragment.border_box.size.block);

View file

@ -92,8 +92,8 @@ impl Drop for Scope {
/// Generate a unique ID. This is used for items such as Fragment
/// which are often reallocated but represent essentially the
/// same data.
pub fn generate_unique_debug_id() -> uint {
unsafe { DEBUG_ID_COUNTER.fetch_add(1, SeqCst) }
pub fn generate_unique_debug_id() -> u16 {
unsafe { DEBUG_ID_COUNTER.fetch_add(1, SeqCst) as u16 }
}
/// Begin a layout debug trace. If this has not been called,

View file

@ -13,6 +13,7 @@ use layout_debug;
use wrapper::ThreadSafeLayoutNode;
use servo_util::geometry::Au;
use std::cmp::max;
use std::fmt;
use style::computed_values::LengthOrPercentageOrAuto;
@ -64,7 +65,7 @@ impl Flow for TableColGroupFlow {
// Retrieve the specified value from the appropriate CSS property.
let inline_size = fragment.style().content_inline_size();
let span: int = match fragment.specific {
TableColumnFragment(col_fragment) => col_fragment.span.unwrap_or(1),
TableColumnFragment(col_fragment) => max(col_fragment.span, 1),
_ => fail!("non-table-column fragment inside table column?!"),
};
for _ in range(0, span) {

View file

@ -160,16 +160,17 @@ impl TextRunScanner {
}
let text_size = old_fragment.border_box.size;
let &NewLinePositions(ref mut new_line_positions) =
new_line_positions.get_mut(logical_offset);
let new_text_fragment_info =
ScannedTextFragmentInfo::new(run.clone(), range, text_size);
box ScannedTextFragmentInfo::new(run.clone(),
range,
mem::replace(new_line_positions, Vec::new()),
text_size);
let new_metrics = new_text_fragment_info.run.metrics_for_range(&range);
let bounding_box_size = bounding_box_for_run_metrics(&new_metrics,
old_fragment.style.writing_mode);
let mut new_fragment = old_fragment.transform(bounding_box_size,
new_text_fragment_info);
let &NewLinePositions(ref mut new_line_positions) =
new_line_positions.get_mut(logical_offset);
new_fragment.new_line_pos = mem::replace(new_line_positions, Vec::new());
let new_fragment = old_fragment.transform(bounding_box_size, new_text_fragment_info);
out_fragments.push(new_fragment)
}