Fix colored row reset performance

This fixes a bug where a row would always get reset completely if its
background does not equal the default terminal background. This leads to
big performance bottlenecks when running commands like `echo "\e[41m" &&
yes`.

Instead of resetting the entire row whenever the template cell is not
empty, the template cell is now compared to the last cell in the row.
The last cell will always be equal to the previous template cell when
`row.occ < row.inner.len()` and if `occ` is equal to the row's length,
the entire row is always reset anyways.

Fixes #2989.
This commit is contained in:
Christian Duerr 2019-12-10 00:35:13 +01:00 committed by GitHub
parent 79b19176ee
commit 36185c4753
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 73 additions and 40 deletions

View File

@ -23,8 +23,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Clipboard escape `OSC 52` not working with empty clipboard parameter - Clipboard escape `OSC 52` not working with empty clipboard parameter
- Direct escape input on Windows using alt - Direct escape input on Windows using alt
- Incorrect window size on X11 when waking up from suspend - Incorrect window size on X11 when waking up from suspend
- Incorrect width of Unicode 11/12 emojis - Width of Unicode 11/12 emojis
- Minimize on windows causing layout issues - Minimize on windows causing layout issues
- Performance bottleneck when clearing colored rows
## 0.4.0 ## 0.4.0

View File

@ -70,6 +70,12 @@ pub trait GridCell {
fn is_empty(&self) -> bool; fn is_empty(&self) -> bool;
fn is_wrap(&self) -> bool; fn is_wrap(&self) -> bool;
fn set_wrap(&mut self, wrap: bool); fn set_wrap(&mut self, wrap: bool);
/// Fast equality approximation.
///
/// This is a faster alternative to [`PartialEq`],
/// but might report inequal cells as equal.
fn fast_eq(&self, other: Self) -> bool;
} }
/// Represents the terminal display contents /// Represents the terminal display contents
@ -140,7 +146,7 @@ pub enum Scroll {
Bottom, Bottom,
} }
impl<T: GridCell + Copy + Clone> Grid<T> { impl<T: GridCell + PartialEq + Copy> Grid<T> {
pub fn new(lines: index::Line, cols: index::Column, scrollback: usize, template: T) -> Grid<T> { pub fn new(lines: index::Line, cols: index::Column, scrollback: usize, template: T) -> Grid<T> {
let raw = Storage::with_capacity(lines, Row::new(cols, &template)); let raw = Storage::with_capacity(lines, Row::new(cols, &template));
Grid { Grid {

View File

@ -29,14 +29,10 @@ use crate::index::Column;
pub struct Row<T> { pub struct Row<T> {
inner: Vec<T>, inner: Vec<T>,
/// occupied entries /// Maximum number of occupied entries.
/// ///
/// Semantically, this value can be understood as the **end** of an /// This is the upper bound on the number of elements in the row, which have been modified
/// Exclusive Range. Thus, /// since the last reset. All cells after this point are guaranteed to be equal.
///
/// - Zero means there are no occupied entries
/// - 1 means there is a value at index zero, but nowhere else
/// - `occ == inner.len` means every value is occupied
pub(crate) occ: usize, pub(crate) occ: usize,
} }
@ -85,21 +81,29 @@ impl<T: Copy> Row<T> {
} }
} }
/// Resets contents to the contents of `other` /// Reset all cells in the row to the `template` cell.
#[inline]
pub fn reset(&mut self, template: &T) pub fn reset(&mut self, template: &T)
where where
T: GridCell, T: GridCell + PartialEq,
{ {
if template.is_empty() { debug_assert!(!self.inner.is_empty());
for item in &mut self.inner[..self.occ] {
*item = *template; let template = *template;
}
self.occ = 0; // Mark all cells as dirty if template cell changed
} else { let len = self.inner.len();
let len = self.inner.len(); if !self.inner[len - 1].fast_eq(template) {
self.inner = vec![*template; len];
self.occ = len; self.occ = len;
} }
// Reset every dirty in the row
// let template = *template;
for item in &mut self.inner[..self.occ] {
*item = template;
}
self.occ = 0;
} }
} }

View File

@ -1,4 +1,4 @@
use std::cmp::Ordering; use std::cmp::{Ordering, PartialEq};
use std::ops::{Index, IndexMut}; use std::ops::{Index, IndexMut};
use std::vec::Drain; use std::vec::Drain;
@ -50,7 +50,7 @@ pub struct Storage<T> {
len: usize, len: usize,
} }
impl<T: PartialEq> ::std::cmp::PartialEq for Storage<T> { impl<T: PartialEq> PartialEq for Storage<T> {
fn eq(&self, other: &Self) -> bool { fn eq(&self, other: &Self) -> bool {
// Make sure length is equal // Make sure length is equal
if self.inner.len() != other.inner.len() { if self.inner.len() != other.inner.len() {
@ -62,9 +62,8 @@ impl<T: PartialEq> ::std::cmp::PartialEq for Storage<T> {
if self.zero >= other.zero { (self, other) } else { (other, self) }; if self.zero >= other.zero { (self, other) } else { (other, self) };
// Calculate the actual zero offset // Calculate the actual zero offset
let len = self.inner.len(); let bigger_zero = bigger.zero;
let bigger_zero = bigger.zero % len; let smaller_zero = smaller.zero;
let smaller_zero = smaller.zero % len;
// Compare the slices in chunks // Compare the slices in chunks
// Chunks: // Chunks:
@ -79,6 +78,7 @@ impl<T: PartialEq> ::std::cmp::PartialEq for Storage<T> {
// Smaller Zero (3): // Smaller Zero (3):
// 7 8 9 | 0 1 2 3 | 4 5 6 // 7 8 9 | 0 1 2 3 | 4 5 6
// C3 C3 C3 | C1 C1 C1 C1 | C2 C2 C2 // C3 C3 C3 | C1 C1 C1 C1 | C2 C2 C2
let len = self.inner.len();
bigger.inner[bigger_zero..] bigger.inner[bigger_zero..]
== smaller.inner[smaller_zero..smaller_zero + (len - bigger_zero)] == smaller.inner[smaller_zero..smaller_zero + (len - bigger_zero)]
&& bigger.inner[..bigger_zero - smaller_zero] && bigger.inner[..bigger_zero - smaller_zero]
@ -149,7 +149,7 @@ impl<T> Storage<T> {
} }
// Update raw buffer length and zero offset // Update raw buffer length and zero offset
self.zero = (self.zero + new_growage) % self.inner.len(); self.zero += new_growage;
self.len += growage; self.len += growage;
} }
@ -201,19 +201,11 @@ impl<T> Storage<T> {
self.len self.len
} }
#[inline]
/// Compute actual index in underlying storage given the requested index. /// Compute actual index in underlying storage given the requested index.
#[inline]
fn compute_index(&self, requested: usize) -> usize { fn compute_index(&self, requested: usize) -> usize {
debug_assert!(requested < self.len); debug_assert!(requested < self.len);
let zeroed = requested + self.zero; self.wrap_index(self.zero + requested)
// This part is critical for performance,
// so an if/else is used here instead of a moludo operation
if zeroed >= self.inner.len() {
zeroed - self.inner.len()
} else {
zeroed
}
} }
pub fn swap_lines(&mut self, a: Line, b: Line) { pub fn swap_lines(&mut self, a: Line, b: Line) {
@ -256,31 +248,48 @@ impl<T> Storage<T> {
} }
} }
/// Rotate the grid, moving all lines up/down in history.
#[inline] #[inline]
pub fn rotate(&mut self, count: isize) { pub fn rotate(&mut self, count: isize) {
debug_assert!(count.abs() as usize <= self.inner.len()); debug_assert!(count.abs() as usize <= self.inner.len());
let len = self.inner.len(); let len = self.inner.len();
self.zero = (self.zero as isize + count + len as isize) as usize % len; self.zero = self.wrap_index((self.zero as isize + count + len as isize) as usize);
} }
// Fast path /// Rotate the grid up, moving all existing lines down in history.
///
/// This is a faster, specialized version of [`rotate`].
#[inline] #[inline]
pub fn rotate_up(&mut self, count: usize) { pub fn rotate_up(&mut self, count: usize) {
self.zero = (self.zero + count) % self.inner.len(); self.zero = self.wrap_index(self.zero + count);
} }
/// Drain all rows in the grid.
pub fn drain(&mut self) -> Drain<'_, Row<T>> { pub fn drain(&mut self) -> Drain<'_, Row<T>> {
self.truncate(); self.truncate();
self.inner.drain(..) self.inner.drain(..)
} }
/// Update the raw storage buffer /// Update the raw storage buffer.
pub fn replace_inner(&mut self, vec: Vec<Row<T>>) { pub fn replace_inner(&mut self, vec: Vec<Row<T>>) {
self.len = vec.len(); self.len = vec.len();
self.inner = vec; self.inner = vec;
self.zero = 0; self.zero = 0;
} }
/// Wrap index to be within the inner buffer.
///
/// This uses if/else instead of the remainder to improve performance,
/// so the assumption is made that `index < self.inner.len() * 2`.
#[inline]
fn wrap_index(&self, index: usize) -> usize {
if index >= self.inner.len() {
index - self.inner.len()
} else {
index
}
}
} }
impl<T> Index<usize> for Storage<T> { impl<T> Index<usize> for Storage<T> {
@ -335,6 +344,10 @@ mod test {
} }
fn set_wrap(&mut self, _wrap: bool) {} fn set_wrap(&mut self, _wrap: bool) {}
fn fast_eq(&self, other: Self) -> bool {
self == &other
}
} }
#[test] #[test]

View File

@ -29,6 +29,10 @@ impl GridCell for usize {
} }
fn set_wrap(&mut self, _wrap: bool) {} fn set_wrap(&mut self, _wrap: bool) {}
fn fast_eq(&self, other: Self) -> bool {
self == &other
}
} }
// Scroll up moves lines upwards // Scroll up moves lines upwards

View File

@ -85,6 +85,11 @@ impl GridCell for Cell {
self.flags.remove(Flags::WRAPLINE); self.flags.remove(Flags::WRAPLINE);
} }
} }
#[inline]
fn fast_eq(&self, other: Self) -> bool {
self.bg == other.bg
}
} }
/// Get the length of occupied cells in a line /// Get the length of occupied cells in a line

File diff suppressed because one or more lines are too long