From 1689f4208f9e119e3e2b8c904251515fea24dac2 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Fri, 24 Jan 2025 18:59:01 +0800 Subject: [PATCH] table: Add measure to table. (#577) - Improve to not calc extra rows if `stripe` not enable. - Improve to not change offset, when scrollbar drag changed less than 1px. - Improve render cells to use `Vec::with_capacity` to prepare enough space. ```bash ZED_MEASUREMENTS=1 MTL_HUD_ENABLED=1 cargo run --release --example table ``` --- crates/story/src/list_story.rs | 45 ++++++---- crates/story/src/table_story.rs | 11 ++- crates/ui/src/lib.rs | 5 ++ crates/ui/src/scroll/scrollbar.rs | 8 +- crates/ui/src/table.rs | 138 ++++++++++++++++++++++-------- 5 files changed, 149 insertions(+), 58 deletions(-) diff --git a/crates/story/src/list_story.rs b/crates/story/src/list_story.rs index ffd7a00c..36ebbb8b 100644 --- a/crates/story/src/list_story.rs +++ b/crates/story/src/list_story.rs @@ -4,8 +4,8 @@ use std::time::Duration; use fake::Fake; use gpui::{ actions, div, px, AppContext, ElementId, FocusHandle, FocusableView, InteractiveElement, - IntoElement, ParentElement, Render, RenderOnce, Styled, Subscription, Task, Timer, View, - ViewContext, VisualContext, WindowContext, + IntoElement, ParentElement, Render, RenderOnce, SharedString, Styled, Subscription, Task, + Timer, View, ViewContext, VisualContext, WindowContext, }; use ui::{ @@ -19,22 +19,31 @@ use ui::{ actions!(list_story, [SelectedCompany]); -#[derive(Clone)] +#[derive(Clone, Default)] struct Company { - name: String, - industry: String, + name: SharedString, + industry: SharedString, last_done: f64, prev_close: f64, + + change_percent: f64, + change_percent_str: SharedString, + last_done_str: SharedString, + prev_close_str: SharedString, // description: String, } impl Company { - fn random_update(&mut self) { - self.last_done = self.prev_close * (1.0 + (-0.2..0.2).fake::()); + fn prepare(mut self) -> Self { + self.change_percent = (self.last_done - self.prev_close) / self.prev_close; + self.change_percent_str = format!("{:.2}%", self.change_percent).into(); + self.last_done_str = format!("{:.2}", self.last_done).into(); + self.prev_close_str = format!("{:.2}", self.prev_close).into(); + self } - fn change_percent(&self) -> f64 { - (self.last_done - self.prev_close) / self.prev_close + fn random_update(&mut self) { + self.last_done = self.prev_close * (1.0 + (-0.2..0.2).fake::()); } } @@ -65,7 +74,7 @@ impl RenderOnce for CompanyListItem { cx.theme().foreground }; - let trend_color = match self.company.change_percent() { + let trend_color = match self.company.change_percent { change if change > 0.0 => hsl(0.0, 79.0, 53.0), change if change < 0.0 => hsl(100.0, 79.0, 53.0), _ => cx.theme().foreground, @@ -114,7 +123,7 @@ impl RenderOnce for CompanyListItem { div() .w(px(65.)) .text_color(text_color) - .child(format!("{:.2}", self.company.last_done)), + .child(self.company.last_done_str.clone()), ) .child( h_flex().w(px(65.)).justify_end().child( @@ -124,7 +133,7 @@ impl RenderOnce for CompanyListItem { .text_size(px(12.)) .px_1() .text_color(trend_color) - .child(format!("{:.2}%", self.company.change_percent())), + .child(self.company.change_percent_str.clone()), ), ), ), @@ -193,8 +202,6 @@ impl ListDelegate for CompanyListDelegate { } fn load_more(&mut self, cx: &mut ViewContext>) { - self.loading = true; - cx.spawn(|view, mut cx| async move { // Simulate network request, delay 1s to load data. Timer::after(Duration::from_secs(1)).await; @@ -206,7 +213,6 @@ impl ListDelegate for CompanyListDelegate { .companies .extend((0..200).map(|_| random_company())); _ = view.delegate_mut().perform_search(&query, cx); - view.delegate_mut().loading = false; view.delegate_mut().is_eof = view.delegate().companies.len() >= 6000; }); }) @@ -325,12 +331,17 @@ impl ListStory { fn random_company() -> Company { let last_done = (0.0..999.0).fake::(); let prev_close = last_done * (-0.1..0.1).fake::(); + Company { - name: fake::faker::company::en::CompanyName().fake(), - industry: fake::faker::company::en::Industry().fake(), + name: fake::faker::company::en::CompanyName() + .fake::() + .into(), + industry: fake::faker::company::en::Industry().fake::().into(), last_done, prev_close, + ..Default::default() } + .prepare() } impl FocusableView for ListStory { diff --git a/crates/story/src/table_story.rs b/crates/story/src/table_story.rs index b3d1b01e..c1561157 100644 --- a/crates/story/src/table_story.rs +++ b/crates/story/src/table_story.rs @@ -373,6 +373,14 @@ impl TableDelegate for StockTableDelegate { .menu("Size XSmall", Box::new(ChangeSize(Size::XSmall))) } + /// NOTE: Performance metrics + /// + /// last render 561 cells total: 232.745µs, avg: 414ns + /// frame duration: 8.825083ms + /// + /// This is means render the full table cells takes 232.745µs. Then 232.745µs / 8.82ms = 2.6% of the frame duration. + /// + /// If we improve the td rendering, we can reduce the time to render the full table cells. fn render_td( &self, row_ix: usize, @@ -578,8 +586,7 @@ impl TableStory { // Spawn a background to random refresh the list cx.spawn(move |this, mut cx| async move { loop { - let delay = (80..150).fake::(); - Timer::after(time::Duration::from_millis(delay)).await; + Timer::after(time::Duration::from_millis(33)).await; this.update(&mut cx, |this, cx| { if !this.refresh_data { diff --git a/crates/ui/src/lib.rs b/crates/ui/src/lib.rs index a6cdc60b..de505ff5 100644 --- a/crates/ui/src/lib.rs +++ b/crates/ui/src/lib.rs @@ -107,3 +107,8 @@ pub fn locale() -> impl Deref { pub fn set_locale(locale: &str) { rust_i18n::set_locale(locale) } + +#[inline] +pub(crate) fn measure_enable() -> bool { + std::env::var("ZED_MEASUREMENTS").is_ok() +} diff --git a/crates/ui/src/scroll/scrollbar.rs b/crates/ui/src/scroll/scrollbar.rs index ec471a5e..2aa9a449 100644 --- a/crates/ui/src/scroll/scrollbar.rs +++ b/crates/ui/src/scroll/scrollbar.rs @@ -760,8 +760,12 @@ impl Element for Scrollbar { ) }; - scroll_handle.set_offset(offset); - cx.notify(Some(view_id)); + if (scroll_handle.offset().y - offset.y).abs() > px(1.) + || (scroll_handle.offset().x - offset.x).abs() > px(1.) + { + scroll_handle.set_offset(offset); + cx.notify(Some(view_id)); + } } } }); diff --git a/crates/ui/src/table.rs b/crates/ui/src/table.rs index 5a5f51c3..88cc432b 100644 --- a/crates/ui/src/table.rs +++ b/crates/ui/src/table.rs @@ -1,4 +1,4 @@ -use std::{cell::Cell, ops::Range, rc::Rc}; +use std::{cell::Cell, ops::Range, rc::Rc, time::Duration}; use crate::{ context_menu::ContextMenuExt, @@ -172,6 +172,7 @@ pub struct Table { /// The visible range of the rows and columns. visible_range: VisibleRangeState, + _measure: Vec, _load_more_task: Task<()>, } @@ -374,6 +375,7 @@ where scrollbar_visible: Edges::all(true), visible_range: VisibleRangeState::default(), _load_more_task: Task::ready(()), + _measure: Vec::new(), }; this.prepare_col_groups(cx); @@ -1140,12 +1142,20 @@ where .h_full() .border_r_1() .border_color(cx.theme().table_row_border) - .children((0..left_cols_count).map(|col_ix| { - self.render_col_wrap(col_ix, cx).child( - self.render_cell(col_ix, cx) - .child(self.delegate.render_td(row_ix, col_ix, cx)), - ) - })), + .children({ + let mut items = Vec::with_capacity(left_cols_count); + + (0..left_cols_count).for_each(|col_ix| { + items.push( + self.render_col_wrap(col_ix, cx).child( + self.render_cell(col_ix, cx) + .child(self.measure_render_td(row_ix, col_ix, cx)), + ), + ); + }); + + items + }), ) } else { None @@ -1170,18 +1180,22 @@ where cx, ); - visible_range - .map(|col_ix| { - let col_ix = col_ix + left_cols_count; - table.render_col_wrap(col_ix, cx).child( - table.render_cell(col_ix, cx).child( - table - .delegate - .render_td(row_ix, col_ix, cx), - ), - ) - }) - .collect::>() + let mut items = Vec::with_capacity( + visible_range.end - visible_range.start, + ); + + visible_range.for_each(|col_ix| { + let col_ix = col_ix + left_cols_count; + let el = table.render_col_wrap(col_ix, cx).child( + table.render_cell(col_ix, cx).child( + table.measure_render_td(row_ix, col_ix, cx), + ), + ); + + items.push(el); + }); + + items } }, ) @@ -1250,6 +1264,48 @@ where .child(self.delegate.render_last_empty_col(cx)) } } + + #[inline] + fn measure_render_td( + &mut self, + row_ix: usize, + col_ix: usize, + cx: &mut ViewContext, + ) -> impl IntoElement { + if !crate::measure_enable() { + return self + .delegate + .render_td(row_ix, col_ix, cx) + .into_any_element(); + } + + let start = std::time::Instant::now(); + let el = self.delegate.render_td(row_ix, col_ix, cx); + self._measure.push(start.elapsed()); + el.into_any_element() + } + + fn measure(&mut self, _: &mut ViewContext) { + if !crate::measure_enable() { + return; + } + + // Print avg measure time of each td + if self._measure.len() > 0 { + let total = self + ._measure + .iter() + .fold(Duration::default(), |acc, d| acc + *d); + let avg = total / self._measure.len() as u32; + eprintln!( + "last render {} cells total: {:?}, avg: {:?}", + self._measure.len(), + total, + avg, + ); + } + self._measure.clear(); + } } impl Sizable for Table @@ -1276,6 +1332,8 @@ where D: TableDelegate, { fn render(&mut self, cx: &mut ViewContext) -> impl IntoElement { + self.measure(cx); + let view = cx.view().clone(); let vertical_scroll_handle = self.vertical_scroll_handle.clone(); let horizontal_scroll_handle = self.horizontal_scroll_handle.clone(); @@ -1301,12 +1359,14 @@ where // Calculate the extra rows needed to fill the table for stripe style. let mut extra_rows_needed = 0; - if let Some(row_height) = row_height { - if row_height > px(0.) { - let actual_height = row_height * rows_count as f32; - let remaining_height = total_height - actual_height; - if remaining_height > px(0.) { - extra_rows_needed = (remaining_height / row_height).ceil() as usize; + if self.stripe { + if let Some(row_height) = row_height { + if row_height > px(0.) { + let actual_height = row_height * rows_count as f32; + let remaining_height = total_height - actual_height; + if remaining_height > px(0.) { + extra_rows_needed = (remaining_height / row_height).ceil() as usize; + } } } } @@ -1359,19 +1419,23 @@ where ); } + let mut items = Vec::with_capacity( + visible_range.end - visible_range.start, + ); + // Render fake rows to fill the table - visible_range - .map(|row_ix| { - // Render real rows for available data - table.render_table_row( - row_ix, - rows_count, - left_cols_count, - cols_count, - cx, - ) - }) - .collect::>() + visible_range.for_each(|row_ix| { + // Render real rows for available data + items.push(table.render_table_row( + row_ix, + rows_count, + left_cols_count, + cols_count, + cx, + )); + }); + + items } }, )