Skip to content
This repository has been archived by the owner on Aug 6, 2023. It is now read-only.

Crossterm Performance Update #173

Merged
merged 1 commit into from
Aug 4, 2019
Merged

Crossterm Performance Update #173

merged 1 commit into from
Aug 4, 2019

Conversation

TimonPost
Copy link
Contributor

I have been working on improving the performance of crosterm. The status can be followed here:crossterm-rs/crossterm#175. On windows, the performance increased with 60% and on Linux, the performance increased with +- 40% .

This is positive for TUI, and now there are no glitches anymore in the screen.

Before:

old

After

new

The noticeable change is, especially in the windows implementation. Windows is way slower with terminal actions. Like I had a benchmark where I did some stuff on windows it took 5 seconds to complete and on Linux it took less than one second.

For now, this should not be merged yet. I am still working on it, and once crossterm 0.10 releases I will update this PR as well.

@TimonPost TimonPost changed the title Crossterm Performance Update WIP: Crossterm Performance Update Jul 20, 2019
@TimonPost
Copy link
Contributor Author

How it used to be:
old_version

How it looks like with the update:

new (2)

It is ready to be released, I updated the version of crossterm

@TimonPost TimonPost changed the title WIP: Crossterm Performance Update Crossterm Performance Update Jul 26, 2019
@matprec
Copy link

matprec commented Jul 27, 2019

This is awesome! To be honest, i'm kind of appalled by the performance of powershell / cmd in general, but especially in combination with interactive terminal applications.

I'm seeing some serious memory leaks in the underlying console subsystem, see tokio-rs/console#22 . I will report back whether this solves it!

@TimonPost
Copy link
Contributor Author

@fdehau can this be merged?

Cargo.toml Outdated
@@ -28,9 +28,9 @@ log = "0.4"
either = "1.5"
unicode-segmentation = "1.2"
unicode-width = "0.1"
termion = { version = "1.5", optional = true }
termion = { version = "1.0" , optional = true }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why a change on the termion version ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good catch, I was trying something out there. But should be reverted.

@fdehau
Copy link
Owner

fdehau commented Jul 31, 2019

Beside my small comment, this is great work and I would be happy to merge it. Could you maybe also squash some commits to make all of them relevant ? I'm thinking that because of the breaking changes in the public api this is going to be in a 0.7 release.

@TimonPost
Copy link
Contributor Author

Awesome, I'll squash my commits, and yes it has braking changes so a new version is required.

@TimonPost
Copy link
Contributor Author

squashed!

@fdehau fdehau merged commit a0f6605 into fdehau:master Aug 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants