Fix some real/saved cursor bugs during resize #3032
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aidanhs
force-pushed
the
aphs-improve-reflow-perf
branch
from
c1d0410 to
33638be
Compare
next_lines is always consolidated to a single Row, which immediately gets removed - we can remove some dead code as a result
Given a 1MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~9s to ~3s
Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~23s to ~20s
See zellij-org#2182 for original introduction that only added it in one branch, this fixes an issue where the saved cursor was incorrectly reset when the real cursor was
Previously if a 20 character line were split into two 10 character lines, the cursor would be placed on the line after the two lines. New characters would then be treated as a new canonical line. This commit fixes this by biasing cursors to the end of the previous line.
aidanhs
force-pushed
the
aphs-improve-reflow-perf
branch
from
33638be to
ff72fe8
Compare
aidanhs
force-pushed
the
aphs-improve-reflow-perf
branch
from
ff72fe8 to
528ccbf
Compare
aidanhs
changed the title
* rust-toolchain: Bump toolchain version to 1.69.0 which, compared to the previous 1.67.0, has the following impacts on `zellij`: - [Turn off debuginfo for build deps][2]: Increases build time (on my machine) from ~230 s in 1.67.0 to ~250 s now, *which is unexpected* This version also changes [handling of the `default-features` flag][3] when specifying dependencies in `Cargo.toml`. If a dependent crate requires `default-features = true` on a crate that is required as `default-features = false` further up the dependency tree, the `true` setting "wins". We only specify `default-features = false` for three crates total: - `names`: This is used only by us - `surf`: This is used only by us - `vte`: This is also required by `strip-ansi-escapes`, but that has `default-features = false` as well How this affects our transitive dependencies is unknown at this point. [2]: rust-lang/cargo#11252 [3]: rust-lang/cargo#11409 * rust-toolchain: Bump toolchain version to 1.70.0 which, compared to the previous 1.69.0, as the following impacts on `zellij`: 1. [Enable sparse registry checkout for crates.io by default][1] This drastically increases the time to first build on a fresh rust installation/a rust installation with a clean cargo registry cache. Previously it took about 75s to populate the deps/cache (with `cargo fetch --locked` and ~100 MBit/s network), whereas now the same process takes ~10 s. 2. [The `OnceCell` type is now part of std][2] In theory, this would allow us to cut a dependency from `zellij-utils`, but the `once_cell` crate is pulled in by another 16 deps, so there's no point in attempting it right now. Build times and binary sizes are unaffected by this change compared to the previous 1.69.0 toolchain. [1]: rust-lang/cargo#11791 [2]: https://doc.rust-lang.org/stable/std/cell/struct.OnceCell.html * rust-toolchain: Bump toolchain version to 1.75.0 which, compared to the previous 1.70.0, has the following impacts on `zellij`: 1. [cross-crate inlining][8] This should increase application performance, as functions can now be inlined across crates. 2. [`async fn` in traits][9] This would allow us to drop the `async_trait` dependency, but it is currently still required by 3 other dependencies. Build time in debug mode (on my own PC) is cut down from 256s to 189s (for a clean build). Build time in release mode is cut down from 473s to 391s (for a clean build). Binary sizes only change minimally (825 MB -> 807 MB in debug, 29 MB -> 30 MB in release). [8]: rust-lang/rust#116505 [9]: rust-lang/rust#115822 * chore: Apply rustfmt. * CHANGELOG: Add PR zellij-org#3039.
…control plugins from the command line (zellij-org#3066) * prototype - working with message from the cli * prototype - pipe from the CLI to plugins * prototype - pipe from the CLI to plugins and back again * prototype - working with better cli interface * prototype - working after removing unused stuff * prototype - working with launching plugin if it is not launched, also fixed event ordering * refactor: change message to cli-message * prototype - allow plugins to send messages to each other * fix: allow cli messages to send plugin parameters (and implement backpressure) * fix: use input_pipe_id to identify cli pipes instead of their message name * fix: come cleanups and add skip_cache parameter * fix: pipe/client-server communication robustness * fix: leaking messages between plugins while loading * feat: allow plugins to specify how a new plugin instance is launched when sending messages * fix: add permissions * refactor: adjust cli api * fix: improve cli plugin loading error messages * docs: cli pipe * fix: take plugin configuration into account when messaging between plugins * refactor: pipe message protobuf interface * refactor: update(event) -> pipe * refactor - rename CliMessage to CliPipe * fix: add is_private to pipes and change some naming * refactor - cli client * refactor: various cleanups * style(fmt): rustfmt * fix(pipes): backpressure across multiple plugins * style: some cleanups * style(fmt): rustfmt * style: fix merge conflict mistake * style(wording): clarify pipe permission
|
Hey, just FYI: I'm reviewing this and made a commit that broke one of the e2e tests. I'm honestly not sure what's up there and plan to investigate further. Might not get to it until next week though. |
|
Alright, turns out it was not breaking the tests but rather fixing a bug (possibly a very old one) with integer overflow. Merging. |
imsnif
added a commit
to aidanhs/zellij
that referenced
this pull request
Jan 22, 2024
…3032) * refactor: Simplify transfer_rows_from_viewport_to_lines_above next_lines is always consolidated to a single Row, which immediately gets removed - we can remove some dead code as a result * perf: Batch remove rows from the viewport for performance Given a 1MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~9s to ~3s * perf: Optimize Row::drain_until by splitting chars in one step Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~23s to ~20s * refactor: Simplify `if let` into a `.map` * refactor: There are only new saved coordinates when there were old ones * refactor: Unify viewport transfer: use common variable names * fix: Use same saved cursor logic in height resize as width See zellij-org#2182 for original introduction that only added it in one branch, this fixes an issue where the saved cursor was incorrectly reset when the real cursor was * fix: Correct saved+real cursor calculations when reflowing long lines * fix: Don't create canonical lines if cursor ends on EOL after resize Previously if a 20 character line were split into two 10 character lines, the cursor would be placed on the line after the two lines. New characters would then be treated as a new canonical line. This commit fixes this by biasing cursors to the end of the previous line. * fix: for cursor index calculation in lines that are already wrapped * chore: test for real/saved cursor position being handled separately * chore: Apply cargo format * chore(repo): update issue templates * Bump rust version to 1.75.0 (zellij-org#3039) * rust-toolchain: Bump toolchain version to 1.69.0 which, compared to the previous 1.67.0, has the following impacts on `zellij`: - [Turn off debuginfo for build deps][2]: Increases build time (on my machine) from ~230 s in 1.67.0 to ~250 s now, *which is unexpected* This version also changes [handling of the `default-features` flag][3] when specifying dependencies in `Cargo.toml`. If a dependent crate requires `default-features = true` on a crate that is required as `default-features = false` further up the dependency tree, the `true` setting "wins". We only specify `default-features = false` for three crates total: - `names`: This is used only by us - `surf`: This is used only by us - `vte`: This is also required by `strip-ansi-escapes`, but that has `default-features = false` as well How this affects our transitive dependencies is unknown at this point. [2]: rust-lang/cargo#11252 [3]: rust-lang/cargo#11409 * rust-toolchain: Bump toolchain version to 1.70.0 which, compared to the previous 1.69.0, as the following impacts on `zellij`: 1. [Enable sparse registry checkout for crates.io by default][1] This drastically increases the time to first build on a fresh rust installation/a rust installation with a clean cargo registry cache. Previously it took about 75s to populate the deps/cache (with `cargo fetch --locked` and ~100 MBit/s network), whereas now the same process takes ~10 s. 2. [The `OnceCell` type is now part of std][2] In theory, this would allow us to cut a dependency from `zellij-utils`, but the `once_cell` crate is pulled in by another 16 deps, so there's no point in attempting it right now. Build times and binary sizes are unaffected by this change compared to the previous 1.69.0 toolchain. [1]: rust-lang/cargo#11791 [2]: https://doc.rust-lang.org/stable/std/cell/struct.OnceCell.html * rust-toolchain: Bump toolchain version to 1.75.0 which, compared to the previous 1.70.0, has the following impacts on `zellij`: 1. [cross-crate inlining][8] This should increase application performance, as functions can now be inlined across crates. 2. [`async fn` in traits][9] This would allow us to drop the `async_trait` dependency, but it is currently still required by 3 other dependencies. Build time in debug mode (on my own PC) is cut down from 256s to 189s (for a clean build). Build time in release mode is cut down from 473s to 391s (for a clean build). Binary sizes only change minimally (825 MB -> 807 MB in debug, 29 MB -> 30 MB in release). [8]: rust-lang/rust#116505 [9]: rust-lang/rust#115822 * chore: Apply rustfmt. * CHANGELOG: Add PR zellij-org#3039. * feat(plugins): introduce 'pipes', allowing users to pipe data to and control plugins from the command line (zellij-org#3066) * prototype - working with message from the cli * prototype - pipe from the CLI to plugins * prototype - pipe from the CLI to plugins and back again * prototype - working with better cli interface * prototype - working after removing unused stuff * prototype - working with launching plugin if it is not launched, also fixed event ordering * refactor: change message to cli-message * prototype - allow plugins to send messages to each other * fix: allow cli messages to send plugin parameters (and implement backpressure) * fix: use input_pipe_id to identify cli pipes instead of their message name * fix: come cleanups and add skip_cache parameter * fix: pipe/client-server communication robustness * fix: leaking messages between plugins while loading * feat: allow plugins to specify how a new plugin instance is launched when sending messages * fix: add permissions * refactor: adjust cli api * fix: improve cli plugin loading error messages * docs: cli pipe * fix: take plugin configuration into account when messaging between plugins * refactor: pipe message protobuf interface * refactor: update(event) -> pipe * refactor - rename CliMessage to CliPipe * fix: add is_private to pipes and change some naming * refactor - cli client * refactor: various cleanups * style(fmt): rustfmt * fix(pipes): backpressure across multiple plugins * style: some cleanups * style(fmt): rustfmt * style: fix merge conflict mistake * style(wording): clarify pipe permission * docs(changelog): introduce pipes * fix: add some robustness and future proofing * fix e2e tests --------- Co-authored-by: Aram Drevekenin <[email protected]> Co-authored-by: har7an <[email protected]>
imsnif
added a commit
that referenced
this pull request
Jan 22, 2024
… reducing size of TerminalCharacter (#3043) * refactor: Simplify transfer_rows_from_viewport_to_lines_above next_lines is always consolidated to a single Row, which immediately gets removed - we can remove some dead code as a result * perf: Batch remove rows from the viewport for performance Given a 1MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~9s to ~3s * perf: Optimize Row::drain_until by splitting chars in one step Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~23s to ~20s * refactor: Simplify `if let` into a `.map` * refactor: There are only new saved coordinates when there were old ones * refactor: Unify viewport transfer: use common variable names * fix: Use same saved cursor logic in height resize as width See #2182 for original introduction that only added it in one branch, this fixes an issue where the saved cursor was incorrectly reset when the real cursor was * fix: Correct saved+real cursor calculations when reflowing long lines * fix: Don't create canonical lines if cursor ends on EOL after resize Previously if a 20 character line were split into two 10 character lines, the cursor would be placed on the line after the two lines. New characters would then be treated as a new canonical line. This commit fixes this by biasing cursors to the end of the previous line. * fix: for cursor index calculation in lines that are already wrapped * chore: test for real/saved cursor position being handled separately * chore: Apply cargo format * refactor: Unify viewport transfer: transfer + cursor update together * perf: Reduce size of TerminalCharacter from 72 to 60 bytes With a 10MB single line catted into a fresh terminal, VmRSS goes from 964092 kB to 874020 kB (as reported by /proc/<pid>/status) before/after this patch Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~15s to ~12.5s * fix(build): Don't unconditionally rebuild zellij-utils * refactor: Remove Copy impl on TerminalCharacter * perf: Rc styles to reduce TerminalCharacter from 60 to 24 bytes With a 10MB single line catted into a fresh terminal, VmRSS goes from 845156 kB to 478396 kB (as reported by /proc/<pid>/status) before/after this patch Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~12.5s to ~7s * perf: Remove RcCharacterStyles::Default, allow enum niche optimisation This reduces TerminalCharacter from 24 to 16 bytes With a 10MB single line catted into a fresh terminal, VmRSS goes from 478396 kB to 398108 kB (as reported by /proc/<pid>/status) before/after this patch Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~7s to ~4.75s * docs: link anchor omission from reset_all is deliberate reset_all is only used from ansi params, and ansi params don't control link anchor * fix: Remove no-op on variable that gets immediately dropped * refactor: Simplify replace_character_at logic The original condition checked absolute_x_index was in bounds, then used the index to manipulate it. This is equivalent to getting a ref to the character at that position and manipulating directly * chore: Run xtask format * chore(repo): update issue templates * Bump rust version to 1.75.0 (#3039) * rust-toolchain: Bump toolchain version to 1.69.0 which, compared to the previous 1.67.0, has the following impacts on `zellij`: - [Turn off debuginfo for build deps][2]: Increases build time (on my machine) from ~230 s in 1.67.0 to ~250 s now, *which is unexpected* This version also changes [handling of the `default-features` flag][3] when specifying dependencies in `Cargo.toml`. If a dependent crate requires `default-features = true` on a crate that is required as `default-features = false` further up the dependency tree, the `true` setting "wins". We only specify `default-features = false` for three crates total: - `names`: This is used only by us - `surf`: This is used only by us - `vte`: This is also required by `strip-ansi-escapes`, but that has `default-features = false` as well How this affects our transitive dependencies is unknown at this point. [2]: rust-lang/cargo#11252 [3]: rust-lang/cargo#11409 * rust-toolchain: Bump toolchain version to 1.70.0 which, compared to the previous 1.69.0, as the following impacts on `zellij`: 1. [Enable sparse registry checkout for crates.io by default][1] This drastically increases the time to first build on a fresh rust installation/a rust installation with a clean cargo registry cache. Previously it took about 75s to populate the deps/cache (with `cargo fetch --locked` and ~100 MBit/s network), whereas now the same process takes ~10 s. 2. [The `OnceCell` type is now part of std][2] In theory, this would allow us to cut a dependency from `zellij-utils`, but the `once_cell` crate is pulled in by another 16 deps, so there's no point in attempting it right now. Build times and binary sizes are unaffected by this change compared to the previous 1.69.0 toolchain. [1]: rust-lang/cargo#11791 [2]: https://doc.rust-lang.org/stable/std/cell/struct.OnceCell.html * rust-toolchain: Bump toolchain version to 1.75.0 which, compared to the previous 1.70.0, has the following impacts on `zellij`: 1. [cross-crate inlining][8] This should increase application performance, as functions can now be inlined across crates. 2. [`async fn` in traits][9] This would allow us to drop the `async_trait` dependency, but it is currently still required by 3 other dependencies. Build time in debug mode (on my own PC) is cut down from 256s to 189s (for a clean build). Build time in release mode is cut down from 473s to 391s (for a clean build). Binary sizes only change minimally (825 MB -> 807 MB in debug, 29 MB -> 30 MB in release). [8]: rust-lang/rust#116505 [9]: rust-lang/rust#115822 * chore: Apply rustfmt. * CHANGELOG: Add PR #3039. * feat(plugins): introduce 'pipes', allowing users to pipe data to and control plugins from the command line (#3066) * prototype - working with message from the cli * prototype - pipe from the CLI to plugins * prototype - pipe from the CLI to plugins and back again * prototype - working with better cli interface * prototype - working after removing unused stuff * prototype - working with launching plugin if it is not launched, also fixed event ordering * refactor: change message to cli-message * prototype - allow plugins to send messages to each other * fix: allow cli messages to send plugin parameters (and implement backpressure) * fix: use input_pipe_id to identify cli pipes instead of their message name * fix: come cleanups and add skip_cache parameter * fix: pipe/client-server communication robustness * fix: leaking messages between plugins while loading * feat: allow plugins to specify how a new plugin instance is launched when sending messages * fix: add permissions * refactor: adjust cli api * fix: improve cli plugin loading error messages * docs: cli pipe * fix: take plugin configuration into account when messaging between plugins * refactor: pipe message protobuf interface * refactor: update(event) -> pipe * refactor - rename CliMessage to CliPipe * fix: add is_private to pipes and change some naming * refactor - cli client * refactor: various cleanups * style(fmt): rustfmt * fix(pipes): backpressure across multiple plugins * style: some cleanups * style(fmt): rustfmt * style: fix merge conflict mistake * style(wording): clarify pipe permission * docs(changelog): introduce pipes * xtask: Disable pusing during publish (#3040) * xtask: Add `--no-push` flag to `publish` which can be used when simulating releases to work without a writable git fork of the zellij code. * xtask: Fix borrow issues * xtask/pipe: Require lockfile in publish to avoid errors from invalid dependency versions. * CHANGELOG: Add PR #3040. * fix(terminal): some real/saved cursor bugs during resize (#3032) * refactor: Simplify transfer_rows_from_viewport_to_lines_above next_lines is always consolidated to a single Row, which immediately gets removed - we can remove some dead code as a result * perf: Batch remove rows from the viewport for performance Given a 1MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~9s to ~3s * perf: Optimize Row::drain_until by splitting chars in one step Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~23s to ~20s * refactor: Simplify `if let` into a `.map` * refactor: There are only new saved coordinates when there were old ones * refactor: Unify viewport transfer: use common variable names * fix: Use same saved cursor logic in height resize as width See #2182 for original introduction that only added it in one branch, this fixes an issue where the saved cursor was incorrectly reset when the real cursor was * fix: Correct saved+real cursor calculations when reflowing long lines * fix: Don't create canonical lines if cursor ends on EOL after resize Previously if a 20 character line were split into two 10 character lines, the cursor would be placed on the line after the two lines. New characters would then be treated as a new canonical line. This commit fixes this by biasing cursors to the end of the previous line. * fix: for cursor index calculation in lines that are already wrapped * chore: test for real/saved cursor position being handled separately * chore: Apply cargo format * chore(repo): update issue templates * Bump rust version to 1.75.0 (#3039) * rust-toolchain: Bump toolchain version to 1.69.0 which, compared to the previous 1.67.0, has the following impacts on `zellij`: - [Turn off debuginfo for build deps][2]: Increases build time (on my machine) from ~230 s in 1.67.0 to ~250 s now, *which is unexpected* This version also changes [handling of the `default-features` flag][3] when specifying dependencies in `Cargo.toml`. If a dependent crate requires `default-features = true` on a crate that is required as `default-features = false` further up the dependency tree, the `true` setting "wins". We only specify `default-features = false` for three crates total: - `names`: This is used only by us - `surf`: This is used only by us - `vte`: This is also required by `strip-ansi-escapes`, but that has `default-features = false` as well How this affects our transitive dependencies is unknown at this point. [2]: rust-lang/cargo#11252 [3]: rust-lang/cargo#11409 * rust-toolchain: Bump toolchain version to 1.70.0 which, compared to the previous 1.69.0, as the following impacts on `zellij`: 1. [Enable sparse registry checkout for crates.io by default][1] This drastically increases the time to first build on a fresh rust installation/a rust installation with a clean cargo registry cache. Previously it took about 75s to populate the deps/cache (with `cargo fetch --locked` and ~100 MBit/s network), whereas now the same process takes ~10 s. 2. [The `OnceCell` type is now part of std][2] In theory, this would allow us to cut a dependency from `zellij-utils`, but the `once_cell` crate is pulled in by another 16 deps, so there's no point in attempting it right now. Build times and binary sizes are unaffected by this change compared to the previous 1.69.0 toolchain. [1]: rust-lang/cargo#11791 [2]: https://doc.rust-lang.org/stable/std/cell/struct.OnceCell.html * rust-toolchain: Bump toolchain version to 1.75.0 which, compared to the previous 1.70.0, has the following impacts on `zellij`: 1. [cross-crate inlining][8] This should increase application performance, as functions can now be inlined across crates. 2. [`async fn` in traits][9] This would allow us to drop the `async_trait` dependency, but it is currently still required by 3 other dependencies. Build time in debug mode (on my own PC) is cut down from 256s to 189s (for a clean build). Build time in release mode is cut down from 473s to 391s (for a clean build). Binary sizes only change minimally (825 MB -> 807 MB in debug, 29 MB -> 30 MB in release). [8]: rust-lang/rust#116505 [9]: rust-lang/rust#115822 * chore: Apply rustfmt. * CHANGELOG: Add PR #3039. * feat(plugins): introduce 'pipes', allowing users to pipe data to and control plugins from the command line (#3066) * prototype - working with message from the cli * prototype - pipe from the CLI to plugins * prototype - pipe from the CLI to plugins and back again * prototype - working with better cli interface * prototype - working after removing unused stuff * prototype - working with launching plugin if it is not launched, also fixed event ordering * refactor: change message to cli-message * prototype - allow plugins to send messages to each other * fix: allow cli messages to send plugin parameters (and implement backpressure) * fix: use input_pipe_id to identify cli pipes instead of their message name * fix: come cleanups and add skip_cache parameter * fix: pipe/client-server communication robustness * fix: leaking messages between plugins while loading * feat: allow plugins to specify how a new plugin instance is launched when sending messages * fix: add permissions * refactor: adjust cli api * fix: improve cli plugin loading error messages * docs: cli pipe * fix: take plugin configuration into account when messaging between plugins * refactor: pipe message protobuf interface * refactor: update(event) -> pipe * refactor - rename CliMessage to CliPipe * fix: add is_private to pipes and change some naming * refactor - cli client * refactor: various cleanups * style(fmt): rustfmt * fix(pipes): backpressure across multiple plugins * style: some cleanups * style(fmt): rustfmt * style: fix merge conflict mistake * style(wording): clarify pipe permission * docs(changelog): introduce pipes * fix: add some robustness and future proofing * fix e2e tests --------- Co-authored-by: Aram Drevekenin <[email protected]> Co-authored-by: har7an <[email protected]> * fix integer overflow again (oops) --------- Co-authored-by: Aram Drevekenin <[email protected]> Co-authored-by: har7an <[email protected]>
xuanyuan300
pushed a commit
to trysthout/zellij
that referenced
this pull request
Jan 23, 2024
…3032) * refactor: Simplify transfer_rows_from_viewport_to_lines_above next_lines is always consolidated to a single Row, which immediately gets removed - we can remove some dead code as a result * perf: Batch remove rows from the viewport for performance Given a 1MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~9s to ~3s * perf: Optimize Row::drain_until by splitting chars in one step Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~23s to ~20s * refactor: Simplify `if let` into a `.map` * refactor: There are only new saved coordinates when there were old ones * refactor: Unify viewport transfer: use common variable names * fix: Use same saved cursor logic in height resize as width See zellij-org#2182 for original introduction that only added it in one branch, this fixes an issue where the saved cursor was incorrectly reset when the real cursor was * fix: Correct saved+real cursor calculations when reflowing long lines * fix: Don't create canonical lines if cursor ends on EOL after resize Previously if a 20 character line were split into two 10 character lines, the cursor would be placed on the line after the two lines. New characters would then be treated as a new canonical line. This commit fixes this by biasing cursors to the end of the previous line. * fix: for cursor index calculation in lines that are already wrapped * chore: test for real/saved cursor position being handled separately * chore: Apply cargo format * chore(repo): update issue templates * Bump rust version to 1.75.0 (zellij-org#3039) * rust-toolchain: Bump toolchain version to 1.69.0 which, compared to the previous 1.67.0, has the following impacts on `zellij`: - [Turn off debuginfo for build deps][2]: Increases build time (on my machine) from ~230 s in 1.67.0 to ~250 s now, *which is unexpected* This version also changes [handling of the `default-features` flag][3] when specifying dependencies in `Cargo.toml`. If a dependent crate requires `default-features = true` on a crate that is required as `default-features = false` further up the dependency tree, the `true` setting "wins". We only specify `default-features = false` for three crates total: - `names`: This is used only by us - `surf`: This is used only by us - `vte`: This is also required by `strip-ansi-escapes`, but that has `default-features = false` as well How this affects our transitive dependencies is unknown at this point. [2]: rust-lang/cargo#11252 [3]: rust-lang/cargo#11409 * rust-toolchain: Bump toolchain version to 1.70.0 which, compared to the previous 1.69.0, as the following impacts on `zellij`: 1. [Enable sparse registry checkout for crates.io by default][1] This drastically increases the time to first build on a fresh rust installation/a rust installation with a clean cargo registry cache. Previously it took about 75s to populate the deps/cache (with `cargo fetch --locked` and ~100 MBit/s network), whereas now the same process takes ~10 s. 2. [The `OnceCell` type is now part of std][2] In theory, this would allow us to cut a dependency from `zellij-utils`, but the `once_cell` crate is pulled in by another 16 deps, so there's no point in attempting it right now. Build times and binary sizes are unaffected by this change compared to the previous 1.69.0 toolchain. [1]: rust-lang/cargo#11791 [2]: https://doc.rust-lang.org/stable/std/cell/struct.OnceCell.html * rust-toolchain: Bump toolchain version to 1.75.0 which, compared to the previous 1.70.0, has the following impacts on `zellij`: 1. [cross-crate inlining][8] This should increase application performance, as functions can now be inlined across crates. 2. [`async fn` in traits][9] This would allow us to drop the `async_trait` dependency, but it is currently still required by 3 other dependencies. Build time in debug mode (on my own PC) is cut down from 256s to 189s (for a clean build). Build time in release mode is cut down from 473s to 391s (for a clean build). Binary sizes only change minimally (825 MB -> 807 MB in debug, 29 MB -> 30 MB in release). [8]: rust-lang/rust#116505 [9]: rust-lang/rust#115822 * chore: Apply rustfmt. * CHANGELOG: Add PR zellij-org#3039. * feat(plugins): introduce 'pipes', allowing users to pipe data to and control plugins from the command line (zellij-org#3066) * prototype - working with message from the cli * prototype - pipe from the CLI to plugins * prototype - pipe from the CLI to plugins and back again * prototype - working with better cli interface * prototype - working after removing unused stuff * prototype - working with launching plugin if it is not launched, also fixed event ordering * refactor: change message to cli-message * prototype - allow plugins to send messages to each other * fix: allow cli messages to send plugin parameters (and implement backpressure) * fix: use input_pipe_id to identify cli pipes instead of their message name * fix: come cleanups and add skip_cache parameter * fix: pipe/client-server communication robustness * fix: leaking messages between plugins while loading * feat: allow plugins to specify how a new plugin instance is launched when sending messages * fix: add permissions * refactor: adjust cli api * fix: improve cli plugin loading error messages * docs: cli pipe * fix: take plugin configuration into account when messaging between plugins * refactor: pipe message protobuf interface * refactor: update(event) -> pipe * refactor - rename CliMessage to CliPipe * fix: add is_private to pipes and change some naming * refactor - cli client * refactor: various cleanups * style(fmt): rustfmt * fix(pipes): backpressure across multiple plugins * style: some cleanups * style(fmt): rustfmt * style: fix merge conflict mistake * style(wording): clarify pipe permission * docs(changelog): introduce pipes * fix: add some robustness and future proofing * fix e2e tests --------- Co-authored-by: Aram Drevekenin <[email protected]> Co-authored-by: har7an <[email protected]>
xuanyuan300
pushed a commit
to trysthout/zellij
that referenced
this pull request
Jan 23, 2024
… reducing size of TerminalCharacter (zellij-org#3043) * refactor: Simplify transfer_rows_from_viewport_to_lines_above next_lines is always consolidated to a single Row, which immediately gets removed - we can remove some dead code as a result * perf: Batch remove rows from the viewport for performance Given a 1MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~9s to ~3s * perf: Optimize Row::drain_until by splitting chars in one step Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~23s to ~20s * refactor: Simplify `if let` into a `.map` * refactor: There are only new saved coordinates when there were old ones * refactor: Unify viewport transfer: use common variable names * fix: Use same saved cursor logic in height resize as width See zellij-org#2182 for original introduction that only added it in one branch, this fixes an issue where the saved cursor was incorrectly reset when the real cursor was * fix: Correct saved+real cursor calculations when reflowing long lines * fix: Don't create canonical lines if cursor ends on EOL after resize Previously if a 20 character line were split into two 10 character lines, the cursor would be placed on the line after the two lines. New characters would then be treated as a new canonical line. This commit fixes this by biasing cursors to the end of the previous line. * fix: for cursor index calculation in lines that are already wrapped * chore: test for real/saved cursor position being handled separately * chore: Apply cargo format * refactor: Unify viewport transfer: transfer + cursor update together * perf: Reduce size of TerminalCharacter from 72 to 60 bytes With a 10MB single line catted into a fresh terminal, VmRSS goes from 964092 kB to 874020 kB (as reported by /proc/<pid>/status) before/after this patch Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~15s to ~12.5s * fix(build): Don't unconditionally rebuild zellij-utils * refactor: Remove Copy impl on TerminalCharacter * perf: Rc styles to reduce TerminalCharacter from 60 to 24 bytes With a 10MB single line catted into a fresh terminal, VmRSS goes from 845156 kB to 478396 kB (as reported by /proc/<pid>/status) before/after this patch Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~12.5s to ~7s * perf: Remove RcCharacterStyles::Default, allow enum niche optimisation This reduces TerminalCharacter from 24 to 16 bytes With a 10MB single line catted into a fresh terminal, VmRSS goes from 478396 kB to 398108 kB (as reported by /proc/<pid>/status) before/after this patch Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~7s to ~4.75s * docs: link anchor omission from reset_all is deliberate reset_all is only used from ansi params, and ansi params don't control link anchor * fix: Remove no-op on variable that gets immediately dropped * refactor: Simplify replace_character_at logic The original condition checked absolute_x_index was in bounds, then used the index to manipulate it. This is equivalent to getting a ref to the character at that position and manipulating directly * chore: Run xtask format * chore(repo): update issue templates * Bump rust version to 1.75.0 (zellij-org#3039) * rust-toolchain: Bump toolchain version to 1.69.0 which, compared to the previous 1.67.0, has the following impacts on `zellij`: - [Turn off debuginfo for build deps][2]: Increases build time (on my machine) from ~230 s in 1.67.0 to ~250 s now, *which is unexpected* This version also changes [handling of the `default-features` flag][3] when specifying dependencies in `Cargo.toml`. If a dependent crate requires `default-features = true` on a crate that is required as `default-features = false` further up the dependency tree, the `true` setting "wins". We only specify `default-features = false` for three crates total: - `names`: This is used only by us - `surf`: This is used only by us - `vte`: This is also required by `strip-ansi-escapes`, but that has `default-features = false` as well How this affects our transitive dependencies is unknown at this point. [2]: rust-lang/cargo#11252 [3]: rust-lang/cargo#11409 * rust-toolchain: Bump toolchain version to 1.70.0 which, compared to the previous 1.69.0, as the following impacts on `zellij`: 1. [Enable sparse registry checkout for crates.io by default][1] This drastically increases the time to first build on a fresh rust installation/a rust installation with a clean cargo registry cache. Previously it took about 75s to populate the deps/cache (with `cargo fetch --locked` and ~100 MBit/s network), whereas now the same process takes ~10 s. 2. [The `OnceCell` type is now part of std][2] In theory, this would allow us to cut a dependency from `zellij-utils`, but the `once_cell` crate is pulled in by another 16 deps, so there's no point in attempting it right now. Build times and binary sizes are unaffected by this change compared to the previous 1.69.0 toolchain. [1]: rust-lang/cargo#11791 [2]: https://doc.rust-lang.org/stable/std/cell/struct.OnceCell.html * rust-toolchain: Bump toolchain version to 1.75.0 which, compared to the previous 1.70.0, has the following impacts on `zellij`: 1. [cross-crate inlining][8] This should increase application performance, as functions can now be inlined across crates. 2. [`async fn` in traits][9] This would allow us to drop the `async_trait` dependency, but it is currently still required by 3 other dependencies. Build time in debug mode (on my own PC) is cut down from 256s to 189s (for a clean build). Build time in release mode is cut down from 473s to 391s (for a clean build). Binary sizes only change minimally (825 MB -> 807 MB in debug, 29 MB -> 30 MB in release). [8]: rust-lang/rust#116505 [9]: rust-lang/rust#115822 * chore: Apply rustfmt. * CHANGELOG: Add PR zellij-org#3039. * feat(plugins): introduce 'pipes', allowing users to pipe data to and control plugins from the command line (zellij-org#3066) * prototype - working with message from the cli * prototype - pipe from the CLI to plugins * prototype - pipe from the CLI to plugins and back again * prototype - working with better cli interface * prototype - working after removing unused stuff * prototype - working with launching plugin if it is not launched, also fixed event ordering * refactor: change message to cli-message * prototype - allow plugins to send messages to each other * fix: allow cli messages to send plugin parameters (and implement backpressure) * fix: use input_pipe_id to identify cli pipes instead of their message name * fix: come cleanups and add skip_cache parameter * fix: pipe/client-server communication robustness * fix: leaking messages between plugins while loading * feat: allow plugins to specify how a new plugin instance is launched when sending messages * fix: add permissions * refactor: adjust cli api * fix: improve cli plugin loading error messages * docs: cli pipe * fix: take plugin configuration into account when messaging between plugins * refactor: pipe message protobuf interface * refactor: update(event) -> pipe * refactor - rename CliMessage to CliPipe * fix: add is_private to pipes and change some naming * refactor - cli client * refactor: various cleanups * style(fmt): rustfmt * fix(pipes): backpressure across multiple plugins * style: some cleanups * style(fmt): rustfmt * style: fix merge conflict mistake * style(wording): clarify pipe permission * docs(changelog): introduce pipes * xtask: Disable pusing during publish (zellij-org#3040) * xtask: Add `--no-push` flag to `publish` which can be used when simulating releases to work without a writable git fork of the zellij code. * xtask: Fix borrow issues * xtask/pipe: Require lockfile in publish to avoid errors from invalid dependency versions. * CHANGELOG: Add PR zellij-org#3040. * fix(terminal): some real/saved cursor bugs during resize (zellij-org#3032) * refactor: Simplify transfer_rows_from_viewport_to_lines_above next_lines is always consolidated to a single Row, which immediately gets removed - we can remove some dead code as a result * perf: Batch remove rows from the viewport for performance Given a 1MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~9s to ~3s * perf: Optimize Row::drain_until by splitting chars in one step Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~23s to ~20s * refactor: Simplify `if let` into a `.map` * refactor: There are only new saved coordinates when there were old ones * refactor: Unify viewport transfer: use common variable names * fix: Use same saved cursor logic in height resize as width See zellij-org#2182 for original introduction that only added it in one branch, this fixes an issue where the saved cursor was incorrectly reset when the real cursor was * fix: Correct saved+real cursor calculations when reflowing long lines * fix: Don't create canonical lines if cursor ends on EOL after resize Previously if a 20 character line were split into two 10 character lines, the cursor would be placed on the line after the two lines. New characters would then be treated as a new canonical line. This commit fixes this by biasing cursors to the end of the previous line. * fix: for cursor index calculation in lines that are already wrapped * chore: test for real/saved cursor position being handled separately * chore: Apply cargo format * chore(repo): update issue templates * Bump rust version to 1.75.0 (zellij-org#3039) * rust-toolchain: Bump toolchain version to 1.69.0 which, compared to the previous 1.67.0, has the following impacts on `zellij`: - [Turn off debuginfo for build deps][2]: Increases build time (on my machine) from ~230 s in 1.67.0 to ~250 s now, *which is unexpected* This version also changes [handling of the `default-features` flag][3] when specifying dependencies in `Cargo.toml`. If a dependent crate requires `default-features = true` on a crate that is required as `default-features = false` further up the dependency tree, the `true` setting "wins". We only specify `default-features = false` for three crates total: - `names`: This is used only by us - `surf`: This is used only by us - `vte`: This is also required by `strip-ansi-escapes`, but that has `default-features = false` as well How this affects our transitive dependencies is unknown at this point. [2]: rust-lang/cargo#11252 [3]: rust-lang/cargo#11409 * rust-toolchain: Bump toolchain version to 1.70.0 which, compared to the previous 1.69.0, as the following impacts on `zellij`: 1. [Enable sparse registry checkout for crates.io by default][1] This drastically increases the time to first build on a fresh rust installation/a rust installation with a clean cargo registry cache. Previously it took about 75s to populate the deps/cache (with `cargo fetch --locked` and ~100 MBit/s network), whereas now the same process takes ~10 s. 2. [The `OnceCell` type is now part of std][2] In theory, this would allow us to cut a dependency from `zellij-utils`, but the `once_cell` crate is pulled in by another 16 deps, so there's no point in attempting it right now. Build times and binary sizes are unaffected by this change compared to the previous 1.69.0 toolchain. [1]: rust-lang/cargo#11791 [2]: https://doc.rust-lang.org/stable/std/cell/struct.OnceCell.html * rust-toolchain: Bump toolchain version to 1.75.0 which, compared to the previous 1.70.0, has the following impacts on `zellij`: 1. [cross-crate inlining][8] This should increase application performance, as functions can now be inlined across crates. 2. [`async fn` in traits][9] This would allow us to drop the `async_trait` dependency, but it is currently still required by 3 other dependencies. Build time in debug mode (on my own PC) is cut down from 256s to 189s (for a clean build). Build time in release mode is cut down from 473s to 391s (for a clean build). Binary sizes only change minimally (825 MB -> 807 MB in debug, 29 MB -> 30 MB in release). [8]: rust-lang/rust#116505 [9]: rust-lang/rust#115822 * chore: Apply rustfmt. * CHANGELOG: Add PR zellij-org#3039. * feat(plugins): introduce 'pipes', allowing users to pipe data to and control plugins from the command line (zellij-org#3066) * prototype - working with message from the cli * prototype - pipe from the CLI to plugins * prototype - pipe from the CLI to plugins and back again * prototype - working with better cli interface * prototype - working after removing unused stuff * prototype - working with launching plugin if it is not launched, also fixed event ordering * refactor: change message to cli-message * prototype - allow plugins to send messages to each other * fix: allow cli messages to send plugin parameters (and implement backpressure) * fix: use input_pipe_id to identify cli pipes instead of their message name * fix: come cleanups and add skip_cache parameter * fix: pipe/client-server communication robustness * fix: leaking messages between plugins while loading * feat: allow plugins to specify how a new plugin instance is launched when sending messages * fix: add permissions * refactor: adjust cli api * fix: improve cli plugin loading error messages * docs: cli pipe * fix: take plugin configuration into account when messaging between plugins * refactor: pipe message protobuf interface * refactor: update(event) -> pipe * refactor - rename CliMessage to CliPipe * fix: add is_private to pipes and change some naming * refactor - cli client * refactor: various cleanups * style(fmt): rustfmt * fix(pipes): backpressure across multiple plugins * style: some cleanups * style(fmt): rustfmt * style: fix merge conflict mistake * style(wording): clarify pipe permission * docs(changelog): introduce pipes * fix: add some robustness and future proofing * fix e2e tests --------- Co-authored-by: Aram Drevekenin <[email protected]> Co-authored-by: har7an <[email protected]> * fix integer overflow again (oops) --------- Co-authored-by: Aram Drevekenin <[email protected]> Co-authored-by: har7an <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cc #2622
Second in a series: #3045, #3032 (this pr), #3043
This PR builds on the first and contains all commits from it - it will be easier to review after merge of the first.
This PR contains small refactors and a fix for some cursor bugs, in preparation for the final PR in the series.
I highly recommend reviewing these changes commit-by-commit, as I've carefully tried to make each one standalone and self-consistent.