From 313ac9f41474222133b344c1c0d8827b4dc2fa50 Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Mon, 12 Apr 2021 16:00:05 +0200 Subject: [PATCH] fix(performance): remove unnecessary allocations from pty (#264) * work * refactor(pty): removed unused code * style(comment): remove unused * style(fmt): rustfmt --- src/client/panes/plugin_pane.rs | 4 +- src/client/panes/terminal_pane.rs | 35 ++---------- src/client/tab.rs | 8 +-- src/common/errors.rs | 4 +- src/common/mod.rs | 6 +- src/common/pty_bus.rs | 92 ++----------------------------- src/common/screen.rs | 4 +- src/main.rs | 1 - 8 files changed, 23 insertions(+), 131 deletions(-) diff --git a/src/client/panes/plugin_pane.rs b/src/client/panes/plugin_pane.rs index caf49acc..49cf2aee 100644 --- a/src/client/panes/plugin_pane.rs +++ b/src/client/panes/plugin_pane.rs @@ -1,6 +1,6 @@ #![allow(clippy::clippy::if_same_then_else)] -use crate::{common::SenderWithContext, pty_bus::VteEvent, tab::Pane, wasm_vm::PluginInstruction}; +use crate::{common::SenderWithContext, pty_bus::VteBytes, tab::Pane, wasm_vm::PluginInstruction}; use std::{sync::mpsc::channel, unimplemented}; @@ -79,7 +79,7 @@ impl Pane for PluginPane { self.position_and_size_override = Some(position_and_size_override); self.should_render = true; } - fn handle_event(&mut self, _event: VteEvent) { + fn handle_pty_bytes(&mut self, _event: VteBytes) { unimplemented!() } fn cursor_coordinates(&self) -> Option<(usize, usize)> { diff --git a/src/client/panes/terminal_pane.rs b/src/client/panes/terminal_pane.rs index 0702863a..d2473ba3 100644 --- a/src/client/panes/terminal_pane.rs +++ b/src/client/panes/terminal_pane.rs @@ -3,15 +3,14 @@ use crate::tab::Pane; use ::nix::pty::Winsize; use ::std::os::unix::io::RawFd; -use ::vte::Perform; use std::fmt::Debug; use crate::panes::grid::Grid; use crate::panes::terminal_character::{ CharacterStyles, TerminalCharacter, EMPTY_TERMINAL_CHARACTER, }; +use crate::pty_bus::VteBytes; use crate::utils::logging::debug_log_to_file; -use crate::VteEvent; #[derive(PartialEq, Eq, Ord, PartialOrd, Hash, Clone, Copy, Debug)] pub enum PaneId { @@ -86,34 +85,10 @@ impl Pane for TerminalPane { self.position_and_size_override = Some(position_and_size_override); self.reflow_lines(); } - fn handle_event(&mut self, event: VteEvent) { - match event { - VteEvent::Print(c) => { - self.print(c); - self.mark_for_rerender(); - } - VteEvent::Execute(byte) => { - self.execute(byte); - } - VteEvent::Hook(params, intermediates, ignore, c) => { - self.hook(¶ms, &intermediates, ignore, c); - } - VteEvent::Put(byte) => { - self.put(byte); - } - VteEvent::Unhook => { - self.unhook(); - } - VteEvent::OscDispatch(params, bell_terminated) => { - let params: Vec<&[u8]> = params.iter().map(|p| &p[..]).collect(); - self.osc_dispatch(¶ms[..], bell_terminated); - } - VteEvent::CsiDispatch(params, intermediates, ignore, c) => { - self.csi_dispatch(¶ms, &intermediates, ignore, c); - } - VteEvent::EscDispatch(intermediates, ignore, byte) => { - self.esc_dispatch(&intermediates, ignore, byte); - } + fn handle_pty_bytes(&mut self, bytes: VteBytes) { + let mut vte_parser = vte::Parser::new(); + for byte in bytes.iter() { + vte_parser.advance(self, *byte); } } fn cursor_coordinates(&self) -> Option<(usize, usize)> { diff --git a/src/client/tab.rs b/src/client/tab.rs index 3a12656d..9cafdcec 100644 --- a/src/client/tab.rs +++ b/src/client/tab.rs @@ -7,7 +7,7 @@ use crate::common::{input::handler::parse_keys, AppInstruction, SenderWithContex use crate::layout::Layout; use crate::os_input_output::OsApi; use crate::panes::{PaneId, PositionAndSize, TerminalPane}; -use crate::pty_bus::{PtyInstruction, VteEvent}; +use crate::pty_bus::{PtyInstruction, VteBytes}; use crate::utils::shared::adjust_to_size; use crate::wasm_vm::PluginInstruction; use crate::{boundaries::Boundaries, panes::PluginPane}; @@ -91,7 +91,7 @@ pub trait Pane { fn reset_size_and_position_override(&mut self); fn change_pos_and_size(&mut self, position_and_size: &PositionAndSize); fn override_size_and_position(&mut self, x: usize, y: usize, size: &PositionAndSize); - fn handle_event(&mut self, event: VteEvent); + fn handle_pty_bytes(&mut self, bytes: VteBytes); fn cursor_coordinates(&self) -> Option<(usize, usize)>; fn adjust_input_to_terminal(&self, input_bytes: Vec) -> Vec; @@ -564,14 +564,14 @@ impl Tab { pub fn has_terminal_pid(&self, pid: RawFd) -> bool { self.panes.contains_key(&PaneId::Terminal(pid)) } - pub fn handle_pty_event(&mut self, pid: RawFd, event: VteEvent) { + pub fn handle_pty_bytes(&mut self, pid: RawFd, bytes: VteBytes) { // if we don't have the terminal in self.terminals it's probably because // of a race condition where the terminal was created in pty_bus but has not // yet been created in Screen. These events are currently not buffered, so // if you're debugging seemingly randomly missing stdout data, this is // the reason if let Some(terminal_output) = self.panes.get_mut(&PaneId::Terminal(pid)) { - terminal_output.handle_event(event); + terminal_output.handle_pty_bytes(bytes); } } pub fn write_to_active_terminal(&mut self, input_bytes: Vec) { diff --git a/src/common/errors.rs b/src/common/errors.rs index 1548aec0..9801428c 100644 --- a/src/common/errors.rs +++ b/src/common/errors.rs @@ -168,7 +168,7 @@ impl Display for ContextType { /// Stack call representations corresponding to the different types of [`ScreenInstruction`]s. #[derive(Debug, Clone, Copy, PartialEq)] pub enum ScreenContext { - HandlePtyEvent, + HandlePtyBytes, Render, NewPane, HorizontalSplit, @@ -210,7 +210,7 @@ pub enum ScreenContext { impl From<&ScreenInstruction> for ScreenContext { fn from(screen_instruction: &ScreenInstruction) -> Self { match *screen_instruction { - ScreenInstruction::Pty(..) => ScreenContext::HandlePtyEvent, + ScreenInstruction::PtyBytes(..) => ScreenContext::HandlePtyBytes, ScreenInstruction::Render => ScreenContext::Render, ScreenInstruction::NewPane(_) => ScreenContext::NewPane, ScreenInstruction::HorizontalSplit(_) => ScreenContext::HorizontalSplit, diff --git a/src/common/mod.rs b/src/common/mod.rs index 7651905c..2466aecf 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -275,19 +275,19 @@ pub fn start(mut os_input: Box, opts: CliArgs) { screen.send_app_instructions.update(err_ctx); screen.send_pty_instructions.update(err_ctx); match event { - ScreenInstruction::Pty(pid, vte_event) => { + ScreenInstruction::PtyBytes(pid, vte_bytes) => { let active_tab = screen.get_active_tab_mut().unwrap(); if active_tab.has_terminal_pid(pid) { // it's most likely that this event is directed at the active tab // look there first - active_tab.handle_pty_event(pid, vte_event); + active_tab.handle_pty_bytes(pid, vte_bytes); } else { // if this event wasn't directed at the active tab, start looking // in other tabs let all_tabs = screen.get_tabs_mut(); for tab in all_tabs.values_mut() { if tab.has_terminal_pid(pid) { - tab.handle_pty_event(pid, vte_event); + tab.handle_pty_bytes(pid, vte_bytes); break; } } diff --git a/src/common/pty_bus.rs b/src/common/pty_bus.rs index 7e854257..2cab8ae8 100644 --- a/src/common/pty_bus.rs +++ b/src/common/pty_bus.rs @@ -6,7 +6,6 @@ use ::std::os::unix::io::RawFd; use ::std::pin::*; use ::std::sync::mpsc::Receiver; use ::std::time::{Duration, Instant}; -use ::vte; use std::path::PathBuf; use super::{ScreenInstruction, SenderWithContext, OPENCALLS}; @@ -64,86 +63,7 @@ impl Stream for ReadFromPid { } } -#[derive(Debug, Clone)] -pub enum VteEvent { - // TODO: try not to allocate Vecs - Print(char), - Execute(u8), // byte - Hook(Vec, Vec, bool, char), // params, intermediates, ignore, char - Put(u8), // byte - Unhook, - OscDispatch(Vec>, bool), // params, bell_terminated - CsiDispatch(Vec, Vec, bool, char), // params, intermediates, ignore, char - EscDispatch(Vec, bool, u8), // intermediates, ignore, byte -} - -struct VteEventSender { - id: RawFd, - sender: SenderWithContext, -} - -impl VteEventSender { - pub fn new(id: RawFd, sender: SenderWithContext) -> Self { - VteEventSender { id, sender } - } -} - -impl vte::Perform for VteEventSender { - fn print(&mut self, c: char) { - let _ = self - .sender - .send(ScreenInstruction::Pty(self.id, VteEvent::Print(c))); - } - fn execute(&mut self, byte: u8) { - let _ = self - .sender - .send(ScreenInstruction::Pty(self.id, VteEvent::Execute(byte))); - } - - fn hook(&mut self, params: &[i64], intermediates: &[u8], ignore: bool, c: char) { - let params = params.iter().copied().collect(); - let intermediates = intermediates.iter().copied().collect(); - let instruction = - ScreenInstruction::Pty(self.id, VteEvent::Hook(params, intermediates, ignore, c)); - let _ = self.sender.send(instruction); - } - - fn put(&mut self, byte: u8) { - let _ = self - .sender - .send(ScreenInstruction::Pty(self.id, VteEvent::Put(byte))); - } - - fn unhook(&mut self) { - let _ = self - .sender - .send(ScreenInstruction::Pty(self.id, VteEvent::Unhook)); - } - - fn osc_dispatch(&mut self, params: &[&[u8]], bell_terminated: bool) { - let params = params.iter().map(|p| p.to_vec()).collect(); - let instruction = - ScreenInstruction::Pty(self.id, VteEvent::OscDispatch(params, bell_terminated)); - let _ = self.sender.send(instruction); - } - - fn csi_dispatch(&mut self, params: &[i64], intermediates: &[u8], ignore: bool, c: char) { - let params = params.iter().copied().collect(); - let intermediates = intermediates.iter().copied().collect(); - let instruction = ScreenInstruction::Pty( - self.id, - VteEvent::CsiDispatch(params, intermediates, ignore, c), - ); - let _ = self.sender.send(instruction); - } - - fn esc_dispatch(&mut self, intermediates: &[u8], ignore: bool, byte: u8) { - let intermediates = intermediates.iter().copied().collect(); - let instruction = - ScreenInstruction::Pty(self.id, VteEvent::EscDispatch(intermediates, ignore, byte)); - let _ = self.sender.send(instruction); - } -} +pub type VteBytes = Vec; /// Instructions related to PTYs (pseudoterminals). #[derive(Clone, Debug)] @@ -178,8 +98,6 @@ fn stream_terminal_bytes( async move { err_ctx.add_call(ContextType::AsyncTask); send_screen_instructions.update(err_ctx); - let mut vte_parser = vte::Parser::new(); - let mut vte_event_sender = VteEventSender::new(pid, send_screen_instructions.clone()); let mut terminal_bytes = ReadFromPid::new(&pid, os_input); let mut last_byte_receive_time: Option = None; @@ -188,13 +106,13 @@ fn stream_terminal_bytes( while let Some(bytes) = terminal_bytes.next().await { let bytes_is_empty = bytes.is_empty(); - for byte in bytes { - if debug { - debug_to_file(byte, pid).unwrap(); + if debug { + for byte in bytes.iter() { + debug_to_file(*byte, pid).unwrap(); } - vte_parser.advance(&mut vte_event_sender, byte); } if !bytes_is_empty { + let _ = send_screen_instructions.send(ScreenInstruction::PtyBytes(pid, bytes)); // for UX reasons, if we got something on the wire, we only send the render notice if: // 1. there aren't any more bytes on the wire afterwards // 2. a certain period (currently 30ms) has elapsed since the last render diff --git a/src/common/screen.rs b/src/common/screen.rs index 41e934cb..1e37ca4f 100644 --- a/src/common/screen.rs +++ b/src/common/screen.rs @@ -8,7 +8,7 @@ use std::sync::mpsc::Receiver; use super::{AppInstruction, SenderWithContext}; use crate::os_input_output::OsApi; use crate::panes::PositionAndSize; -use crate::pty_bus::{PtyInstruction, VteEvent}; +use crate::pty_bus::{PtyInstruction, VteBytes}; use crate::tab::Tab; use crate::{errors::ErrorContext, wasm_vm::PluginInstruction}; use crate::{layout::Layout, panes::PaneId}; @@ -18,7 +18,7 @@ use zellij_tile::data::{Event, ModeInfo, TabInfo}; /// Instructions that can be sent to the [`Screen`]. #[derive(Debug, Clone)] pub enum ScreenInstruction { - Pty(RawFd, VteEvent), + PtyBytes(RawFd, VteBytes), Render, NewPane(PaneId), HorizontalSplit(PaneId), diff --git a/src/main.rs b/src/main.rs index 716acb91..7d8ca6d5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -21,7 +21,6 @@ use structopt::StructOpt; use crate::cli::CliArgs; use crate::command_is_executing::CommandIsExecuting; use crate::os_input_output::get_os_input; -use crate::pty_bus::VteEvent; use crate::utils::{ consts::{ZELLIJ_IPC_PIPE, ZELLIJ_TMP_DIR, ZELLIJ_TMP_LOG_DIR}, logging::*,