Skip to content

terminal: Fix performance issues with hyperlink regex matching #44407

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.

Sign up for GitHub

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

Jump to bottom
Merged
osiewicz merged 6 commits into from
Dec 8, 2025
Merged

terminal: Fix performance issues with hyperlink regex matching #44407

osiewicz merged 6 commits into from
Dec 8, 2025

Conversation

@osiewicz
Copy link
Contributor

@osiewicz osiewicz commented Dec 8, 2025 •
edited
Loading

Problem statement: When given a line that contained a lot of matches of
your hyperlink regex of choice (thanks to #40305), we would look for matches
that intersected with currently hovered point. This is hella
expensive, because we would re-walk the whole alacritty grid for each
match. With the repro that Joseph shared, we had to go through 4000 such
matches on each frame render.

Problem solution: We now convert the hovered point into a range within
the line (byte-wise) in order to throw away matches that do not intersect the
hovered range. This lets us avoid performing the unnecessary conversion
when we know it's never going to yield a match range that intersects the
hovered point.

Release Notes:

  • terminal: Fixed performance regression when handling long lines.
@osiewicz
terminal: Fix performance issues with hyperlink regex matching
82e00d1 
Problem statement: When given a line that contained a lot of matches of
your hyperlink regex of choice (thanks to , we would look for matches
that intersected with currently hovered point. This is *hella*
expensive, because we would re-walk the whole alacritty grid for each
match. With the repro that Joseph shared, we had to go through 4000 such
matches on each frame render.

Problem solution: We now convert the hovered point into a range within
the line in order to throw away matches that do not intersect the
hovered range. This lets us avoid performing the unnecessary conversion
when we know it's never going to yield a match range that intersects the
hovered point.
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Dec 8, 2025
@osiewicz osiewicz requested review from Veykril and benbrandt December 8, 2025 15:05
osiewicz
osiewicz commented Dec 8, 2025
View reviewed changes
davewa
davewa suggested changes Dec 8, 2025
View reviewed changes
@osiewicz osiewicz requested a review from davewa December 8, 2025 16:00
@osiewicz osiewicz force-pushed the terminal-regex-fixes branch from 50e1fb3 to b133d08 Compare December 8, 2025 18:50
davewa
davewa approved these changes Dec 8, 2025
View reviewed changes
Copy link
Contributor

@davewa davewa left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for finding and fixing this regression, I'll be adding a perf test for this scenario soon in a separate PR.
@osiewicz
Copy link
Contributor Author

osiewicz commented Dec 8, 2025

@davewa thanks for a thorough review!
@osiewicz osiewicz merged commit 3d6cc3d into main Dec 8, 2025
23 checks passed
@osiewicz osiewicz deleted the terminal-regex-fixes branch December 8, 2025 20:03
@osiewicz
Copy link
Contributor Author

osiewicz commented Dec 8, 2025

/cherry-pick preview
github-actions bot pushed a commit that referenced this pull request Dec 8, 2025
@osiewicz @davewa
terminal: Fix performance issues with hyperlink regex matching (#44407)
9614f9b 
Problem statement: When given a line that contained a lot of matches of
your hyperlink regex of choice (thanks to #40305), we would look for
matches
that intersected with currently hovered point. This is *hella*
expensive, because we would re-walk the whole alacritty grid for each
match. With the repro that Joseph shared, we had to go through 4000 such
matches on each frame render.

Problem solution: We now convert the hovered point into a range within
the line (byte-wise) in order to throw away matches that do not
intersect the
hovered range. This lets us avoid performing the unnecessary conversion
when we know it's never going to yield a match range that intersects the
hovered point.

Release Notes:

- terminal: Fixed performance regression when handling long lines.

---------

Co-authored-by: Dave Waggoner <waggoner.dave@gmail.com>
@zed-zippy zed-zippy bot mentioned this pull request Dec 8, 2025
Merged
zed-zippy bot added a commit that referenced this pull request Dec 8, 2025
@zed-zippy @osiewicz @davewa
terminal: Fix performance issues with hyperlink regex matching (#44407)…
716983b 
… (cherry-pick to preview) (#44423)

Cherry-pick of #44407 to preview

----
Problem statement: When given a line that contained a lot of matches of
your hyperlink regex of choice (thanks to #40305), we would look for
matches
that intersected with currently hovered point. This is *hella*
expensive, because we would re-walk the whole alacritty grid for each
match. With the repro that Joseph shared, we had to go through 4000 such
matches on each frame render.

Problem solution: We now convert the hovered point into a range within
the line (byte-wise) in order to throw away matches that do not
intersect the
hovered range. This lets us avoid performing the unnecessary conversion
when we know it's never going to yield a match range that intersects the
hovered point.

Release Notes:

- terminal: Fixed performance regression when handling long lines.

---------

Co-authored-by: Dave Waggoner <waggoner.dave@gmail.com>

Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com>
Co-authored-by: Dave Waggoner <waggoner.dave@gmail.com>
This was referenced Dec 11, 2025
Closed
Closed
@osiewicz
Copy link
Contributor Author

osiewicz commented Dec 11, 2025

/cherry-pick stable
@zed-zippy
Copy link
Contributor

zed-zippy bot commented Dec 11, 2025

🍒💥 Cherry-pick did not succeed
https://github.com/zed-industries/zed/actions/runs/20137844778
@Zachiah Zachiah mentioned this pull request Dec 11, 2025
Closed
This was referenced Dec 12, 2025
Merged
Merged
CherryWorm pushed a commit to CherryWorm/zed that referenced this pull request Dec 16, 2025
@osiewicz @davewa @CherryWorm
terminal: Fix performance issues with hyperlink regex matching (zed-i…
e936a3c 
…ndustries#44407)

Problem statement: When given a line that contained a lot of matches of
your hyperlink regex of choice (thanks to zed-industries#40305), we would look for
matches
that intersected with currently hovered point. This is *hella*
expensive, because we would re-walk the whole alacritty grid for each
match. With the repro that Joseph shared, we had to go through 4000 such
matches on each frame render.

Problem solution: We now convert the hovered point into a range within
the line (byte-wise) in order to throw away matches that do not
intersect the
hovered range. This lets us avoid performing the unnecessary conversion
when we know it's never going to yield a match range that intersects the
hovered point.

Release Notes:

- terminal: Fixed performance regression when handling long lines.

---------

Co-authored-by: Dave Waggoner <waggoner.dave@gmail.com>
probably-neb pushed a commit that referenced this pull request Dec 16, 2025
@davewa
terminal: Improve scroll performance (#44714)
73b37e9 
Related to: 
- #44510 
- #44407 

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
someone13574 pushed a commit to someone13574/zed that referenced this pull request Dec 16, 2025
@osiewicz @davewa @someone13574
terminal: Fix performance issues with hyperlink regex matching (zed-i…
ace55be 
…ndustries#44407)

Problem statement: When given a line that contained a lot of matches of
your hyperlink regex of choice (thanks to zed-industries#40305), we would look for
matches
that intersected with currently hovered point. This is *hella*
expensive, because we would re-walk the whole alacritty grid for each
match. With the repro that Joseph shared, we had to go through 4000 such
matches on each frame render.

Problem solution: We now convert the hovered point into a range within
the line (byte-wise) in order to throw away matches that do not
intersect the
hovered range. This lets us avoid performing the unnecessary conversion
when we know it's never going to yield a match range that intersects the
hovered point.

Release Notes:

- terminal: Fixed performance regression when handling long lines.

---------

Co-authored-by: Dave Waggoner <waggoner.dave@gmail.com>
probably-neb pushed a commit that referenced this pull request Dec 17, 2025
@davewa
terminal: Improve regex hyperlink performance for long lines (#44721)
8aab646 
Related to
- #44407

This PR further improves performance for regex hyperlink finding by
eliminating unnecessary regex matching. Currently, we repeatedly search
for matches from the start of the line until the match contains the
hovered point. This is only required to support custom regexes which
match strings containing spaces, with multiple matches on a single line.
This isn't actually a useful scenario, and is no longer supported. This
PR changes to only search twice, the first match starting from the start
of the line, and the hovered word (space-delimited). The most dramatic
improvement is for long lines with many words.

In addition to the above changes, this PR:
- Adds test for the scenarios from #44407 and #44510 
- Simplifies the logic added in #44407

Performance measurements

For the scenario from #44407, this improves the perf test's iteration
time from 1.22ms to 0.47ms.

main:

| Branch | Command | Iter/sec | Mean [ms] | SD [ms] | Iterations |
Importance (weight) |
|:---|:---|---:|---:|---:|---:|---:|
| main |
terminal_hyperlinks::tests::path::perf::pr_44407_hyperlink_benchmark |
819.64 | 937.60 | 2.20 | 768 | average (50) |
| this PR |
terminal_hyperlinks::tests::path::perf::pr_44407_hyperlink_benchmark |
2099.79 | 1463.20 | 7.20 | 3072 | average (50) |

Release Notes:

- terminal: Improve path hyperlink performance for long lines
HactarCE pushed a commit that referenced this pull request Dec 17, 2025
@davewa @HactarCE
terminal: Improve regex hyperlink performance for long lines (#44721)
690d56f 
Related to
- #44407

This PR further improves performance for regex hyperlink finding by
eliminating unnecessary regex matching. Currently, we repeatedly search
for matches from the start of the line until the match contains the
hovered point. This is only required to support custom regexes which
match strings containing spaces, with multiple matches on a single line.
This isn't actually a useful scenario, and is no longer supported. This
PR changes to only search twice, the first match starting from the start
of the line, and the hovered word (space-delimited). The most dramatic
improvement is for long lines with many words.

In addition to the above changes, this PR:
- Adds test for the scenarios from #44407 and #44510 
- Simplifies the logic added in #44407

Performance measurements

For the scenario from #44407, this improves the perf test's iteration
time from 1.22ms to 0.47ms.

main:

| Branch | Command | Iter/sec | Mean [ms] | SD [ms] | Iterations |
Importance (weight) |
|:---|:---|---:|---:|---:|---:|---:|
| main |
terminal_hyperlinks::tests::path::perf::pr_44407_hyperlink_benchmark |
819.64 | 937.60 | 2.20 | 768 | average (50) |
| this PR |
terminal_hyperlinks::tests::path::perf::pr_44407_hyperlink_benchmark |
2099.79 | 1463.20 | 7.20 | 3072 | average (50) |

Release Notes:

- terminal: Improve path hyperlink performance for long lines
rtfeldman pushed a commit that referenced this pull request Jan 5, 2026
@davewa
terminal: Improve regex hyperlink performance for long lines (#44721)
16422ca 
Related to
- #44407

This PR further improves performance for regex hyperlink finding by
eliminating unnecessary regex matching. Currently, we repeatedly search
for matches from the start of the line until the match contains the
hovered point. This is only required to support custom regexes which
match strings containing spaces, with multiple matches on a single line.
This isn't actually a useful scenario, and is no longer supported. This
PR changes to only search twice, the first match starting from the start
of the line, and the hovered word (space-delimited). The most dramatic
improvement is for long lines with many words.

In addition to the above changes, this PR:
- Adds test for the scenarios from #44407 and #44510 
- Simplifies the logic added in #44407

Performance measurements

For the scenario from #44407, this improves the perf test's iteration
time from 1.22ms to 0.47ms.

main:

| Branch | Command | Iter/sec | Mean [ms] | SD [ms] | Iterations |
Importance (weight) |
|:---|:---|---:|---:|---:|---:|---:|
| main |
terminal_hyperlinks::tests::path::perf::pr_44407_hyperlink_benchmark |
819.64 | 937.60 | 2.20 | 768 | average (50) |
| this PR |
terminal_hyperlinks::tests::path::perf::pr_44407_hyperlink_benchmark |
2099.79 | 1463.20 | 7.20 | 3072 | average (50) |

Release Notes:

- terminal: Improve path hyperlink performance for long lines
LivioGama pushed a commit to LivioGama/zed that referenced this pull request Jan 20, 2026
@davewa
terminal: Improve regex hyperlink performance for long lines (zed-ind…
5202c99 
…ustries#44721)

Related to
- zed-industries#44407

This PR further improves performance for regex hyperlink finding by
eliminating unnecessary regex matching. Currently, we repeatedly search
for matches from the start of the line until the match contains the
hovered point. This is only required to support custom regexes which
match strings containing spaces, with multiple matches on a single line.
This isn't actually a useful scenario, and is no longer supported. This
PR changes to only search twice, the first match starting from the start
of the line, and the hovered word (space-delimited). The most dramatic
improvement is for long lines with many words.

In addition to the above changes, this PR:
- Adds test for the scenarios from zed-industries#44407 and zed-industries#44510 
- Simplifies the logic added in zed-industries#44407

Performance measurements

For the scenario from zed-industries#44407, this improves the perf test's iteration
time from 1.22ms to 0.47ms.

main:

| Branch | Command | Iter/sec | Mean [ms] | SD [ms] | Iterations |
Importance (weight) |
|:---|:---|---:|---:|---:|---:|---:|
| main |
terminal_hyperlinks::tests::path::perf::pr_44407_hyperlink_benchmark |
819.64 | 937.60 | 2.20 | 768 | average (50) |
| this PR |
terminal_hyperlinks::tests::path::perf::pr_44407_hyperlink_benchmark |
2099.79 | 1463.20 | 7.20 | 3072 | average (50) |

Release Notes:

- terminal: Improve path hyperlink performance for long lines
LivioGama pushed a commit to LivioGama/zed that referenced this pull request Jan 20, 2026
@davewa
terminal: Improve regex hyperlink performance for long lines (zed-ind…
4b1e92c 
…ustries#44721)

Related to
- zed-industries#44407

This PR further improves performance for regex hyperlink finding by
eliminating unnecessary regex matching. Currently, we repeatedly search
for matches from the start of the line until the match contains the
hovered point. This is only required to support custom regexes which
match strings containing spaces, with multiple matches on a single line.
This isn't actually a useful scenario, and is no longer supported. This
PR changes to only search twice, the first match starting from the start
of the line, and the hovered word (space-delimited). The most dramatic
improvement is for long lines with many words.

In addition to the above changes, this PR:
- Adds test for the scenarios from zed-industries#44407 and zed-industries#44510 
- Simplifies the logic added in zed-industries#44407

Performance measurements

For the scenario from zed-industries#44407, this improves the perf test's iteration
time from 1.22ms to 0.47ms.

main:

| Branch | Command | Iter/sec | Mean [ms] | SD [ms] | Iterations |
Importance (weight) |
|:---|:---|---:|---:|---:|---:|---:|
| main |
terminal_hyperlinks::tests::path::perf::pr_44407_hyperlink_benchmark |
819.64 | 937.60 | 2.20 | 768 | average (50) |
| this PR |
terminal_hyperlinks::tests::path::perf::pr_44407_hyperlink_benchmark |
2099.79 | 1463.20 | 7.20 | 3072 | average (50) |

Release Notes:

- terminal: Improve path hyperlink performance for long lines
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

5 participants

@osiewicz @benbrandt @Veykril @davewa