Start indexing and resolving Rubydex graph#3992
Start indexing and resolving Rubydex graph#3992vinistock wants to merge 1 commit intorubydex_adoption_feature_branchfrom
Conversation
d3fa68d to
c7cf7ea
Compare
4d27e4e to
5eae955
Compare
c7cf7ea to
de5e731
Compare
de5e731 to
e0a1f4a
Compare
| progress("indexing-progress", message: "Indexing workspace...") | ||
| @global_state.graph.index_workspace | ||
|
|
||
| progress("indexing-progress", message: "Resolving graph...") | ||
| @global_state.graph.resolve |
There was a problem hiding this comment.
Should this be inside its own Thread.new block as well, like the existing indexing? Or is Rubydex handling it in some other way
There was a problem hiding this comment.
The old indexing happens inside a thread because it's too slow and we don't want to block requests while we're still indexing.
Rubydex is fast enough that we don't have to run indexing in a thread, which will simplify a lot of our code.
For example, right now, we need code to bust the code lens cache and trigger a refresh after indexing because we may have computed incorrect lenses if a request comes in before indexing is finished.
There was a problem hiding this comment.
Right now it still takes 30s to index core using rubydex, and even if we cut it down to 20s I'd argue that it's still a noticeable wait.
I'd vote for keeping it in a thread for now, so to users the behaviour is simply "background indexing got faster", instead of actually seeing longer blocking on requests?
And after fully migrated to rubydex, we see how fast we can realistically get to and what cleanups will be worth the tradeoff.
There was a problem hiding this comment.
The asynchronous aspect of indexing is the source of various bugs, so I would prefer we invert the decision here. Remove the complexity and revisit if users actually complain it's too slow.
For example, #3639 is difficult to fix properly with async indexing. We have the code lens hack to prevent issues, but we also have a terrible hack in file watched events to keep sleeping and re-enqueuing until indexing is complete.
There was a problem hiding this comment.
I see. If it clears out many bugs at once then let's do it then.
Do you have a fully list of such bugs/cleanups somewhere so we don't forget to do them or parellalize the work?
Motivation
This PR starts indexing and resolving the Rubydex graph on boot and for tests. Note that consuming incremental changes is not yet a part of the API, so this PR doesn't address that yet.
Our strategy, for smaller and easier to review PRs, will be to maintain both indexing mechanisms while we migrate each request one by one. That way, we don't have to review a massive amount of changes and we can rely on CI.
Implementation
The implementation makes sure we're: