Skip to content

Start indexing and resolving Rubydex graph #3992

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

Open
vinistock wants to merge 1 commit into
base: rubydex_adoption_feature_branch
Choose a base branch
from
+24 −4
Open

Start indexing and resolving Rubydex graph #3992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lib/ruby_lsp/global_state.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ class GlobalState
#: RubyIndexer::Index
attr_reader :index

#: Rubydex::Graph
attr_reader :graph

#: Encoding
attr_reader :encoding

Expand Down Expand Up @@ -58,6 +61,7 @@ def initialize
@test_library = "minitest" #: String
@has_type_checker = true #: bool
@index = RubyIndexer::Index.new #: RubyIndexer::Index
@graph = Rubydex::Graph.new #: Rubydex::Graph
@supported_formatters = {} #: Hash[String, Requests::Support::Formatter]
@type_inferrer = TypeInferrer.new(@index) #: TypeInferrer
@addon_settings = {} #: Hash[String, untyped]
Expand Down Expand Up @@ -117,6 +121,7 @@ def apply_options(options)
all_dependencies = gather_direct_and_indirect_dependencies
workspace_uri = options.dig(:workspaceFolders, 0, :uri)
@workspace_uri = URI(workspace_uri) if workspace_uri
@graph.workspace_path = workspace_path

specified_formatter = options.dig(:initializationOptions, :formatter)
rubocop_has_addon = defined?(::RuboCop::Version::STRING) &&
Expand Down Expand Up @@ -189,12 +194,16 @@ def apply_options(options)

encodings = options.dig(:capabilities, :general, :positionEncodings)
@encoding = if !encodings || encodings.empty?
@graph.encoding = "utf16"
Encoding::UTF_16LE
elsif encodings.include?(Constant::PositionEncodingKind::UTF8)
@graph.encoding = "utf8"
Encoding::UTF_8
elsif encodings.include?(Constant::PositionEncodingKind::UTF16)
@graph.encoding = "utf16"
Encoding::UTF_16LE
else
@graph.encoding = "utf32"
Encoding::UTF_32
end
@index.configuration.encoding = @encoding
Expand Down
16 changes: 12 additions & 4 deletions lib/ruby_lsp/server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1228,12 +1228,18 @@ def shutdown

#: -> void
def perform_initial_indexing
progress("indexing-progress", message: "Indexing workspace...")
@global_state.graph.index_workspace

progress("indexing-progress", message: "Resolving graph...")
@global_state.graph.resolve
Comment on lines +1231 to +1235
Copy link
Contributor

@alexcrocha alexcrocha Mar 18, 2026

Choose a reason for hiding this comment

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

Should this be inside its own Thread.new block as well, like the existing indexing? Or is Rubydex handling it in some other way

Copy link
Member Author

@vinistock vinistock Mar 18, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Member

@st0012 st0012 Mar 18, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@vinistock vinistock Mar 18, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Member

@st0012 st0012 Mar 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

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?


# The begin progress invocation happens during `initialize`, so that the notification is sent before we are
# stuck indexing files
Thread.new do
begin
@global_state.index.index_all do |percentage|
progress("indexing-progress", percentage)
progress("indexing-progress", percentage: percentage)
true
rescue ClosedQueueError
# Since we run indexing on a separate thread, it's possible to kill the server before indexing is complete.
Expand Down Expand Up @@ -1287,11 +1293,13 @@ def begin_progress(id, title, percentage: 0)
send_message(Notification.progress_begin(id, title, percentage: percentage, message: "#{percentage}% completed"))
end

#: (String id, Integer percentage) -> void
def progress(id, percentage)
#: (String, ?message: String?, ?percentage: Integer?) -> void
def progress(id, message: nil, percentage: nil)
return unless @global_state.client_capabilities.supports_progress

send_message(Notification.progress_report(id, percentage: percentage, message: "#{percentage}% completed"))
message ||= "#{percentage}% completed" if percentage

send_message(Notification.progress_report(id, percentage: percentage, message: message))
end

#: (String id) -> void
Expand Down
3 changes: 3 additions & 0 deletions lib/ruby_lsp/test_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ def with_server(source = nil, uri = Kernel.URI("file:///fake.rb"), stub_no_typec
})

server.global_state.index.index_single(uri, source)
graph = server.global_state.graph
graph.index_source(uri.to_s, source, "ruby")
graph.resolve
end

server.load_addons(include_project_addons: false) if load_addons
Expand Down
Loading