Skip to content

Conversation

davewa
Copy link
Contributor

@davewa davewa commented Oct 13, 2025

Performance issue discovered while working on #28238. This PR fixes terminal to only search the hovered line for hyperlinks and adds a benchmark. Before this fix, hyperlink detection grows linearly with terminal content, with this fix it is proportional only to the hovered line. The gains come from replacing visible_regex_match_iter, which searched all visible lines, with code that only searches the line hovered on.

For a terminal with 4,000 lines like:

Compiling terminal v0.1.0 (/Hyperlinks/Bench/Source/zed-hyperlinks/crates/terminal)

Currently (on my M2 MacBook Air):

find_from_grid_point_bench
                        time:   [9.8996 ms 9.9096 ms 9.9250 ms]

This PR:

find_from_grid_point_bench
                        time:   [2.6988 µs 2.7020 µs 2.7053 µs]
                        change: [-99.973% -99.973% -99.973%] (p = 0.00 < 0.05)
                        Performance has improved.

Release Notes:

  • Improved terminal hyperlink performance, especially for terminals with a lot of content visible.

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Oct 13, 2025
@davewa davewa changed the title Improve terminal hyperlink performance terminal: Improve terminal hyperlink performance Oct 13, 2025
@cavebatsofware
Copy link
Contributor

Just looking at this and the history of this issue it appears that the gains you are seeing are coming from criterion/alacrity. I think it would be good to describe at least briefly why those inclusions gain performance here even if it is a guess. The benchmarks are great but the history does benefit from the why explanation when looking back, this information can be valuable looking back sometimes.

@davewa
Copy link
Contributor Author

davewa commented Oct 13, 2025

Thanks for looking at this. The gains come from replacing visible_regex_match_iter which searched all visible lines, with code that only searches the line hovered on. I'll add that to the description.

@cavebatsofware
Copy link
Contributor

At a look I'd approve this. Haven't local tested but it looks solid.

@mikayla-maki
Copy link
Member

I'm not sure this behavior is correct. What happens if you have a link that spans multiple lines? e.g. this:

http://www.reallylong.link/rll/RCY3W1Q80bCOEtPH9D3vzbVGLFdnT_yKcyvYbYFebRVa9DyOvi44KgHKsS_y1LrvQoaBbXnVV7Gv6EorI4xAOga5XnNmZK0J8_94gIzXLgofoIJToUQtp7wRbDz4FcsS/HZ/v00c/Q6T5O1gQyUIyqNfz3YzYMkivVHU8nU1EpTolWKLoF7f_UTA2neriOMLQjxGGthRoOJPs2b5XwuT517vqrvHSHT6q1Rk3yi2FotaCisNcNNV0P/YGyuDRYpaCPbgWkk/32nuqfGSAE1QKz3Lb_xWvRjEfiG46S64ONZpcFrUxlGBdRvkUISUpm71kkU_ZNkvqVcQj0DgbeFvXiP60SF/kyEGvavdNibUyYTJnO3Vp70NXkeeFCZyfC2ZFfotgRFTCRvPyWjVq5t_ZUuRpYAerxCajpbR8X63/cNc9tg0ctUo_GyTmH1/libbBK7350MmL11UAlUeK4Dm/KFxV3VWlr7iXvLnzK0fI4lM9dHC3Nkw7F_F1UC9b_sAWw7Eacq8LfZ3qWRsrdzJlmQwJWfTl32XLr32704wAKgMiQpm7J7iKxNmYM1TUZ3tUo74SeMkSdQZAUl8INS1VaiGi0/cLtC4nC1jUfCPmHLrXq8exJPPsbFPCyqK9fDpb8uyqYWVJsYgppq43pYwbIH_lRl8DnVTKEPhsGzSPq0huBv7nM7HBMAQWbN00K5jxTOzyjIqj4maehn6IlQRvz5Ihl5tc7oxDOevPap3B558kZnHDGgdV/MnvoomBDJjkJY0eufdx2D1TpeQQLb828HJ2PY1nw6csSS9/npVOe/61e_sUe2T5SDLVjxfRUikxZhMbP/8b3Cu66LEOZqZVtMpRjicGGGNVSesRFun08StA8IheowNBQpt1cKRT7ckE7RdB6Ae6sdzpEAPSHaYYpuW/DQEzocZvsx6ldeD3oePmo0PHOQ6e0QqB0Cd4nb8EW3Sh9ZSLka8gZTycucmboyqpPmfYwzZzVWsKV6eQ9Vo1bJXUhunNMVVxlS5lHfEG5f4BfjrULd3HxGelbMWfdwX1PDjw8d5WSwlqDeKWiIYX0xC7LVEgEcOvBTAmx33pnEBygWlshVZ72ldqivqatCFh4BsgvCwhm6SCDKH5UQjGWp6LE95gQBw2b5KHfYndofAon6rNyCd_98C/dvdnXJ/iWYDx9L3kH7NL3Jl6ZXtn57WfEgj7KSzvUJheMJVL/G45fETaXrP5xQXbf9Zm0gi6bKADYwDfK65tp0GMbUPNFIyYi7W7Qb1uCiPcVU10oc6mOWj4cRxTj/WfAihy69pzLXJDPzxTHfPpth5noySPJ6Vpc95_rj_5AijpGz_VxRryV0J1idV/K0Yz0AJ53usT4fWH/PWboil1rC74A8C_50DVKRq1q7aMjizkTvdmS6BNcN2iG_qFihDCGyNDrcVUOt5FlYDKijiWMEOvcmtOBtH1klgV5VMFC1G73gDeJQs7cKBDSehWuSYCijJn5_FZ5PiBOwtX5BO9bHkFk3r4dhZFLUjLedOuL/kPRKhA0J_YRbcSBCd1hz1/M46aMxyQ/2ZGGwAiGmeOa0pknwGdulN9KhNv2zvhUIwgEleIiNbjCHmACqK39D8pKMQFHtJsKJ9s_0MMBo3jilom36XFkI_x9chSA1Em5hBAbQ3RgdeKRfXPYcK3YfkND3Gx6WQH1kF3r6nHUKoNAla4ZqXlbtfJg9Gdoh8k4FK33BSRGcHhualBzMDSsRoWgDhb3ZYPdh5iJrgjFBqNbte8JpQEA/h/4EdBuK33qTV_ToMtnN1khjlgrHb_K7uKAT6if3_DKtnyLqcBpe9lsJHMCYe01aIpw8HT/D9nf7pB1U9lAmgmUS7cWM3gSRcBxWtGF8qZx9KO5D7CPEmnXNyJoB/Crepj6w7vRSapHGd1tNuqjGQKSRzFv2RkT6q7_xHdn03Xg0KtKKYJfcLJhl0Jsbz3Ab

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants