use a dedicated render thread for indicatif spinner#23279
Draft
cburroughs wants to merge 1 commit into
Draft
Conversation
Previously we had the classic combination of doing "real work" and trying to keep a UI snappy on the same (scheduler) thread. This results in jittery spinners that can be sluggish under load. This change splits it out so there is a dedicated render thread. This matches the architecture used by the `experimental-prodash` renderer. The "obvious" alternative to get a steady tick would be `enable_steady_tick`, but that was rejected in pantsbuild#19931 due to desync concerns. Using the large repo from pantsbuild#23236 ``` $ asciinema record data/baseline.cast --command '_PANTS_VERSION_OVERRIDE=2.31.0 PANTS_DEBUG= /home/ecsb/src/o/pants-trees/baseline/pants --no-pantsd --changed-since=HEAD~1 --changed-dependents=transitive list' ``` baseline: https://asciinema.org/a/b67xISILRXqBQl4U after: https://asciinema.org/a/DK0s3SB6ISjab6ji Notice: I used a LLM to try a variety of approaches to reduce jitter and to write the code for this particular restructuring. It is both the most clearly helpful, and a building block for anything else. I also used a LLM to write a script to analyze asciinema logs, but in this case the visual outcome was obvious.
Contributor
Author
|
@jsanders would you mind taking a look at this rust code? |
jsanders
reviewed
Apr 30, 2026
jsanders
left a comment
jsanders
left a comment
There was a problem hiding this comment.
This all looks very reasonable to me!
| cv.notify_all(); | ||
| } | ||
| if let Some(h) = self.join_handle.take() { | ||
| let _ = h.join(); |
There was a problem hiding this comment.
Arguably this could catch a panic in the thread, e.g. if let Err(e) = h.join() { log::warn!("indicatif render thread panicked: {e:?}"); }. But I think it's pretty common to ignore this. Up to you.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Previously we had the classic combination of doing "real work" and trying to keep a UI snappy on the same (scheduler) thread. This results in jittery spinners that can be sluggish under load.
This change splits it out so there is a dedicated render thread. This matches the architecture used by the
experimental-prodashrenderer. The "obvious" alternative to get a steady tick would beenable_steady_tick, but that was rejected in #19931 due to desync concerns.Using the large repo from #23236
baseline: https://asciinema.org/a/b67xISILRXqBQl4U
after: https://asciinema.org/a/DK0s3SB6ISjab6ji
Notice: I used a LLM to try a variety of approaches to reduce jitter and to write the code for this particular restructuring. It is both the most clearly helpful, and a building block for anything else. I also used a LLM to write a script to analyze asciinema logs, but in this case the visual outcome was obvious.