pogmommy/zellij
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(performance): plug memory leak (#2675)

Browse source
This commit is contained in:
Aram Drevekenin 2023-08-04 10:22:46 +02:00 • committed by GitHub
parent f75033e1c3
commit 0fc1939c6c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 42 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
zellij-server/src/output/mod.rs
View file

@ -874,7 +874,7 @@ impl OutputBuffer {
y_offset: usize, y_offset: usize,
) -> Vec<CharacterChunk> { ) -> Vec<CharacterChunk> {
if self.should_update_all_lines { if self.should_update_all_lines {
let mut changed_chunks = Vec::with_capacity(viewport.len()); let mut changed_chunks = Vec::new();
for line_index in 0..viewport_height { for line_index in 0..viewport_height {
let terminal_characters = let terminal_characters =
self.extract_line_from_viewport(line_index, viewport, viewport_width); self.extract_line_from_viewport(line_index, viewport, viewport_width);
@ -888,7 +888,7 @@ impl OutputBuffer {
let mut line_changes = self.changed_lines.to_vec(); let mut line_changes = self.changed_lines.to_vec();
line_changes.sort_unstable(); line_changes.sort_unstable();
line_changes.dedup(); line_changes.dedup();
let mut changed_chunks = Vec::with_capacity(line_changes.len()); let mut changed_chunks = Vec::new();
for line_index in line_changes { for line_index in line_changes {
let terminal_characters = let terminal_characters =
self.extract_line_from_viewport(line_index, viewport, viewport_width); self.extract_line_from_viewport(line_index, viewport, viewport_width);

80
zellij-server/src/panes/grid.rs
View file

@ -135,8 +135,8 @@ fn transfer_rows_from_lines_above_to_viewport(
lines_added_to_viewport -= top_non_canonical_rows_in_dst.len() as isize; lines_added_to_viewport -= top_non_canonical_rows_in_dst.len() as isize;
next_lines.push(next_line); next_lines.push(next_line);
next_lines.append(&mut top_non_canonical_rows_in_dst); next_lines.append(&mut top_non_canonical_rows_in_dst);
next_lines = Row::from_rows(next_lines, max_viewport_width) next_lines =
.split_to_rows_of_length(max_viewport_width); Row::from_rows(next_lines).split_to_rows_of_length(max_viewport_width);
if next_lines.is_empty() { if next_lines.is_empty() {
// no more lines at lines_above, the line we popped was probably empty // no more lines at lines_above, the line we popped was probably empty
break; break;
@ -149,7 +149,7 @@ fn transfer_rows_from_lines_above_to_viewport(
lines_added_to_viewport += 1; lines_added_to_viewport += 1;
} }
if !next_lines.is_empty() { if !next_lines.is_empty() {
let excess_row = Row::from_rows(next_lines, 0); let excess_row = Row::from_rows(next_lines);
bounded_push(lines_above, sixel_grid, excess_row); bounded_push(lines_above, sixel_grid, excess_row);
} }
match usize::try_from(lines_added_to_viewport) { match usize::try_from(lines_added_to_viewport) {
@ -179,7 +179,7 @@ fn transfer_rows_from_viewport_to_lines_above(
next_lines.append(&mut bottom_canonical_row_and_wraps_in_dst); next_lines.append(&mut bottom_canonical_row_and_wraps_in_dst);
} }
next_lines.push(next_line); next_lines.push(next_line);
next_lines = vec![Row::from_rows(next_lines, 0)]; next_lines = vec![Row::from_rows(next_lines)];
} else { } else {
break; // no more rows break; // no more rows
} }
@ -191,8 +191,7 @@ fn transfer_rows_from_viewport_to_lines_above(
} }
} }
if !next_lines.is_empty() { if !next_lines.is_empty() {
let excess_rows = Row::from_rows(next_lines, max_viewport_width) let excess_rows = Row::from_rows(next_lines).split_to_rows_of_length(max_viewport_width);
.split_to_rows_of_length(max_viewport_width);
for row in excess_rows { for row in excess_rows {
viewport.insert(0, row); viewport.insert(0, row);
} }
@ -217,12 +216,12 @@ fn transfer_rows_from_lines_below_to_viewport(
let mut canonical_line = get_viewport_bottom_canonical_row_and_wraps(viewport); let mut canonical_line = get_viewport_bottom_canonical_row_and_wraps(viewport);
lines_pulled_from_viewport += canonical_line.len(); lines_pulled_from_viewport += canonical_line.len();
canonical_line.append(&mut top_non_canonical_rows_in_lines_below); canonical_line.append(&mut top_non_canonical_rows_in_lines_below);
next_lines = Row::from_rows(canonical_line, max_viewport_width) next_lines =
.split_to_rows_of_length(max_viewport_width); Row::from_rows(canonical_line).split_to_rows_of_length(max_viewport_width);
} else { } else {
let canonical_row = get_top_canonical_row_and_wraps(lines_below); let canonical_row = get_top_canonical_row_and_wraps(lines_below);
next_lines = Row::from_rows(canonical_row, max_viewport_width) next_lines =
.split_to_rows_of_length(max_viewport_width); Row::from_rows(canonical_row).split_to_rows_of_length(max_viewport_width);
} }
} else { } else {
break; // no more rows break; // no more rows
@ -235,7 +234,7 @@ fn transfer_rows_from_lines_below_to_viewport(
} }
} }
if !next_lines.is_empty() { if !next_lines.is_empty() {
let excess_row = Row::from_rows(next_lines, 0); let excess_row = Row::from_rows(next_lines);
lines_below.insert(0, excess_row); lines_below.insert(0, excess_row);
} }
} }
@ -404,7 +403,7 @@ impl Debug for Grid {
let mut buffer: Vec<Row> = self.viewport.clone(); let mut buffer: Vec<Row> = self.viewport.clone();
// pad buffer // pad buffer
for _ in buffer.len()..self.height { for _ in buffer.len()..self.height {
buffer.push(Row::new(self.width).canonical()); buffer.push(Row::new().canonical());
} }
// display sixel placeholder // display sixel placeholder
@ -461,13 +460,14 @@ impl Grid {
debug: bool, debug: bool,
) -> Self { ) -> Self {
let sixel_grid = SixelGrid::new(character_cell_size.clone(), sixel_image_store); let sixel_grid = SixelGrid::new(character_cell_size.clone(), sixel_image_store);
// make sure this is initialized as it is used internally
// if it was already initialized (which should happen normally unless this is a test or
// something changed since this comment was written), we get an Error which we ignore
// I don't know why this needs to be a OneCell, but whatevs
let _ = SCROLL_BUFFER_SIZE.set(DEFAULT_SCROLL_BUFFER_SIZE);
Grid { Grid {
lines_above: VecDeque::with_capacity( lines_above: VecDeque::new(),
// .get_or_init() is used instead of .get().unwrap() to prevent viewport: vec![Row::new().canonical()],
// unit tests from panicking where SCROLL_BUFFER_SIZE is uninitialized.
*SCROLL_BUFFER_SIZE.get_or_init(|| DEFAULT_SCROLL_BUFFER_SIZE),
),
viewport: vec![Row::new(columns).canonical()],
lines_below: vec![], lines_below: vec![],
horizontal_tabstops: create_horizontal_tabstops(columns), horizontal_tabstops: create_horizontal_tabstops(columns),
cursor: Cursor::new(0, 0), cursor: Cursor::new(0, 0),
@ -823,7 +823,7 @@ impl Grid {
for mut canonical_line in viewport_canonical_lines { for mut canonical_line in viewport_canonical_lines {
let mut canonical_line_parts: Vec<Row> = vec![]; let mut canonical_line_parts: Vec<Row> = vec![];
if canonical_line.columns.is_empty() { if canonical_line.columns.is_empty() {
canonical_line_parts.push(Row::new(new_columns).canonical()); canonical_line_parts.push(Row::new().canonical());
} }
while !canonical_line.columns.is_empty() { while !canonical_line.columns.is_empty() {
let next_wrap = canonical_line.drain_until(new_columns); let next_wrap = canonical_line.drain_until(new_columns);
@ -1248,7 +1248,7 @@ impl Grid {
// FIXME: this should add an empty line with the pad_character // FIXME: this should add an empty line with the pad_character
// but for some reason this breaks rendering in various situations // but for some reason this breaks rendering in various situations
// it needs to be investigated and fixed // it needs to be investigated and fixed
let new_row = Row::new(self.width).canonical(); let new_row = Row::new().canonical();
self.viewport.push(new_row); self.viewport.push(new_row);
} }
if self.cursor.y == self.height.saturating_sub(1) { if self.cursor.y == self.height.saturating_sub(1) {
@ -1314,13 +1314,10 @@ impl Grid {
None => { None => {
// pad lines until cursor if they do not exist // pad lines until cursor if they do not exist
for _ in self.viewport.len()..self.cursor.y { for _ in self.viewport.len()..self.cursor.y {
self.viewport.push(Row::new(self.width).canonical()); self.viewport.push(Row::new().canonical());
} }
self.viewport.push( self.viewport
Row::new(self.width) .push(Row::new().with_character(terminal_character).canonical());
.with_character(terminal_character)
.canonical(),
);
self.output_buffer.update_line(self.cursor.y); self.output_buffer.update_line(self.cursor.y);
}, },
} }
@ -1409,14 +1406,14 @@ impl Grid {
} else { } else {
self.viewport.remove(0); self.viewport.remove(0);
} }
let wrapped_row = Row::new(self.width); let wrapped_row = Row::new();
self.viewport.push(wrapped_row); self.viewport.push(wrapped_row);
self.selection.move_up(1); self.selection.move_up(1);
self.output_buffer.update_all_lines(); self.output_buffer.update_all_lines();
} else { } else {
self.cursor.y += 1; self.cursor.y += 1;
if self.viewport.len() <= self.cursor.y { if self.viewport.len() <= self.cursor.y {
let line_wrapped_row = Row::new(self.width); let line_wrapped_row = Row::new();
self.viewport.push(line_wrapped_row); self.viewport.push(line_wrapped_row);
self.output_buffer.update_line(self.cursor.y); self.output_buffer.update_line(self.cursor.y);
} }
@ -1494,8 +1491,7 @@ impl Grid {
if scroll_region_bottom < self.viewport.len() { if scroll_region_bottom < self.viewport.len() {
self.viewport.remove(scroll_region_bottom); self.viewport.remove(scroll_region_bottom);
} }
self.viewport self.viewport.insert(current_line_index, Row::new()); // TODO: .canonical() ?
.insert(current_line_index, Row::new(self.width)); // TODO: .canonical() ?
} else if current_line_index > scroll_region_top } else if current_line_index > scroll_region_top
&& current_line_index <= scroll_region_bottom && current_line_index <= scroll_region_bottom
{ {
@ -1654,9 +1650,9 @@ impl Grid {
self.should_render = true; self.should_render = true;
} }
pub fn reset_terminal_state(&mut self) { pub fn reset_terminal_state(&mut self) {
self.lines_above = VecDeque::with_capacity(*SCROLL_BUFFER_SIZE.get().unwrap()); self.lines_above = VecDeque::new();
self.lines_below = vec![]; self.lines_below = vec![];
self.viewport = vec![Row::new(self.width).canonical()]; self.viewport = vec![Row::new().canonical()];
self.alternate_screen_state = None; self.alternate_screen_state = None;
self.cursor_key_mode = false; self.cursor_key_mode = false;
self.scroll_region = None; self.scroll_region = None;
@ -2571,14 +2567,10 @@ impl Perform for Grid {
}, },
1049 => { 1049 => {
// enter alternate buffer // enter alternate buffer
let current_lines_above = std::mem::replace( let current_lines_above =
&mut self.lines_above, std::mem::replace(&mut self.lines_above, VecDeque::new());
VecDeque::with_capacity(*SCROLL_BUFFER_SIZE.get().unwrap()), let current_viewport =
); std::mem::replace(&mut self.viewport, vec![Row::new().canonical()]);
let current_viewport = std::mem::replace(
&mut self.viewport,
vec![Row::new(self.width).canonical()],
);
let current_cursor = let current_cursor =
std::mem::replace(&mut self.cursor, Cursor::new(0, 0)); std::mem::replace(&mut self.cursor, Cursor::new(0, 0));
let sixel_image_store = self.sixel_grid.sixel_image_store.clone(); let sixel_image_store = self.sixel_grid.sixel_image_store.clone();
@ -3057,9 +3049,9 @@ impl Debug for Row {
} }
impl Row { impl Row {
pub fn new(width: usize) -> Self { pub fn new() -> Self {
Row { Row {
columns: VecDeque::with_capacity(width), columns: VecDeque::new(),
is_canonical: false, is_canonical: false,
width: None, width: None,
} }
@ -3071,9 +3063,9 @@ impl Row {
width: None, width: None,
} }
} }
pub fn from_rows(mut rows: Vec<Row>, width: usize) -> Self { pub fn from_rows(mut rows: Vec<Row>) -> Self {
if rows.is_empty() { if rows.is_empty() {
Row::new(width) Row::new()
} else { } else {
let mut first_row = rows.remove(0); let mut first_row = rows.remove(0);
for row in &mut rows { for row in &mut rows {

8
zellij-server/src/ui/pane_boundaries_frame.rs
View file

@ -9,7 +9,7 @@ use zellij_utils::pane_size::Viewport;
use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; use unicode_width::{UnicodeWidthChar, UnicodeWidthStr};
fn foreground_color(characters: &str, color: Option<PaletteColor>) -> Vec<TerminalCharacter> { fn foreground_color(characters: &str, color: Option<PaletteColor>) -> Vec<TerminalCharacter> {
let mut colored_string = Vec::with_capacity(characters.chars().count()); let mut colored_string = Vec::new();
for character in characters.chars() { for character in characters.chars() {
let styles = match color { let styles = match color {
Some(palette_color) => { Some(palette_color) => {
@ -36,7 +36,7 @@ fn foreground_color(characters: &str, color: Option<PaletteColor>) -> Vec<Termin
} }
fn background_color(characters: &str, color: Option<PaletteColor>) -> Vec<TerminalCharacter> { fn background_color(characters: &str, color: Option<PaletteColor>) -> Vec<TerminalCharacter> {
let mut colored_string = Vec::with_capacity(characters.chars().count()); let mut colored_string = Vec::new();
for character in characters.chars() { for character in characters.chars() {
let styles = match color { let styles = match color {
Some(palette_color) => { Some(palette_color) => {
@ -367,7 +367,7 @@ impl PaneFrame {
} else { } else {
let length_of_each_half = (max_length - middle_truncated_sign.width()) / 2; let length_of_each_half = (max_length - middle_truncated_sign.width()) / 2;
let mut first_part: String = String::with_capacity(length_of_each_half); let mut first_part: String = String::new();
for char in full_text.chars() { for char in full_text.chars() {
if first_part.width() + char.width().unwrap_or(0) > length_of_each_half { if first_part.width() + char.width().unwrap_or(0) > length_of_each_half {
break; break;
@ -376,7 +376,7 @@ impl PaneFrame {
} }
} }
let mut second_part: String = String::with_capacity(length_of_each_half); let mut second_part: String = String::new();
for char in full_text.chars().rev() { for char in full_text.chars().rev() {
if second_part.width() + char.width().unwrap_or(0) > length_of_each_half { if second_part.width() + char.width().unwrap_or(0) > length_of_each_half {
break; break;