SpinnerHandle::text() is abysmally slow

[Tyrn]

Hi,

Are you aware of that? It's either crossterm, or I'm using the wrong method for updating the spinner message.

Thank you for the update!

[mainrs]

No, I wasn’t aware of it. I actually do not use the text method myself. Now I’m curious on why it’s that slow. I’ll try to investigate at the weekend.

[mainrs]

Can you post the code that you used that resulted in slow execution times?

[Tyrn]

Here is a dedicated test case.

The picture is very suggestive: ten updates a second is ok, no worse than any other spinner; on a hundred it's choking miserably. Some queue gets clogged, probably. I'd add another method to the SpinnerHandle, besides text(). Say, update() or message(), with "no guarantee". The new method will be easy on that queue. I have a suspicion that the corresponding method from the spinner crate works this way. It can be fired 1000 times per second, and performs like the DummySpinner.

[Milo123459]

Hey, I can reproduce this. I'm not sure what's going on. Perhaps try bumping dependencies like crossterm?

[mainrs]

I think it's the way how I pull for new messages inside the queue. I did not have time to properly investigate though.

[Milo123459]

Here is something that is currently happening on my app:

https://user-images.githubusercontent.com/50248166/150021239-2cd563ba-d96d-4b58-a559-d34a532e39ef.mp4

As you can see, when I edit the text to remove a space from the first text, it takes a moment and looks like this, which is incorrect / a visual bug.

[mainrs]

@Milo123459

Do you mean the small indent that happens? The prompt moves to the left and then shows the done symbol? Do you also update using text()? Or do you call done()?

[Milo123459]

I call .text then .done

[Milo123459]

Is there any way I can help in resolving this issue? It's rather annoying and bothering me a little bit in my apps 😅

[mainrs]

Maybe trying out a profiler like flamegraph might reveal more. It might be due to the animation delay that each animation has. An animation consists of frames and a duration that should be slept during each frame. If you take a look here: https://github.com/mainrs/terminal-spinners-rs/blob/d384d10a993f3a50dff4ca9e2f862346fb240554/src/lib.rs#L167

The render thread waits for that duration before processing the next frame. And only during the next frame the text changes. So it might be that.

This can probably be easy tested by creating a custom animation and setting the duration really high (like 3 seconds or so) and try to change the frame. If it takes 3 seconds, it's definitely the above problem.

[Milo123459]

Hey, I can't work on this for a while (sadly). Perhaps @mainrs you could take a look into it? This is sadly proving to be quite annoying and I have had some people report this to me and say that it's very offputting 😔

[Milo123459]

@mainrs I hate to ask, but do you have any updates?

[mainrs]

Nope, haven't looked into it. I was hoping that you would to be honest. I'll report back on Monday. I might have a fix for this.

[Milo123459]

Thanks for the update!

Sorry for not being able to work on this.

[mainrs]

Don't worry, I just assumed things. And no problem! I'll ping you on Monday.

[mainrs]

I think I have found a solution, I need to test it later on though

[Milo123459]

Hey! Can I help test it?

[mainrs]

The solution I had in mind sadly didn't work out! I've written a test case that checks for this specific issue. I'll push it later on to the branch.

I do have another idea that I'll try later on these day.

[Milo123459]

Do keep us updated!

[Milo123459]

@mainrs have you had a chance to look at this?

[mainrs]

Nop, I currently am working on my thesis for university and won't have time till October. I've found another crate on reddit yesterday: https://github.com/ad4mx/spinoff

It uses the same idea as I did but takes advantage of an AtomicBool to immediately stop the animation without the lag. I do not know if it exhibits the same issues as this crate does with text changes.

After submitting my thesis I will definitely try to figure this out. I need it myself in a lot of projects. I just did not have the proper time and motivation. Sorry.

[Milo123459]

That's fine! Good luck with uni, and thanks for sharing that other crate.

[ad4mx]

Here is something that is currently happening on my app: 482cxCTd.mp4

As you can see, when I edit the text to remove a space from the first text, it takes a moment and looks like this, which is incorrect / a visual bug.

Hi, maintainer of ad4mx/spinoff here. If I properly understood the problem, the fix in my crate could be something like this:

...
let sp = Spinner::new(Spinners::Dots, "cargo fmt", None);
sleep(Duration::from_secs(1));
sp.success(format!("cargo fmt {}", time_counted).into());;

This code should basically function the same, without the graphical bugs.

This is because .success() stops the thread corresponding to the spinner, deletes the last line, and replaces it with a check mark and a message.

By the way, I really like the idea of glitter!

[ad4mx]

By the way, I could add an equivalent to the ::text() function to my crate if you want.

[Milo123459]

By the way, I really like the idea of glitter!

Thanks for the kind words about glitter! :)

[Milo123459]

By the way, I could add an equivalent to the ::text() function to my crate if you want.

Yeah, that'd be super helpful!

[ad4mx]

Hi, I'll try to add it in 0.6 since I have something else already in the works for 0.5.

[ad4mx]

@Milo123459 Hi, it's done! https://github.com/ad4mx/spinoff/releases/tag/v0.5.1

[Milo123459]

Thank you!

[Milo123459]

Was just using it now, when I realised spinoff seems to be adding a space before the spinner. Any ideas?

PS: I'm using Spinnners::Dots.

[mainrs]

Would you mind opening an issue on the spinoff repository for that? @Milo123459