Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
Documentation, improve stack perf
Browse files Browse the repository at this point in the history
  • Loading branch information
@MichaReiser
MichaReiser committed Oct 10, 2022
1 parent 27fe822 commit 81d4ca0
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 54 deletions.
113 changes: 83 additions & 30 deletions crates/rome_formatter/src/printer/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,12 +420,24 @@ impl<'a> Printer<'a> {
}

/// Tries to fit as much content as possible on a single line.
/// Each item forms a virtual group that is either printed in flat or expanded mode.
/// It handles three different cases:
///
/// * The first and second content fit on a single line. It prints the content and separator in flat mode.
/// * The first content fits on a single line, but the second doesn't. It prints the content in flat and the separator in expanded mode.
/// * Neither the first nor the second content fit on the line. It brings the first content and the separator in expanded mode.
/// `Fill` is a sequence of *item*, *separator*, *item*, *separator*, *item*, ... entries.
/// The goal is to fit as many items (with their separators) on a single line as possible and
/// first expand the *separator* if the content exceeds the print width and only fallback to expanding
/// the *item*s if the *item* or the *item* and the expanded *separator* don't fit on the line.
///
/// The implementation handles the following 5 cases:
///
/// * The *item*, *separator*, and the *next item* fit on the same line.
/// Print the *item* and *separator* in flat mode.
/// * The *item* and *separator* fit on the line but there's not enough space for the *next item*.
/// Print the *item* in flat mode and the *separator* in expanded mode.
/// * The *item* fits on the line but the *separator* does not in flat mode.
/// Print the *item* in flat mode and the *separator* in expanded mode.
/// * The *item* fits on the line but the *separator* does not in flat **NOR** expanded mode.
/// Print the *item* and *separator* in expanded mode.
/// * The *item* does not fit on the line.
/// Print the *item* and *separator* in expanded mode.
fn print_fill_entries(
&mut self,
queue: &mut PrintQueue<'a>,
Expand All @@ -444,13 +456,15 @@ impl<'a> Printer<'a> {
while matches!(queue.top(), Some(FormatElement::Tag(Tag::StartEntry))) {
let mut measurer = FitsMeasurer::new_flat(queue, stack, self);

// The number of item/separator pairs that should be printed in flat.
let mut fitting_pairs = 0usize;
let mut item_fits = measurer.fill_entry_fits(PrintMode::Flat)?;
// The number of item/separator pairs that fit on the same line.
let mut flat_pairs = 0usize;
let mut item_fits = measurer.fill_item_fits()?;

let last_pair_layout = if item_fits {
// Measure the remaining pairs until the first item or separator that does not fit (or the end of the fill element).
// Optimisation to avoid re-measuring the next-item.
// Optimisation to avoid re-measuring the next-item twice:
// * Once when measuring if the *item*, *separator*, *next-item* fit
// * A second time when measuring if *next-item*, *separator*, *next-next-item* fit.
loop {
// Item that fits without a following separator.
if !matches!(
Expand All @@ -460,7 +474,7 @@ impl<'a> Printer<'a> {
break FillPairLayout::Flat;
}

let separator_fits = measurer.fill_entry_fits(PrintMode::Flat)?;
let separator_fits = measurer.fill_separator_fits(PrintMode::Flat)?;

// Item fits but the flat separator does not.
if !separator_fits {
Expand All @@ -475,14 +489,14 @@ impl<'a> Printer<'a> {
break FillPairLayout::Flat;
}

item_fits = measurer.fill_entry_fits(PrintMode::Flat)?;
item_fits = measurer.fill_item_fits()?;

if item_fits {
fitting_pairs += 1;
flat_pairs += 1;
} else {
// Item and separator both fit, but the next element won't. Therefore,
// print the separator in expanded mode and then re-measure the item
// in the next loop.
// Item and separator both fit, but the next element doesn't.
// Print the separator in expanded mode and then re-measure if the item now
// fits in the next iteration of the outer loop.
break FillPairLayout::ItemFlatSeparatorExpanded;
}
}
Expand All @@ -495,9 +509,10 @@ impl<'a> Printer<'a> {

self.state.measured_group_fits = true;

for _ in 0..fitting_pairs {
self.print_entry(queue, stack, args.with_print_mode(PrintMode::Flat))?;
self.print_entry(queue, stack, args.with_print_mode(PrintMode::Flat))?;
// Print all pairs that fit in flat mode.
for _ in 0..flat_pairs {
self.print_fill_item(queue, stack, args.with_print_mode(PrintMode::Flat))?;
self.print_fill_separator(queue, stack, args.with_print_mode(PrintMode::Flat))?;
}

let item_mode = match last_pair_layout {
Expand All @@ -507,8 +522,8 @@ impl<'a> Printer<'a> {
let mut measurer = FitsMeasurer::new_flat(queue, stack, self);
// SAFETY: That the item fits is guaranteed by `ItemMaybeFlat`.
// Re-measuring is required to get the measurer in the correct state for measuring the separator.
assert!(measurer.fill_entry_fits(PrintMode::Flat)?);
let separator_fits = measurer.fill_entry_fits(PrintMode::Expanded)?;
assert!(measurer.fill_item_fits()?);
let separator_fits = measurer.fill_separator_fits(PrintMode::Expanded)?;
measurer.finish();

if separator_fits {
Expand All @@ -519,7 +534,7 @@ impl<'a> Printer<'a> {
}
};

self.print_entry(queue, stack, args.with_print_mode(item_mode))?;
self.print_fill_item(queue, stack, args.with_print_mode(item_mode))?;

if matches!(queue.top(), Some(FormatElement::Tag(Tag::StartEntry))) {
let separator_mode = match last_pair_layout {
Expand All @@ -529,8 +544,10 @@ impl<'a> Printer<'a> {
| FillPairLayout::ItemMaybeFlat => PrintMode::Expanded,
};

// Push a new stack frame with print mode `Flat` for the case where the separator gets printed in expanded mode
// but does contain a group to ensure that the group will measure "fits" with the "flat" versions of the next item/separator.
stack.push(TagKind::Fill, args.with_print_mode(PrintMode::Flat));
self.print_entry(queue, stack, args.with_print_mode(separator_mode))?;
self.print_fill_separator(queue, stack, args.with_print_mode(separator_mode))?;
stack.pop(TagKind::Fill)?;
}
}
Expand All @@ -542,6 +559,26 @@ impl<'a> Printer<'a> {
}
}

/// Semantic alias for [Self::print_entry] for fill items.
fn print_fill_item(
&mut self,
queue: &mut PrintQueue<'a>,
stack: &mut PrintCallStack,
args: PrintElementArgs,
) -> PrintResult<()> {
self.print_entry(queue, stack, args)
}

/// Semantic alias for [Self::print_entry] for fill separators.
fn print_fill_separator(
&mut self,
queue: &mut PrintQueue<'a>,
stack: &mut PrintCallStack,
args: PrintElementArgs,
) -> PrintResult<()> {
self.print_entry(queue, stack, args)
}

/// Fully print an element (print the element itself and all its descendants)
///
/// Unlike [print_element], this function ensures the entire element has
Expand Down Expand Up @@ -796,6 +833,7 @@ struct FitsMeasurer<'a, 'print> {
queue: FitsQueue<'a, 'print>,
stack: FitsCallStack<'print>,
printer: &'print mut Printer<'a>,
must_be_flat: bool,
}

impl<'a, 'print> FitsMeasurer<'a, 'print> {}
Expand All @@ -807,7 +845,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
printer: &'print mut Printer<'a>,
) -> Self {
let mut measurer = Self::new(print_queue, print_stack, printer);
measurer.state.must_be_flat = true;
measurer.must_be_flat = true;
measurer
}

Expand All @@ -827,7 +865,6 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
let fits_state = FitsState {
pending_indent: printer.state.pending_indent,
pending_space: printer.state.pending_space,
must_be_flat: false,
line_width: printer.state.line_width,
has_line_suffix: printer.state.line_suffixes.has_pending(),
};
Expand All @@ -836,6 +873,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
state: fits_state,
queue: fits_queue,
stack: fits_stack,
must_be_flat: false,
printer,
}
}
Expand Down Expand Up @@ -865,10 +903,26 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
Ok(true)
}

/// Tests if the content of a `Fill` item fits in [PrintMode::Flat].
///
/// Returns `Err` if the top element of the queue is not a [Tag::StartEntry]
/// or if the document has any mismatching start/end tags.
fn fill_item_fits(&mut self) -> PrintResult<bool> {
self.fill_entry_fits(PrintMode::Flat)
}

/// Tests if the content of a `Fill` separator fits with `mode`.
///
/// Returns `Err` if the top element of the queue is not a [Tag::StartEntry]
/// or if the document has any mismatching start/end tags.
fn fill_separator_fits(&mut self, mode: PrintMode) -> PrintResult<bool> {
self.fill_entry_fits(mode)
}

/// Tests if the elements between the [Tag::StartEntry] and [Tag::EndEntry]
/// of a fill item or separator fits with `mode`.
///
/// Returns `Err` if the queue isn't positioned at a [Tag::StartEntry] or
/// Returns `Err` if the queue isn't positioned at a [Tag::StartEntry] or if
/// the matching [Tag::EndEntry] is missing.
fn fill_entry_fits(&mut self, mode: PrintMode) -> PrintResult<bool> {
let start_entry = self.queue.top();
Expand Down Expand Up @@ -910,7 +964,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
}
LineMode::Soft => {}
LineMode::Hard | LineMode::Empty => {
return Ok(if self.state.must_be_flat {
return Ok(if self.must_be_flat {
Fits::No
} else {
Fits::Yes
Expand Down Expand Up @@ -940,7 +994,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
let char_width = match c {
'\t' => self.options().tab_width,
'\n' => {
return Ok(if self.state.must_be_flat {
return Ok(if self.must_be_flat {
Fits::No
} else {
Fits::Yes
Expand All @@ -965,7 +1019,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
}

FormatElement::ExpandParent => {
if self.state.must_be_flat {
if self.must_be_flat {
return Ok(Fits::No);
}
}
Expand Down Expand Up @@ -1006,7 +1060,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
}

FormatElement::Tag(StartGroup(group)) => {
if self.state.must_be_flat && !group.mode().is_flat() {
if self.must_be_flat && !group.mode().is_flat() {
return Ok(Fits::No);
}

Expand Down Expand Up @@ -1173,7 +1227,6 @@ struct FitsState {
pending_indent: Indention,
pending_space: bool,
has_line_suffix: bool,
must_be_flat: bool,
line_width: usize,
}

Expand Down
4 changes: 2 additions & 2 deletions crates/rome_formatter/src/printer/queue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,8 @@ impl<'a, Q> FusedIterator for QueueContentIterator<'a, '_, Q> where Q: Queue<'a>
/// A predicate determining when to end measuring if some content fits on the line.
///
/// Called for every [`element`](FormatElement) in the [FitsQueue] when measuring if a content
/// fits on the line. The measuring of the content ends for the first [`element`](FormatElement) that this
/// predicate returns `false`.
/// fits on the line. The measuring of the content ends after the first element [`element`](FormatElement) for which this
/// predicate returns `true` (similar to a take while iterator except that it takes while the predicate returns `false`).
pub(super) trait FitsEndPredicate {
fn is_end(&mut self, element: &FormatElement) -> PrintResult<bool>;
}
Expand Down
30 changes: 8 additions & 22 deletions crates/rome_formatter/src/printer/stack.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ pub(super) struct StackedStack<'a, T> {
/// The content of the original stack.
original: &'a [T],

/// The index of the "top" element in the original stack.
original_top: usize,

/// Items that have been pushed since the creation of this stack and aren't part of the `original` stack.
stack: Vec<T>,
}
Expand All @@ -52,11 +49,7 @@ impl<'a, T> StackedStack<'a, T> {

/// Creates a new stack that uses `stack` for storing its elements.
pub(super) fn with_vec(original: &'a [T], stack: Vec<T>) -> Self {
Self {
original_top: original.len(),
original,
stack,
}
Self { original, stack }
}

/// Returns the underlying `stack` vector.
Expand All @@ -70,13 +63,12 @@ where
T: Copy,
{
fn pop(&mut self) -> Option<T> {
self.stack.pop().or_else(|| {
if self.original_top == 0 {
None
} else {
self.original_top -= 1;
Some(self.original[self.original_top])
self.stack.pop().or_else(|| match self.original {
[rest @ .., last] => {
self.original = rest;
Some(*last)
}
_ => None,
})
}

Expand All @@ -85,17 +77,11 @@ where
}

fn top(&self) -> Option<&T> {
self.stack.last().or_else(|| {
if self.original_top == 0 {
None
} else {
Some(&self.original[self.original_top - 1])
}
})
self.stack.last().or_else(|| self.original.last())
}

fn is_empty(&self) -> bool {
self.original_top == 0 && self.stack.is_empty()
self.original.is_empty() && self.stack.is_empty()
}
}

Expand Down

0 comments on commit 81d4ca0

Please sign in to comment.