Skip to content

terminal: Improve scroll performance#44714

Merged
probably-neb merged 4 commits intozed-industries:mainfrom
davewa:terminal-scroll-perf
Dec 16, 2025
Merged

terminal: Improve scroll performance#44714
probably-neb merged 4 commits intozed-industries:mainfrom
davewa:terminal-scroll-perf

Conversation

@davewa
Copy link
Contributor

@davewa davewa commented Dec 12, 2025

Related to:

Previously we were searching for hyperlinks on every scroll, even if Cmd was not held. With this PR,

  • We only search for hyperlinks on scroll if Cmd is held
  • We now clear last_hovered_word in all cases where Cmd is not held
  • Renamed word_from_position -> schedule_find_hyperlink
  • Simplified logic in schedule_find_hyperlink

Performance measurements

The test scrolls up and down 20,000x in a loop. However, since this PR is just removing a code path that was very dependent on the length of the line in terminal, it's not super meaningful as a comparison. The test uses a line length of "long line ".repeat(1000), and in main the performance is directly proportional to the line length, so for benchmarking it in main it only scrolls up and down 20x. I think all that is really useful to say is that currently scrolling is slow, and proportional to the line length, and with this PR it is buttery-smooth and unaffected by line length. I've included a few data points below anyway. At least the test can help catch future regressions.

Branch Command Scrolls Iter/sec Mean [ms] SD [ms] Iterations Importance (weight)
main tests::perf::scroll_long_line_benchmark 40 16.85 712.00 2.80 12 average (50)
this PR tests::perf::scroll_long_line_benchmark 40 116.22 413.60 0.50 48 average (50)
this PR tests::perf::scroll_long_line_benchmark 40,000 9.19 1306.40 7.00 12 average (50)
only overhead tests::perf::scroll_long_line_benchmark 0 114.29 420.90 2.00 48 average (50)

Release Notes:

  • terminal: Improved scroll performance

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Dec 12, 2025
@github-actions github-actions bot added the community champion Issues filed by our amazing community champions! 🫶 label Dec 12, 2025
@github-project-automation github-project-automation bot moved this to Community Champion PRs in Quality Week – December 2025 Dec 12, 2025
@MrSubidubi MrSubidubi changed the title terminal: Improve scroll perf terminal: Improve scroll performance Dec 13, 2025
@davewa
Copy link
Contributor Author

davewa commented Dec 13, 2025

I thought maybe in this case I could be okay with submitting a performance PR without performance measurements, but I ... just ... can't ... lol. Putting this back into draft to add a performance test.

@davewa davewa marked this pull request as draft December 13, 2025 15:51
@franciskafyi
Copy link
Collaborator

We love tests! @davewa ping me when this is ready and I'll make sure we get to this during Quality Week.

@davewa davewa marked this pull request as ready for review December 13, 2025 22:37
@davewa
Copy link
Contributor Author

davewa commented Dec 13, 2025

@franciskafyi fyi, ready for review now.

@probably-neb
Copy link
Collaborator

I'm ready to merge this when you are @davewa

@probably-neb probably-neb moved this from Community Champion PRs to In progress in Quality Week – December 2025 Dec 16, 2025
@davewa
Copy link
Contributor Author

davewa commented Dec 16, 2025

I’m ready!

@probably-neb probably-neb merged commit 73b37e9 into zed-industries:main Dec 16, 2025
24 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in Quality Week – December 2025 Dec 16, 2025
@probably-neb
Copy link
Collaborator

Great work as usual @davewa!

@davewa davewa deleted the terminal-scroll-perf branch December 18, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement community champion Issues filed by our amazing community champions! 🫶

Projects

Development

Successfully merging this pull request may close these issues.

3 participants