Skip to content

Commit

Permalink
Optimize display of large text bodies
Browse files Browse the repository at this point in the history
It turns out calculating the width of a text body is expensive for large bodies, because we have to count every grapheme. This change caches the text dimensions so we only have to calculate it when the text changes.

Further progress on #356
  • Loading branch information
LucasPickering committed Oct 27, 2024
1 parent 135fc12 commit 3b3b1c0
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 87 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
### Fixed

- Fixed `ignore_certificate_hosts` and `large_body_size` fields not being loaded from config
- Improve performance of large response bodies [#356](https://github.com/LucasPickering/slumber/issues/356)
- This includes disabling prettyification and syntax highlighting on bodies over 1 MB (this size is configurable, via the `large_body_size` [config field](https://slumber.lucaspickering.me/book/api/configuration/index.html))
- Loading a large response body should no longer cause the UI to freeze or low framerate

## [2.2.0] - 2024-10-22

Expand Down
20 changes: 11 additions & 9 deletions crates/tui/src/view/common/template_preview.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
context::TuiContext,
message::Message,
view::{draw::Generate, util::highlight, ViewContext},
view::{draw::Generate, state::Identified, util::highlight, ViewContext},
};
use ratatui::{
buffer::Buffer,
Expand Down Expand Up @@ -35,7 +35,7 @@ pub struct TemplatePreview {
/// because it also gets an initial value. There should be effectively zero
/// contention on the mutex because of the single write, and reads being
/// single-threaded.
text: Arc<Mutex<Text<'static>>>,
text: Arc<Mutex<Identified<Text<'static>>>>,
}

impl TemplatePreview {
Expand All @@ -46,15 +46,16 @@ impl TemplatePreview {
/// unrendered and rendered content.
pub fn new(template: Template, content_type: Option<ContentType>) -> Self {
// Calculate raw text
let text = highlight::highlight_if(
let text: Identified<Text> = highlight::highlight_if(
content_type,
// We have to clone the template to detach the lifetime. We're
// choosing to pay one upfront cost here so we don't have to
// recompute the text on each render. Ideally we could hold onto
// the template and have this text reference it, but that would be
// self-referential
template.display().into_owned().into(),
);
)
.into();
let text = Arc::new(Mutex::new(text));

// Trigger a task to render the preview and write the answer back into
Expand All @@ -78,17 +79,17 @@ impl TemplatePreview {
/// mutex
fn calculate_rendered_text(
chunks: Vec<TemplateChunk>,
destination: &Mutex<Text<'static>>,
destination: &Mutex<Identified<Text<'static>>>,
content_type: Option<ContentType>,
) {
let text = TextStitcher::stitch_chunks(&chunks);
let text = highlight::highlight_if(content_type, text);
*destination
.lock()
.expect("Template preview text lock is poisoned") = text;
.expect("Template preview text lock is poisoned") = text.into();
}

pub fn text(&self) -> impl '_ + Deref<Target = Text<'static>> {
pub fn text(&self) -> impl '_ + Deref<Target = Identified<Text<'static>>> {
self.text
.lock()
.expect("Template preview text lock is poisoned")
Expand All @@ -103,7 +104,8 @@ impl From<Template> for TemplatePreview {

/// Clone internal text. Only call this for small pieces of text
impl Generate for &TemplatePreview {
type Output<'this> = Text<'this>
type Output<'this>
= Text<'this>
where
Self: 'this;

Expand All @@ -117,7 +119,7 @@ impl Generate for &TemplatePreview {

impl Widget for &TemplatePreview {
fn render(self, area: Rect, buf: &mut Buffer) {
self.text().deref().render(area, buf)
(&**self.text()).render(area, buf)
}
}

Expand Down
112 changes: 72 additions & 40 deletions crates/tui/src/view/common/text_window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
context::UpdateContext,
draw::{Draw, DrawMetadata},
event::{Event, EventHandler, Update},
state::{Identified, StateCell},
},
};
use ratatui::{
Expand All @@ -19,6 +20,7 @@ use ratatui::{
use slumber_config::Action;
use std::{cell::Cell, cmp};
use unicode_width::UnicodeWidthStr;
use uuid::Uuid;

/// A scrollable (but not editable) block of text. Internal state will be
/// updated on each render, to adjust to the text's width/height. Generally the
Expand All @@ -27,14 +29,13 @@ use unicode_width::UnicodeWidthStr;
/// (especially if it includes syntax highlighting).
#[derive(derive_more::Debug, Default)]
pub struct TextWindow {
/// Cache the size of the text window, because it's expensive to calculate.
/// Checking the width of a text requires counting all its graphemes.
text_size: StateCell<Uuid, TextSize>,
/// Horizontal scroll
offset_x: Cell<usize>,
/// Vertical scroll
offset_y: Cell<usize>,
/// How wide is the full text content?
text_width: Cell<usize>,
/// How tall is the full text content?
text_height: Cell<usize>,
/// How wide is the visible text area, excluding gutter/scrollbars?
window_width: Cell<usize>,
/// How tall is the visible text area, exluding gutter/scrollbars?
Expand All @@ -45,7 +46,7 @@ pub struct TextWindow {
pub struct TextWindowProps<'a> {
/// Text to render. We take a reference because this component tends to
/// contain a lot of text, and we don't want to force a clone on render
pub text: &'a Text<'a>,
pub text: &'a Identified<Text<'a>>,
/// Extra text to render below the text window
pub footer: Option<Text<'a>>,
pub margins: ScrollbarMargins,
Expand All @@ -70,21 +71,35 @@ impl Default for ScrollbarMargins {
}
}

#[derive(Debug, Default)]
struct TextSize {
/// Number of graphemes in the longest line in the text
width: usize,
/// Number of lines in the text
height: usize,
}

impl TextWindow {
/// Get the final line that we can't scroll past. This will be the first
/// line of the last page of text
fn max_scroll_line(&self) -> usize {
self.text_height
let text_height = self
.text_size
.get()
.saturating_sub(self.window_height.get())
.map(|state| state.height)
.unwrap_or_default();
text_height.saturating_sub(self.window_height.get())
}

/// Get the final column that we can't scroll (horizontally) past. This will
/// be the left edge of the rightmost "page" of text
fn max_scroll_column(&self) -> usize {
self.text_width
let text_width = self
.text_size
.get()
.saturating_sub(self.window_width.get())
.map(|state| state.width)
.unwrap_or_default();
text_width.saturating_sub(self.window_width.get())
}

fn scroll_up(&mut self, lines: usize) {
Expand Down Expand Up @@ -137,6 +152,11 @@ impl TextWindow {
.take(self.window_height.get())
.enumerate();
for (y, line) in lines {
// This could be expensive if we're skipping a lot of graphemes,
// i.e. scrolled far to the right in a wide body. Fortunately that's
// a niche use case so not optimized for yet. To fix this we would
// have to map grapheme number -> byte offset and cache that,
// because skipping bytes is O(1) instead of O(n)
let graphemes = line
.styled_graphemes(Style::default())
.skip(self.offset_x.get())
Expand Down Expand Up @@ -185,31 +205,39 @@ impl<'a> Draw<TextWindowProps<'a>> for TextWindow {
) {
let styles = &TuiContext::get().styles;

// Assume no line wrapping when calculating line count
// Note: Paragraph has methods for this, but that requires an owned copy
// of Text, which involves a lot of cloning
let text_height = props.text.lines.len();
let text_width = props
.text
.lines
.iter()
.map(Line::width)
.max()
.unwrap_or_default();
let text_state = self.text_size.get_or_update(&props.text.id(), || {
// Note: Paragraph has methods for this, but that requires an
// owned copy of Text, which involves a lot of cloning

// This counts _graphemes_, not bytes, so it's O(n)
let text_width = props
.text
.lines
.iter()
.map(Line::width)
.max()
.unwrap_or_default();
// Assume no line wrapping when calculating line count
let text_height = props.text.lines.len();
TextSize {
width: text_width,
height: text_height,
}
});

let [gutter_area, _, text_area] = Layout::horizontal([
// Size gutter based on width of max line number
Constraint::Length((text_height as f32).log10().floor() as u16 + 1),
Constraint::Length(
(text_state.height as f32).log10().floor() as u16 + 1,
),
Constraint::Length(1), // Spacer
Constraint::Min(0),
])
.areas(metadata.area());
let has_vertical_scroll = text_height > text_area.height as usize;
let has_horizontal_scroll = text_width > text_area.width as usize;
let has_vertical_scroll = text_state.height > text_area.height as usize;
let has_horizontal_scroll = text_state.width > text_area.width as usize;

// Store text and window sizes for calculations in the update code
self.text_width.set(text_width);
self.text_height.set(text_height);
self.window_width.set(text_area.width as usize);
self.window_height.set(text_area.height as usize);

Expand All @@ -218,8 +246,10 @@ impl<'a> Draw<TextWindowProps<'a>> for TextWindow {

// Draw line numbers in the gutter
let first_line = self.offset_y.get() + 1;
let last_line =
cmp::min(first_line + self.window_height.get() - 1, text_height);
let last_line = cmp::min(
first_line + self.window_height.get() - 1,
text_state.height,
);
frame.render_widget(
Paragraph::new(
(first_line..=last_line)
Expand All @@ -244,7 +274,7 @@ impl<'a> Draw<TextWindowProps<'a>> for TextWindow {
x: text_area.x,
y: text_area.y
+ (cmp::min(
self.text_height.get(),
text_state.height,
self.window_height.get(),
)) as u16,
width: text_area.width,
Expand All @@ -257,7 +287,7 @@ impl<'a> Draw<TextWindowProps<'a>> for TextWindow {
if has_vertical_scroll {
frame.render_widget(
Scrollbar {
content_length: self.text_height.get(),
content_length: text_state.height,
offset: self.offset_y.get(),
// We substracted the margin from the text area before, so
// we have to add that back now
Expand All @@ -270,7 +300,7 @@ impl<'a> Draw<TextWindowProps<'a>> for TextWindow {
if has_horizontal_scroll {
frame.render_widget(
Scrollbar {
content_length: self.text_width.get(),
content_length: text_state.width,
offset: self.offset_x.get(),
orientation: ScrollbarOrientation::HorizontalBottom,
// See note on other scrollbar for +1
Expand Down Expand Up @@ -299,7 +329,8 @@ mod tests {
harness: TestHarness,
) {
let text =
Text::from("line 1\nline 2 is longer\nline 3\nline 4\nline 5");
Text::from("line 1\nline 2 is longer\nline 3\nline 4\nline 5")
.into();
let mut component = TestComponent::new(
&harness,
&terminal,
Expand Down Expand Up @@ -383,7 +414,8 @@ mod tests {
#[with(35, 3)] terminal: TestTerminal,
harness: TestHarness,
) {
let text = Text::from("intro\nπŸ’šπŸ’™πŸ’œ this is a longer line\noutro");
let text =
Text::from("intro\nπŸ’šπŸ’™πŸ’œ this is a longer line\noutro").into();
TestComponent::new(
&harness,
&terminal,
Expand All @@ -410,7 +442,7 @@ mod tests {
#[with(10, 2)] terminal: TestTerminal,
harness: TestHarness,
) {
let text = Text::from("πŸ’šπŸ’™πŸ’œπŸ’šπŸ’™πŸ’œ");
let text = Text::raw("πŸ’šπŸ’™πŸ’œπŸ’šπŸ’™πŸ’œ").into();
TestComponent::new(
&harness,
&terminal,
Expand Down Expand Up @@ -438,9 +470,9 @@ mod tests {
#[with(10, 3)] terminal: TestTerminal,
harness: TestHarness,
) {
let text = ["1 this is a long line", "2", "3", "4", "5"]
.join("\n")
.into();
let text =
Text::from_iter(["1 this is a long line", "2", "3", "4", "5"])
.into();
let mut component = TestComponent::new(
&harness,
&terminal,
Expand All @@ -462,7 +494,7 @@ mod tests {
assert_eq!(component.data().offset_x.get(), 10);
assert_eq!(component.data().offset_y.get(), 2);

let text = ["1 less long line", "2", "3", "4"].join("\n").into();
let text = Text::from_iter(["1 less long line", "2", "3", "4"]).into();
component.set_props(TextWindowProps {
text: &text,
margins: ScrollbarMargins {
Expand All @@ -481,9 +513,9 @@ mod tests {
/// automatically be clamped to match
#[rstest]
fn test_grow_window(terminal: TestTerminal, harness: TestHarness) {
let text = ["1 this is a long line", "2", "3", "4", "5"]
.join("\n")
.into();
let text =
Text::from_iter(["1 this is a long line", "2", "3", "4", "5"])
.into();
let mut component = TestComponent::new(
&harness,
&terminal,
Expand Down
Loading

0 comments on commit 3b3b1c0

Please sign in to comment.