From 8cb1acc27d863942401df80c3acb8b4016ad6083 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Thu, 8 Jul 2021 20:35:58 +0000 Subject: [PATCH] Fix PTY performance regressions The patch 9e7655e introduced some changes which improved rendering with very dense grids, but the automatic benchmarks indicated a slight performance difference in the `dense_cells` benchmark. Caching the terminal lock between iterations rather than always calling `try_lock` resolves that issue. While breaking early in the `WouldBlock` case with `unprocessed != 0` does also help resolve these issues, it shows some more significant fluctuations. Combining both fixes does not help. Additionally on Windows receiving `Ok(0)` from the PTY will also occur instead of a `WouldBlock` error, so handling that fixes freezing on Windows. Fixes #5305. --- .builds/freebsd.yml | 1 + .builds/linux.yml | 1 + Cargo.lock | 157 +++++++++++++++------------ alacritty_terminal/src/event_loop.rs | 45 +++++++- 4 files changed, 130 insertions(+), 74 deletions(-) diff --git a/.builds/freebsd.yml b/.builds/freebsd.yml index 70ee6d43..b67d2672 100644 --- a/.builds/freebsd.yml +++ b/.builds/freebsd.yml @@ -8,6 +8,7 @@ packages: - x11-fonts/fontconfig - x11-fonts/dejavu - x11/libxcb + - x11/libxkbcommon sources: - https://github.com/alacritty/alacritty diff --git a/.builds/linux.yml b/.builds/linux.yml index 3f793175..dc107515 100644 --- a/.builds/linux.yml +++ b/.builds/linux.yml @@ -7,6 +7,7 @@ packages: - freetype2 - fontconfig - libxcb + - libxkbcommon sources: - https://github.com/alacritty/alacritty diff --git a/Cargo.lock b/Cargo.lock index 66534e9f..0cf3214d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -143,12 +143,6 @@ version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0d8c1fef690941d3e7788d328517591fecc684c084084702d6ff1641e993699a" -[[package]] -name = "byteorder" -version = "1.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" - [[package]] name = "calloop" version = "0.6.5" @@ -161,9 +155,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.0.67" +version = "1.0.68" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e3c69b077ad434294d3ce9f1f6143a2a4b89a8a2d54ef813d85003a4fd1137fd" +checksum = "4a72c244c1ff497a746a7e1fb3d14bd08420ecda70c8f25c7112f2781652d787" [[package]] name = "cfg-if" @@ -523,9 +517,9 @@ dependencies = [ [[package]] name = "embed-resource" -version = "1.6.2" +version = "1.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d0ea6debf1262982d24274dc85f3374b42534df140897c25cea86b81e017d470" +checksum = "45de30eb317b4cd3882ee16623cb3004e5fb99a8f4cd40097cadf61efbc54adc" dependencies = [ "cc", "vswhom", @@ -661,9 +655,9 @@ checksum = "3dcaa9ae7725d12cdb85b3ad99a434db70b468c09ded17e012d86b5c1010f7a7" [[package]] name = "getrandom" -version = "0.2.2" +version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c9495705279e7140bf035dde1f6e750c162df8b625267cd52cc44e0b156732c8" +checksum = "7fcd999463524c52659517fe2cea98493cfe485d10565e7b0fb07dbba7ad2753" dependencies = [ "cfg-if 1.0.0", "libc", @@ -755,9 +749,9 @@ dependencies = [ [[package]] name = "hermit-abi" -version = "0.1.18" +version = "0.1.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "322f4de77956e22ed0e5032c359a0f1273f1f7f0d79bfa3b8ffbc730d7fbcc5c" +checksum = "62b467343b94ba476dcb2500d242dadbb39557df889310ac77c5d99100aaac33" dependencies = [ "libc", ] @@ -854,9 +848,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "libc" -version = "0.2.94" +version = "0.2.98" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18794a8ad5b29321f790b55d93dfba91e125cb1a9edbd4f8e3150acc771c1a5e" +checksum = "320cfe77175da3a483efed4bc0adc1968ca050b098ce4f2f1c13a56626128790" [[package]] name = "libloading" @@ -920,9 +914,9 @@ checksum = "60302e4db3a61da70c0cb7991976248362f30319e88850c487b9b95bbf059e00" [[package]] name = "memchr" -version = "2.4.0" +version = "2.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b16bd47d9e329435e309c58469fe0791c2d0d1ba96ec0954152a5ae2b04387dc" +checksum = "0ee1c47aaa256ecabcaea351eae4a9b01ef39ed810004e298d2511ed284b1525" [[package]] name = "memmap2" @@ -933,6 +927,15 @@ dependencies = [ "libc", ] +[[package]] +name = "memmap2" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "723e3ebdcdc5c023db1df315364573789f8857c11b631a2fdfad7c00f5c046b4" +dependencies = [ + "libc", +] + [[package]] name = "miniz_oxide" version = "0.3.7" @@ -1088,9 +1091,9 @@ dependencies = [ [[package]] name = "nom" -version = "6.1.2" +version = "6.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e7413f999671bd4745a7b624bd370a569fb6bc574b23c83a3c5ed2e453f3d5e2" +checksum = "9c5c51b9083a3c620fa67a2a635d1ce7d95b897e957d6b28ff9a5da960a103a6" dependencies = [ "memchr", "version_check", @@ -1098,9 +1101,9 @@ dependencies = [ [[package]] name = "notify" -version = "4.0.16" +version = "4.0.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2599080e87c9bd051ddb11b10074f4da7b1223298df65d4c2ec5bcf309af1533" +checksum = "ae03c8c853dba7bfd23e571ff0cff7bc9dceb40a4cd684cd1681824183f45257" dependencies = [ "bitflags", "filetime", @@ -1167,9 +1170,9 @@ dependencies = [ [[package]] name = "once_cell" -version = "1.7.2" +version = "1.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af8b08b04175473088b46763e51ee54da5f9a164bc162f615b91bc179dbf15a3" +checksum = "692fcb63b64b1758029e0a96ee63e049ce8c5948587f2f7208df04625e5f6b56" [[package]] name = "osmesa-sys" @@ -1248,9 +1251,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.26" +version = "1.0.27" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a152013215dca273577e18d2bf00fa862b89b24169fb78c4c95aeb07992c9cec" +checksum = "f0d8caf72986c1a598726adc988bb5984792ef84f5ee5aa50209145ee8077038" dependencies = [ "unicode-xid", ] @@ -1275,9 +1278,9 @@ dependencies = [ [[package]] name = "redox_syscall" -version = "0.2.7" +version = "0.2.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85dd92e586f7355c633911e11f77f3d12f04b1b1bd76a198bd34ae3af8341ef2" +checksum = "5ab49abadf3f9e1c4bc499e8845e152ad87d2ad2d30371841171169e9d75feee" dependencies = [ "bitflags", ] @@ -1294,19 +1297,18 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.1.9" +version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae1ded71d66a4a97f5e961fd0cb25a5f366a42a41570d16a763a69c092c26ae4" +checksum = "6c230d73fb8d8c1b9c0b3135c5142a8acee3a0558fb8db5cf1cb65f8d7862132" dependencies = [ - "byteorder", "regex-syntax", ] [[package]] name = "regex-syntax" -version = "0.6.24" +version = "0.6.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "00efb87459ba4f6fb2169d20f68565555688e1250ee6825cdf6254f8b48fafb2" +checksum = "f497285884f3fcff424ffc933e56d7cbca511def0c9831a7f9b5f6153e3cc89b" [[package]] name = "rusttype" @@ -1347,18 +1349,18 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" [[package]] name = "serde" -version = "1.0.125" +version = "1.0.126" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "558dc50e1a5a5fa7112ca2ce4effcb321b0300c0d4ccf0776a9f60cd89031171" +checksum = "ec7505abeacaec74ae4778d9d9328fe5a5d04253220a85c4ee022239fc996d03" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.125" +version = "1.0.126" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b093b7a2bb58203b5da3056c05b4ec1fed827dcfdb37347a8841695263b3d06d" +checksum = "963a7dbc9895aeac7ac90e74f34a5d5261828f79df35cbed41e10189d3804d43" dependencies = [ "proc-macro2", "quote", @@ -1432,9 +1434,9 @@ dependencies = [ [[package]] name = "signal-hook-registry" -version = "1.3.0" +version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "16f1d0fef1604ba8f7a073c7e701f213e056707210e9020af4528e0101ce11a6" +checksum = "e51e73328dc4ac0c7ccbda3a494dfa03df1de2f46018127f60c693f2648455b0" dependencies = [ "libc", ] @@ -1463,7 +1465,7 @@ dependencies = [ "dlib 0.4.2", "lazy_static", "log", - "memmap2", + "memmap2 0.1.0", "nix 0.18.0", "wayland-client", "wayland-cursor", @@ -1471,12 +1473,29 @@ dependencies = [ ] [[package]] -name = "smithay-clipboard" -version = "0.6.3" +name = "smithay-client-toolkit" +version = "0.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "06384dfaf645908220d976ae24ed39f6cf92efecb0225ea0a948e403014de527" +checksum = "ec783683499a2cfc85b6df3d04f83b1907b5cbd98a1aed44667dbdf1eac4e64c" dependencies = [ - "smithay-client-toolkit", + "bitflags", + "dlib 0.5.0", + "lazy_static", + "log", + "memmap2 0.2.3", + "nix 0.20.0", + "wayland-client", + "wayland-cursor", + "wayland-protocols", +] + +[[package]] +name = "smithay-clipboard" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "986c5b4a7bd4f50d4c51f81f844745535cb488360f9cf63293780b109b9295f3" +dependencies = [ + "smithay-client-toolkit 0.14.0", "wayland-client", ] @@ -1500,9 +1519,9 @@ checksum = "6446ced80d6c486436db5c078dde11a9f73d42b57fb273121e160b84f63d894c" [[package]] name = "syn" -version = "1.0.71" +version = "1.0.73" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ad184cc9470f9117b2ac6817bfe297307418819ba40552f9b3846f05c33d5373" +checksum = "f71489ff30030d2ae598524f61326b902466f72a0fb1a8564c001cc63425bcc7" dependencies = [ "proc-macro2", "quote", @@ -1520,18 +1539,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.24" +version = "1.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e0f4a65597094d4483ddaed134f409b2cb7c1beccf25201a9f73c719254fa98e" +checksum = "93119e4feac1cbe6c798c34d3a53ea0026b0b1de6a120deef895137c0529bfe2" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.24" +version = "1.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7765189610d8241a44529806d6fd1f2e0a08734313a35d5b3a556f92b381f3c0" +checksum = "060d69a0afe7796bf42e9e2ff91f5ee691fb15c53d38b4b62a9a53eb23164745" dependencies = [ "proc-macro2", "quote", @@ -1652,9 +1671,9 @@ checksum = "fd6fbd9a79829dd1ad0cc20627bf1ed606756a7f77edff7b66b7064f9cb327c6" [[package]] name = "wayland-client" -version = "0.28.5" +version = "0.28.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "06ca44d86554b85cf449f1557edc6cc7da935cc748c8e4bf1c507cbd43bae02c" +checksum = "e3ab332350e502f159382201394a78e3cc12d0f04db863429260164ea40e0355" dependencies = [ "bitflags", "downcast-rs", @@ -1668,9 +1687,9 @@ dependencies = [ [[package]] name = "wayland-commons" -version = "0.28.5" +version = "0.28.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8bd75ae380325dbcff2707f0cd9869827ea1d2d6d534cff076858d3f0460fd5a" +checksum = "a21817947c7011bbd0a27e11b17b337bfd022e8544b071a2641232047966fbda" dependencies = [ "nix 0.20.0", "once_cell", @@ -1680,9 +1699,9 @@ dependencies = [ [[package]] name = "wayland-cursor" -version = "0.28.5" +version = "0.28.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b37e5455ec72f5de555ec39b5c3704036ac07c2ecd50d0bffe02d5fe2d4e65ab" +checksum = "be610084edd1586d45e7bdd275fe345c7c1873598caa464c4fb835dee70fa65a" dependencies = [ "nix 0.20.0", "wayland-client", @@ -1691,9 +1710,9 @@ dependencies = [ [[package]] name = "wayland-egl" -version = "0.28.5" +version = "0.28.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9461a67930ec16da7a4fd8b50e9ffa23f4417240b43ec84008bd1b2c94421c94" +checksum = "99ba1ab1e18756b23982d36f08856d521d7df45015f404a2d7c4f0b2d2f66956" dependencies = [ "wayland-client", "wayland-sys", @@ -1701,9 +1720,9 @@ dependencies = [ [[package]] name = "wayland-protocols" -version = "0.28.5" +version = "0.28.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "95df3317872bcf9eec096c864b69aa4769a1d5d6291a5b513f8ba0af0efbd52c" +checksum = "286620ea4d803bacf61fa087a4242ee316693099ee5a140796aaba02b29f861f" dependencies = [ "bitflags", "wayland-client", @@ -1713,9 +1732,9 @@ dependencies = [ [[package]] name = "wayland-scanner" -version = "0.28.5" +version = "0.28.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "389d680d7bd67512dc9c37f39560224327038deb0f0e8d33f870900441b68720" +checksum = "ce923eb2deb61de332d1f356ec7b6bf37094dc5573952e1c8936db03b54c03f1" dependencies = [ "proc-macro2", "quote", @@ -1724,9 +1743,9 @@ dependencies = [ [[package]] name = "wayland-sys" -version = "0.28.5" +version = "0.28.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2907bd297eef464a95ba9349ea771611771aa285b932526c633dc94d5400a8e2" +checksum = "d841fca9aed7febf9bed2e9796c49bf58d4152ceda8ac949ebe00868d8f0feb8" dependencies = [ "dlib 0.5.0", "lazy_static", @@ -1802,7 +1821,7 @@ dependencies = [ "percent-encoding", "raw-window-handle", "serde", - "smithay-client-toolkit", + "smithay-client-toolkit 0.12.3", "wayland-client", "winapi 0.3.9", "x11-dl", @@ -1810,9 +1829,9 @@ dependencies = [ [[package]] name = "winreg" -version = "0.8.0" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d107f8c6e916235c4c01cabb3e8acf7bea8ef6a63ca2e7fa0527c049badfc48c" +checksum = "16cdb3898397cf7f624c294948669beafaeebc5577d5ec53d0afb76633593597" dependencies = [ "winapi 0.3.9", ] @@ -1838,9 +1857,9 @@ dependencies = [ [[package]] name = "x11-clipboard" -version = "0.5.1" +version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5e937afd03b64b7be4f959cc044e09260a47241b71e56933f37db097bf7859d" +checksum = "b397ace6e980510de59a4fe3d4c758dffab231d6d747ce9fa1aba6b6035d5f32" dependencies = [ "xcb", ] diff --git a/alacritty_terminal/src/event_loop.rs b/alacritty_terminal/src/event_loop.rs index 1a6ee8f3..b0676e4d 100644 --- a/alacritty_terminal/src/event_loop.rs +++ b/alacritty_terminal/src/event_loop.rs @@ -221,10 +221,13 @@ where // Reserve the next terminal lock for PTY reading. let _terminal_lease = Some(self.terminal.lease()); + let mut terminal = None; loop { // Read from the PTY. match self.pty.reader().read(&mut buf[unprocessed..]) { + // This is received on Windows/macOS when no more data is readable from the PTY. + Ok(0) if unprocessed == 0 => break, Ok(got) => unprocessed += got, Err(err) => match err.kind() { ErrorKind::Interrupted | ErrorKind::WouldBlock => { @@ -238,11 +241,14 @@ where } // Attempt to lock the terminal. - let mut terminal = match self.terminal.try_lock_unfair() { - // Force block if we are at the buffer size limit. - None if unprocessed >= READ_BUFFER_SIZE => self.terminal.lock_unfair(), - None => continue, + let terminal = match &mut terminal { Some(terminal) => terminal, + None => terminal.insert(match self.terminal.try_lock_unfair() { + // Force block if we are at the buffer size limit. + None if unprocessed >= READ_BUFFER_SIZE => self.terminal.lock_unfair(), + None => continue, + Some(terminal) => terminal, + }), }; // Write a copy of the bytes to the ref test file. @@ -252,7 +258,7 @@ where // Parse the incoming bytes. for byte in &buf[..unprocessed] { - state.parser.advance(&mut *terminal, *byte); + state.parser.advance(&mut **terminal, *byte); } processed += unprocessed; @@ -425,3 +431,32 @@ where }) } } + +trait OptionInsert { + type T; + fn insert(&mut self, value: Self::T) -> &mut Self::T; +} + +// TODO: Remove when MSRV is >= 1.53.0. +// +/// Trait implementation to support Rust version < 1.53.0. +/// +/// This is taken [from STD], further license information can be found in the [rust-lang/rust +/// repository]. +/// +/// +/// [from STD]: https://github.com/rust-lang/rust/blob/6e0b554619a3bb7e75b3334e97f191af20ef5d76/library/core/src/option.rs#L829-L858 +/// [rust-lang/rust repository]: https://github.com/rust-lang/rust/blob/master/LICENSE-MIT +impl OptionInsert for Option { + type T = T; + + fn insert(&mut self, value: T) -> &mut T { + *self = Some(value); + + match self { + Some(v) => v, + // SAFETY: the code above just filled the option + None => unsafe { std::hint::unreachable_unchecked() }, + } + } +}