From aa50f1b4fec50fd6130e9c923547faad7a5a51cd Mon Sep 17 00:00:00 2001 From: Oz Date: Sun, 7 Jun 2026 02:49:11 +0000 Subject: [PATCH] fix: reduce table layout memory by Arc-sharing, reusing TextFrames, and adding row limit (APP-4687) Three optimizations to reduce memory spikes during markdown table rendering in layout_table_block: 1. Wrap TableBlockCache fields (table, cell_offset_maps, offset_map) in Arc and share via Arc::clone instead of deep-cloning on every layout pass. 2. Retain the measurement-pass TextFrame from measure_table_cells and reuse it in the final pass when the cell's natural width fits within the computed column width without wrapping, avoiding redundant text layout. 3. Add MAX_TABLE_BODY_ROWS (500) limit to truncate very large tables during layout, preventing unbounded memory growth. Full source text is preserved in the buffer for copy/paste and editing. Co-Authored-By: Oz --- crates/editor/src/content/edit.rs | 73 +++++++++++++++---- crates/editor/src/content/text.rs | 12 +-- .../editor/src/render/element/table_tests.rs | 6 +- .../editor/src/render/model/location_tests.rs | 6 +- crates/editor/src/render/model/mod.rs | 6 +- crates/editor/src/render/model/mod_tests.rs | 10 +-- 6 files changed, 78 insertions(+), 35 deletions(-) diff --git a/crates/editor/src/content/edit.rs b/crates/editor/src/content/edit.rs index caad98e49f..2407e1dc4e 100644 --- a/crates/editor/src/content/edit.rs +++ b/crates/editor/src/content/edit.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use std::mem; use std::ops::Range; use std::path::{Path, PathBuf}; +use std::sync::Arc; use anyhow::{Result, anyhow}; use itertools::Itertools; @@ -20,7 +21,7 @@ use warpui_core::fonts::Weight; use warpui_core::image_cache::ImageType; use warpui_core::text::char_slice; use warpui_core::text::point::Point; -use warpui_core::text_layout::{StyleAndFont, TextAlignment}; +use warpui_core::text_layout::{StyleAndFont, TextAlignment, TextFrame}; use warpui_core::units::{IntoPixels, Pixels}; use warpui_core::{AppContext, SingletonEntity}; @@ -123,6 +124,11 @@ const DEFAULT_IMAGE_HEIGHT_LINE_MULTIPLIER: f32 = 10.0; const MIN_TABLE_CELL_CONTENT_WIDTH_EMS: f32 = 1.0; const MAX_TABLE_CELL_CONTENT_WIDTH_PX: f32 = 500.0; +/// Maximum number of body rows rendered for a markdown table. Tables larger than this are truncated +/// during layout to prevent unbounded memory growth — the full source text is still preserved in +/// the buffer so that copy/paste and editing are unaffected. +const MAX_TABLE_BODY_ROWS: usize = 500; + /// Metadata for rendering a temporary block in the render model. Temporary blocks are not selectable or editable. #[derive(Debug, Clone, PartialEq)] pub struct TemporaryBlock { @@ -243,6 +249,10 @@ struct LayOutArgs { struct TableCellTextLayout { paragraph_style: ParagraphStyles, text_layout: InlineTextLayoutInput, + /// TextFrame from the measurement pass (unconstrained width). Reused in the final pass when + /// the cell's natural width fits within the computed column width, avoiding a redundant + /// text layout. + measure_frame: Arc, } /// The first table layout pass measures each cell without a column-width constraint so we can @@ -287,7 +297,7 @@ fn measure_table_cells( text: line.text, style_runs: line.style_runs, }; - let frame = layout.layout_text_with_options( + let measure_frame = layout.layout_text_with_options( &text_layout.text, ¶graph_style, &text_layout.style_runs, @@ -295,7 +305,7 @@ fn measure_table_cells( TextAlignment::Left, ); - let cell_width = (frame.max_width() + table_style.cell_padding * 2.0) + let cell_width = (measure_frame.max_width() + table_style.cell_padding * 2.0) .into_pixels() .max(minimum_table_cell_width(table_style)) .min(maximum_table_cell_width(table_style)); @@ -305,6 +315,7 @@ fn measure_table_cells( row_text_layouts.push(TableCellTextLayout { paragraph_style, text_layout, + measure_frame, }); } cell_text_layouts.push(row_text_layouts); @@ -1208,9 +1219,10 @@ fn layout_table_block( let cached = style_cache .or(owned_cache.as_ref()) .expect("cache is always populated from style or owned fallback"); - let table = cached.table.clone(); - let cell_offset_maps = cached.cell_offset_maps.clone(); - let offset_map = cached.offset_map.clone(); + // Share via Arc instead of deep-cloning the parsed table data. + let table = Arc::clone(&cached.table); + let cell_offset_maps = Arc::clone(&cached.cell_offset_maps); + let offset_map = Arc::clone(&cached.offset_map); let table_style = layout.rich_text_styles().table_style; let mut header_paragraph_style = layout.paragraph_styles(&text_block.style); @@ -1224,14 +1236,34 @@ fn layout_table_block( body_paragraph_style.line_height_ratio = TABLE_LINE_HEIGHT_RATIO; body_paragraph_style.baseline_ratio = TABLE_BASELINE_RATIO; + // Truncate very large tables to cap memory usage during layout. The header row plus up to + // MAX_TABLE_BODY_ROWS body rows are rendered; the full source text is still preserved in the + // buffer. + let truncated_table; + let effective_table: &FormattedTable = if table.rows.len() > MAX_TABLE_BODY_ROWS { + log::warn!( + "Table has {} rows; truncating to {} for layout", + table.rows.len(), + MAX_TABLE_BODY_ROWS + ); + truncated_table = FormattedTable { + headers: table.headers.clone(), + alignments: table.alignments.clone(), + rows: table.rows[..MAX_TABLE_BODY_ROWS].to_vec(), + }; + &truncated_table + } else { + &table + }; + let (column_widths, cell_text_layouts) = measure_table_cells( - &table, + effective_table, layout, &table_style, header_paragraph_style, body_paragraph_style, ); - let cell_links = table_cell_links(&table); + let cell_links = table_cell_links(effective_table); let mut row_heights = Vec::with_capacity(cell_text_layouts.len()); let mut cell_text_frames = Vec::with_capacity(cell_text_layouts.len()); @@ -1249,13 +1281,24 @@ fn layout_table_block( .unwrap_or(0.0) - table_style.cell_padding * 2.0) .max(0.0); - let frame = layout.layout_text_with_options( - &cell.text_layout.text, - &cell.paragraph_style, - &cell.text_layout.style_runs, - cell_content_width, - table_column_text_alignment(&table, col_idx), - ); + + // Reuse the measurement-pass TextFrame when the cell fits within the column width + // without wrapping and the column alignment is Left (same as the measurement pass). + // This avoids a redundant text layout for cells that don't need re-wrapping. + let alignment = table_column_text_alignment(effective_table, col_idx); + let frame = if cell.measure_frame.max_width() <= cell_content_width + && alignment == TextAlignment::Left + { + Arc::clone(&cell.measure_frame) + } else { + layout.layout_text_with_options( + &cell.text_layout.text, + &cell.paragraph_style, + &cell.text_layout.style_runs, + cell_content_width, + alignment, + ) + }; let cell_layout = CellLayout::from_text_frame(&frame); let text_height = cell_layout .line_heights diff --git a/crates/editor/src/content/text.rs b/crates/editor/src/content/text.rs index 734e3ad070..ba65c0ff51 100644 --- a/crates/editor/src/content/text.rs +++ b/crates/editor/src/content/text.rs @@ -88,9 +88,9 @@ pub fn table_cell_offset_maps( /// table layout) avoid re-running `table_from_internal_format_with_inline_markdown` and /// `table_cell_offset_maps` on every access. pub struct TableBlockCache { - pub table: FormattedTable, - pub cell_offset_maps: Vec>, - pub offset_map: TableOffsetMap, + pub table: Arc, + pub cell_offset_maps: Arc>>, + pub offset_map: Arc, } impl TableBlockCache { @@ -108,9 +108,9 @@ impl TableBlockCache { .collect::>(); let offset_map = TableOffsetMap::new(cell_lengths); Self { - table, - cell_offset_maps, - offset_map, + table: Arc::new(table), + cell_offset_maps: Arc::new(cell_offset_maps), + offset_map: Arc::new(offset_map), } } } diff --git a/crates/editor/src/render/element/table_tests.rs b/crates/editor/src/render/element/table_tests.rs index 3abb3a79cd..c24c52aa9e 100644 --- a/crates/editor/src/render/element/table_tests.rs +++ b/crates/editor/src/render/element/table_tests.rs @@ -82,14 +82,14 @@ fn test_laid_out_table() -> LaidOutTable { let cell_text_frames = vec![vec![Arc::new(TextFrame::mock("")); 2]; 3]; LaidOutTable { - table, + table: Arc::new(table), config, row_heights, column_widths, total_height, - offset_map, + offset_map: Arc::new(offset_map), content_length, - cell_offset_maps, + cell_offset_maps: Arc::new(cell_offset_maps), row_y_offsets, col_x_offsets, cell_text_frames, diff --git a/crates/editor/src/render/model/location_tests.rs b/crates/editor/src/render/model/location_tests.rs index b64343cf74..5aefa508b9 100644 --- a/crates/editor/src/render/model/location_tests.rs +++ b/crates/editor/src/render/model/location_tests.rs @@ -62,7 +62,7 @@ fn test_table_layout() -> LaidOutTable { let cell_frame = Arc::new(TextFrame::mock("aaa")); LaidOutTable { - table, + table: Arc::new(table), config: TableBlockConfig { width: 60.0.into_pixels(), spacing: Default::default(), @@ -86,9 +86,9 @@ fn test_table_layout() -> LaidOutTable { row_heights: vec![20.0.into_pixels(), 20.0.into_pixels()], column_widths: vec![30.0.into_pixels(), 30.0.into_pixels()], total_height: 40.0.into_pixels(), - offset_map, + offset_map: Arc::new(offset_map), content_length, - cell_offset_maps, + cell_offset_maps: Arc::new(cell_offset_maps), row_y_offsets: vec![0.0, 20.0, 40.0], col_x_offsets: vec![0.0, 30.0, 60.0], cell_text_frames: vec![ diff --git a/crates/editor/src/render/model/mod.rs b/crates/editor/src/render/model/mod.rs index ae60fc5b2f..e75edc4488 100644 --- a/crates/editor/src/render/model/mod.rs +++ b/crates/editor/src/render/model/mod.rs @@ -1198,14 +1198,14 @@ impl CellLayout { #[derive(Debug, Clone)] pub struct LaidOutTable { - pub table: FormattedTable, + pub table: Arc, pub config: TableBlockConfig, pub row_heights: Vec, pub column_widths: Vec, pub total_height: Pixels, - pub offset_map: table_offset_map::TableOffsetMap, + pub offset_map: Arc, pub content_length: CharOffset, - pub cell_offset_maps: Vec>, + pub cell_offset_maps: Arc>>, pub row_y_offsets: Vec, pub col_x_offsets: Vec, pub cell_text_frames: Vec>>, diff --git a/crates/editor/src/render/model/mod_tests.rs b/crates/editor/src/render/model/mod_tests.rs index 5ec7e0457a..d232610ce5 100644 --- a/crates/editor/src/render/model/mod_tests.rs +++ b/crates/editor/src/render/model/mod_tests.rs @@ -1241,7 +1241,7 @@ fn make_test_laid_out_table() -> LaidOutTable { let cell_layout = make_test_cell_layout(); let cell_frame = Arc::new(TextFrame::mock("aaa")); LaidOutTable { - table, + table: Arc::new(table), config: TableBlockConfig { width: 60.0.into_pixels(), spacing: DEFAULT_BLOCK_SPACINGS.text, @@ -1300,9 +1300,9 @@ fn make_test_laid_out_table() -> LaidOutTable { row_heights: vec![20.0.into_pixels(), 20.0.into_pixels()], column_widths: vec![30.0.into_pixels(), 30.0.into_pixels()], total_height: 40.0.into_pixels(), - offset_map, + offset_map: Arc::new(offset_map), content_length, - cell_offset_maps, + cell_offset_maps: Arc::new(cell_offset_maps), row_y_offsets: vec![0.0, 20.0, 40.0], col_x_offsets: vec![0.0, 30.0, 60.0], cell_text_frames: vec![ @@ -1383,7 +1383,7 @@ fn test_disabled_horizontal_scroll_reveal_offset_is_noop() { #[test] fn test_link_at_offset_uses_cached_cell_links() { let mut table = make_test_laid_out_table(); - table.table = FormattedTable { + table.table = Arc::new(FormattedTable { headers: vec![ vec![ FormattedTextFragment::plain_text("a"), @@ -1402,7 +1402,7 @@ fn test_link_at_offset_uses_cached_cell_links() { vec![FormattedTextFragment::plain_text("ccc")], vec![FormattedTextFragment::plain_text("ddd")], ]], - }; + }); table.cell_links = vec![ vec![ vec![ParsedUrl::new(1..3, "https://warp.dev".into())],