Skip to content
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.

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

Bump anyhow to 1.0.80 and drop backtrace crate #263

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

DavisVaughan
Copy link
Contributor

@DavisVaughan DavisVaughan commented Mar 6, 2024

On the Windows VM, the indexer::start() command we use in LSP initialization takes over 40 seconds to complete when run in the vctrs/ project. On a Mac, this typically takes a few milliseconds.

This is a bigger difference than just computer horsepower.

The way the indexer code is written is a little strange, index_function() and index_comment() end up propagating an anyhow error upwards in the extremely common cases where no match is found, even though this error is immediately discarded in index_node(). These anyhow errors always capture backtraces from what I can tell, so they actually have non zero cost to create, in particular on Windows apparently! The index_function/comment() functions are called hundreds of time, so this adds up.

With anyhow 1.0.71, the backtrace crate was used for backtrace capture, but in >=1.0.77 with Rust >=1.65, it started using the standard library's new backtrace support.
dtolnay/anyhow#293
rust-lang/rust#64154

They also encourage you to stop enabling the backtrace feature, as it is always on in Rust >=1.65 now:

enabling the backtrace crate feature does nothing essentially (except downloading the dependency, so don't do that).

Switching to anyhow 1.0.80 (current release) somehow ends up fixing the performance issue. My guess is that std backtraces are either lazier or just way faster to capture on windows.

I consider this a "quick fix" and intend to rewrite the indexer code in a follow up PR to have an API that works more off Option than Result, avoiding this entirely (hopefully making it faster on all platforms).


Side note, I originally thought this was related to this anyhow performance regression:
dtolnay/anyhow#347

But that seems to actually be unrelated and is a different scenario (there upgrading to 1.0.80 actually hurt performance, i think they previously were not capturing backtraces at all and all of a sudden they started to do so)

@DavisVaughan DavisVaughan merged commit fe5f7cb into main Mar 6, 2024
1 check passed
@DavisVaughan DavisVaughan deleted the feature/bump-anyhow branch March 6, 2024 22:28
@kevinushey
Copy link
Contributor

Belated LGTM; thanks for running that down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants