-
-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CPU Usage #10
Comments
Hey.Thanks for reporting this. I have a good idea of where the problems are and can definitely optimize this. I've been delaying working on cleanups and optimizations to focus on the implementations. But now I think it's time I start looking into that area. |
- Write to pipes only when the value changes. - Introduce a minimal delay to the background threads. Fixes: #10
- Write to pipes only when the value changes. - Sleep when not reading key event or messages. Fixes: #10
- Write to pipes only when the value changes. - Sleep when not reading key event or messages. Fixes: #10
Reopening this. |
I'll probably need to get a test suit up to measure the cpu usage in a vm environment. |
I think profiling the app could help. I tested on Ubuntu 20.04. Peeking into your code (I'm a Rust beginner) I see a lot of object cloning and threading going on. |
Try 1: 1bfb011 Flamegraph: CPU usage: 17% (for other reasons)
Probably because bash tried to capture the tty which was being read in a blocking way. |
I'll continue working on it after #39 to make sure we don't break anything in order to optimize things. |
Related: crossterm-rs/crossterm#397 |
The suggested solution was to rewrite the codebase in So, this is my 2nd attempt to fix the issue without changing much of the code. @IcyFlamingArrow @amalic @maximbaz can you please test if this works for you? |
Cranking up sleep time will definitely be helpful, but like the author in the other thread said, it could mess up rrsponsiveness. Will give it another try later, and let you know asap. |
It doesn't seem to have much impact in my machine since the sleep time should apply only for the first click. When holding the key, subsequent events should be faster. |
To me it looks like a significant improvement, it dropped from 50% cpu usage in idle to 2.5%. I don't feel any decrease in responsiveness (remember to build in release mode when testing, just in case). |
Awesome. Thanks. |
It will definitely get better as |
Just to keep everyone in the know, a lot of CPU and performance optimizations were made. The latest version uses about 0.1% - 0.7% CPU when idle and also feels a lot snappier.
|
Although, it still has plenty of potential for more optimization. |
in previous message were
here also constantly happens |
Thanks for reporting. I see it went up again. As for why:
|
|
As you can see from the previous comments, it's due to an open crossterm issue.
It's more or less the same, some simple code running on a thread. I'm not sure if it's optimal, but comparing modification times shouldn't cause any major overhead. |
here is similar issue about watching directory in
no, it should be faster and it's not take so much CPU, because it's build in kernel you can check this with inotify-tools: and watch this process in |
Hmm wow didn't know. Regardless, as I understand inotify will make it Linux dependent. I did a simple experiment.
So, I'd first try to optimize reading inputs. Maybe switching the input reading logic to termion might help. |
yes, but you can use cfg attribute to use it only on UNIX |
To consider, cross-platform notify library, uses inotify under the hood for Unix: https://crates.io/crates/notify |
Looks good. But from the experiments in my last comment, we know that the real issue isn't with the watcher. It's the input reading mechanism. I think something changed when I last upgraded crossterm. I'll have a closer look. |
Following https://github.com/crossterm-rs/crossterm/blob/master/examples/event-stream-async-std.rs let's see if we can somehow reduce CPU usage and improve responsiveness using async rust. Ref: #10
Following https://github.com/crossterm-rs/crossterm/blob/master/examples/event-stream-async-std.rs let's see if we can somehow reduce CPU usage and improve responsiveness using async rust. Ref: #10
Following https://github.com/crossterm-rs/crossterm/blob/master/examples/event-stream-async-std.rs let's see if we can somehow reduce CPU usage and improve responsiveness using async rust. Ref: #10
Following https://github.com/crossterm-rs/crossterm/blob/master/examples/event-stream-async-std.rs let's see if we can somehow reduce CPU usage and improve responsiveness using async rust. Ref: #10
NOTE: keeping this issue open to implement notify/inotify for Linux with test cases for all platforms. |
This is an uneducated question, so please take it for what it is, but: Why is polling for input needed at all? I haven't tried the Windows side of things, but can't we just wait for input? (example reader in C); After all, the application usually exits on user input / command, I thought. The problem, I assume, comes if input is read, but parsed/actions executed in a separate thread. Edit: I just noticed just going up and down through contents of a directory uses a lot of CPU (45% when holding, vs 16% in ranger), even in a small dir (24 files). I assume xplr doesn't reload the directory on every action/move, so could all that be caused just by updating the relative item numbers or all the extra file information? |
I guess it's because of the lua function calls along with all the serialisation and deserialization required for that. The number of columns being rendering in the table somewhat impacts scrolling performance. Using plugins like zentable.xplr could be an easy fix, still, each render requires at-least one lua function call. |
Unfortunately, whether manually disabling all columns, except the filename, or using the plugin, the CPU usage on holding down arrow is still ~48%. |
Hmm... I think we can now start porting some of the custom Lua stuff to the Rust side as built-in defaults. That should optimize things significantly, as long as it's not being customized. |
Need to ensure that we don't break compatibility. |
The xlpr file manager results in intense CPU usage when the binary is executed.
How can I avoid this?
https://ibb.co/7j1Rm77
The version I am using is the xrpl-git from the aur.
The text was updated successfully, but these errors were encountered: