user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
my rust fork https://www.rust-lang.org
23033 commits 10 branches 154 tags 1.3 GiB
Find a file
bors bc131422e9 Auto merge of #13222 - BorysMinaiev:master, r=flodiebold 
Remove redundant 'resolve_obligations_as_possible' call

Hi! I was looking for a "good first issue" and saw this one: https://github.com/rust-lang/rust-analyzer/issues/7542. I like searching for performance improvements, so I wanted to try to find something useful there.

There are two tests in integrated_benchmarks.rs, I looked at 'integrated_highlighting_benchmark' (not the one discussed in the issue above).

Profile from that test looks like this:
```
$ RUN_SLOW_BENCHES=1 cargo test --release --package rust-analyzer --lib -- integrated_benchmarks::integrated_highlighting_benchmark --exact --nocapture
    Finished release [optimized] target(s) in 0.06s
     Running unittests src/lib.rs (target/release/deps/rust_analyzer-a80ca6bb8f877458)

running 1 test
workspace loading: 358.45ms
initial: 9.60s
change: 13.96µs
cpu profiling is disabled, uncomment `default = [ "cpu_profiler" ]` in Cargo.toml to enable.
  273ms - highlight
      143ms - infer:wait @ per_query_memory_usage
          143ms - infer_query
                0   - crate_def_map:wait (3165 calls)
                4ms - deref_by_trait (967 calls)
               96ms - resolve_obligations_as_possible (22106 calls)
                0   - trait_solve::wait (2068 calls)
       21ms - Semantics::analyze_impl (18 calls)
        0   - SourceBinder::to_module_def (20 calls)
       36ms - classify_name (19 calls)
       19ms - classify_name_ref (308 calls)
        0   - crate_def_map:wait (461 calls)
        4ms - descend_into_macros (628 calls)
        0   - generic_params_query (4 calls)
        0   - impl_data_with_diagnostics_query (1 calls)
       45ms - infer:wait (37 calls)
        0   - resolve_obligations_as_possible (2 calls)
        0   - source_file_to_def (1 calls)
        0   - trait_solve::wait (42 calls)
after change: 275.23ms
test integrated_benchmarks::integrated_highlighting_benchmark ... ok
```
22106 calls to `resolve_obligations_as_possible` seem like the main issue there.

One thing I noticed (and fixed in this PR) is that `InferenceContext::resolve_ty_shallow` first calls `resolve_obligations_as_possible`, and then calls `InferenceTable::resolve_ty_shallow`. But `InferenceTable::resolve_ty_shallow` [inside](2e9f1204ca/crates/hir-ty/src/infer/unify.rs (L372)) again calls `resolve_obligations_as_possible`.

`resolve_obligations_as_possible` inside has a while loop, which works until it can't find any helpful information. So calling this function for the second time does nothing, so one of the calls could be safely removed.

`InferenceContext::resolve_ty_shallow` is actually quite a hot place, and after fixing it, the total number of `resolve_obligations_as_possible` in this test is reduced to 15516 (from 22106). "After change" time also improves from ~270ms to ~240ms, which is not a very huge win, but still something measurable.

Same profile after PR:
```
$ RUN_SLOW_BENCHES=1 cargo test --release --package rust-analyzer --lib -- integrated_benchmarks::integrated_highlighting_benchmark --exact --nocapture
    Finished release [optimized] target(s) in 0.06s
     Running unittests src/lib.rs (target/release/deps/rust_analyzer-a80ca6bb8f877458)

running 1 test
workspace loading: 339.86ms
initial: 9.28s
change: 10.69µs
cpu profiling is disabled, uncomment `default = [ "cpu_profiler" ]` in Cargo.toml to enable.
  236ms - highlight
      110ms - infer:wait @ per_query_memory_usage
          110ms - infer_query
                0   - crate_def_map:wait (3165 calls)
                4ms - deref_by_trait (967 calls)
               64ms - resolve_obligations_as_possible (15516 calls)
                0   - trait_solve::wait (2068 calls)
       21ms - Semantics::analyze_impl (18 calls)
        0   - SourceBinder::to_module_def (20 calls)
       34ms - classify_name (19 calls)
       18ms - classify_name_ref (308 calls)
        0   - crate_def_map:wait (461 calls)
        3ms - descend_into_macros (628 calls)
        0   - generic_params_query (4 calls)
        0   - impl_data_with_diagnostics_query (1 calls)
       45ms - infer:wait (37 calls)
        0   - resolve_obligations_as_possible (2 calls)
        0   - source_file_to_def (1 calls)
        0   - trait_solve::wait (42 calls)
after change: 238.15ms
test integrated_benchmarks::integrated_highlighting_benchmark ... ok
```

The performance of this test could be further improved but at the cost of making code more complicated, so I wanted to check if such a change is desirable before sending another PR.

`resolve_obligations_as_possible` is actually called a lot of times even when no new information was provided. As I understand, `resolve_obligations_as_possible` could do something useful only if some variables/values were unified since the last check. We can store a boolean variable inside `InferenceTable`, which indicates if `try_unify` was called after last `resolve_obligations_as_possible`. If it wasn't called, we can safely not call `resolve_obligations_as_possible` again.

I tested this change locally, and it reduces the number of `resolve_obligations_as_possible` to several thousand (it is not shown in the profile anymore, so don't know the exact number), and the total time is reduced to ~180ms. Here is a generated profile:
```
$ RUN_SLOW_BENCHES=1 cargo test --release --package rust-analyzer --lib -- integrated_benchmarks::integrated_highlighting_benchmark --exact --nocapture
    Finished release [optimized] target(s) in 0.06s
     Running unittests src/lib.rs (target/release/deps/rust_analyzer-a80ca6bb8f877458)

running 1 test
workspace loading: 349.92ms
initial: 8.56s
change: 11.32µs
cpu profiling is disabled, uncomment `default = [ "cpu_profiler" ]` in Cargo.toml to enable.
  175ms - highlight
       21ms - Semantics::analyze_impl (18 calls)
        0   - SourceBinder::to_module_def (20 calls)
       33ms - classify_name (19 calls)
       17ms - classify_name_ref (308 calls)
        0   - crate_def_map:wait (461 calls)
        3ms - descend_into_macros (628 calls)
        0   - generic_params_query (4 calls)
        0   - impl_data_with_diagnostics_query (1 calls)
       97ms - infer:wait (38 calls)
        0   - resolve_obligations_as_possible (2 calls)
        0   - source_file_to_def (1 calls)
        0   - trait_solve::wait (42 calls)
after change: 177.04ms
test integrated_benchmarks::integrated_highlighting_benchmark ... ok
```
Let me know if adding a new bool field seems like a reasonable tradeoff, so I can send a PR.
2022-09-12 11:30:24 +00:00
.cargo Add CARGO_WORKSPACE_DIR env var in .cargo/config.toml 2022-07-19 13:02:16 +02:00
.github use rustup minimal profile and add curl retries 2022-09-10 23:43:33 -07:00
.vscode Take into account renamed extension id when launching 2022-08-12 18:06:58 +02:00
assets Automatically change text color in logo based on dark mode 2022-03-06 23:06:53 +11:00
bench_data Add benchmark test for mbe 2021-02-25 05:47:13 +08:00
crates Remove redundant 'resolve_obligations_as_possible' call 2022-09-12 11:03:44 +01:00
docs Add a short blurb about VS Code Live Share to the manual 2022-08-23 16:47:26 +02:00
editors/code Auto merge of #13133 - Veykril:diag-hack, r=Veykril 2022-08-28 09:56:06 +00:00
lib Lift out the module scope into a field in the Resolver 2022-09-02 17:02:12 +02:00
xtask Use the release branch in xtask promote 2022-08-03 09:48:44 +03:00
.editorconfig prettier config 2022-05-17 18:12:49 +01:00
.git-blame-ignore-revs add prettier format commit to ignored revisions 2022-05-17 18:15:51 +01:00
.gitattributes Fix .gitattributes for test_data 2022-07-24 14:05:35 +02:00
.gitignore internal: add "Shuffle Crate Graph" command 2021-12-07 16:37:19 +01:00
Cargo.lock Make use of NoHash hashing for FileId and CrateId 2022-08-25 20:41:49 +02:00
Cargo.toml style: rename crates to kebab case 2022-05-01 10:48:58 +00:00
LICENSE-APACHE Licenses 2018-01-10 22:47:04 +03:00
LICENSE-MIT Licenses 2018-01-10 22:47:04 +03:00
PRIVACY.md Update privacy note 2021-12-23 14:04:15 +02:00
README.md Fix r-a spelling in some places 2022-08-01 13:47:09 +02:00
rustfmt.toml Remove forcing \n via rustfmt 2019-11-02 22:19:59 +03:00
triagebot.toml Add triagebot.toml 2022-04-21 17:43:12 +02:00

README.md

rust-analyzer logo

rust-analyzer is a modular compiler frontend for the Rust language. It is a part of a larger rls-2.0 effort to create excellent IDE support for Rust.

Quick Start

https://rust-analyzer.github.io/manual.html#installation

Documentation

If you want to contribute to rust-analyzer or are just curious about how things work under the hood, check the ./docs/dev folder.

If you want to use rust-analyzer's language server with your editor of choice, check the manual folder. It also contains some tips & tricks to help you be more productive when using rust-analyzer.

Security and Privacy

See the corresponding sections of the manual.

Communication

For usage and troubleshooting requests, please use "IDEs and Editors" category of the Rust forum:

https://users.rust-lang.org/c/ide/14

For questions about development and implementation, join rust-analyzer working group on Zulip:

https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Frust-analyzer

Quick Links

License

rust-analyzer is primarily distributed under the terms of both the MIT license and the Apache License (Version 2.0).

See LICENSE-APACHE and LICENSE-MIT for details.