cursive icon indicating copy to clipboard operation
cursive copied to clipboard

[BUG] TextView performance regression with LinearLayout

Open night-crawler opened this issue 2 years ago • 1 comments

Describe the bug compute_rows() method is called multiple times. In views with thousands of rows it causes significant performance issues. This issue is a twin of https://github.com/gyscos/cursive/issues/582 (EDIT: one more: 698)

To Reproduce The easiest way to reproduce is here: https://github.com/night-crawler/cursive-lazy-text-view/blob/main/src/main.rs (You can clone the full sample)

use cursive::direction::Orientation;
use cursive::theme::ColorStyle;
use cursive::traits::{Nameable, Resizable, Scrollable};
use cursive::utils::markup::StyledString;
use cursive::view::ScrollStrategy;
use cursive::views::{Checkbox, EditView, LinearLayout, Panel, ResizedView};

// use crate::ltv::TextView;
use cursive::views::TextView;

fn mk_tv() -> ResizedView<TextView> {
    let line = "Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.";
    let mut tv = TextView::new("");

    for _ in 0..100 {
        for color in [ColorStyle::primary(), ColorStyle::secondary(), ColorStyle::highlight_inactive()].iter() {
            tv.append(StyledString::styled(line, *color));
        }
    }

    tv.full_screen()
}

fn main() {
    let mut siv = cursive::default();

    let mut main_layout = LinearLayout::new(Orientation::Vertical);
    let mut filter_layout = LinearLayout::new(Orientation::Horizontal);

    let filter_edit_view = EditView::new();
    let since_minutes_edit_view = EditView::new();
    let filter_tail_lines_edit_view = EditView::new();

    let cb_timestamps = Checkbox::new()
        .checked()
        .with_name("cb1");

    let cb_previous = Checkbox::new()
        .checked()
        .with_name("prev");

    let filter_edit_view_panel = Panel::new(filter_edit_view).title("Search").full_width();
    let since_minutes_panel = Panel::new(since_minutes_edit_view).title("Since minutes");
    let filter_tail_lines_panel = Panel::new(filter_tail_lines_edit_view).title("Tail lines");
    let cb_timestamps_panel = Panel::new(cb_timestamps).title("Timestamps");
    let cb_previous_panel = Panel::new(cb_previous).title("Previous");

    filter_layout.add_child(filter_edit_view_panel);
    filter_layout.add_child(cb_timestamps_panel);
    filter_layout.add_child(cb_previous_panel);
    filter_layout.add_child(since_minutes_panel);
    filter_layout.add_child(filter_tail_lines_panel);

    main_layout.add_child(filter_layout.full_width());

    let tv = mk_tv().scrollable()
        .scroll_x(true)
        .scroll_y(true)
        .scroll_strategy(ScrollStrategy::StickToBottom);
    main_layout.add_child(tv);

    let panel = Panel::new(main_layout)
        .title("Sample")
        .with_name("Sample");

    siv.add_layer(panel);
    siv.run();
}

Expected behavior Should be redraw rows without a need / that often.

Screenshots image

Environment

  • Operating system used: Linux 6.0.5-zen3-xanmod1-1
  • Backend used: termion
  • Current locale (run locale in a terminal): LANG=en_US.UTF-8
  • Cursive version: 0.20

Additional context

When cached, it obviously becomes much faster (https://github.com/night-crawler/cursive-lazy-text-view/blob/main/src/ltv.rs#L378). If such a caching is acceptable, I can open a PR, or implement a better idea and open a PR.

night-crawler avatar Apr 17 '23 22:04 night-crawler

Hi, and thanks for the report!

I know about the performance issues of LinearLayout, and am still not sure how to fix that. Caching helps a bit, but as the nesting of LinearLayout gets deeper, you start to need exponentially more cache values :(

I'm not sure yet how to get a one-pass layout logic to avoid this. Might have to redesign a bit the entire layout system to surface more information. :^S

In the meantime, I'd be fine with some caching to improve the most common use-cases.

gyscos avatar Jul 07 '23 14:07 gyscos