-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Feature flags #1715
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
Feature flags #1715
Conversation
bors r+ |
1715: Feature flags r=matklad a=matklad Co-authored-by: Aleksey Kladov <[email protected]>
Build failed |
bors r+ |
1715: Feature flags r=matklad a=matklad Co-authored-by: Aleksey Kladov <[email protected]>
Build failed |
bors r+ |
1715: Feature flags r=matklad a=matklad Co-authored-by: Aleksey Kladov <[email protected]>
Build failed |
bors r+ |
1715: Feature flags r=matklad a=matklad Co-authored-by: Aleksey Kladov <[email protected]>
Build succeeded |
Should we be using |
Quite likely! I haven't yet fully understood how configuration is supposed to work with LSP, so I just pile additional stuff onto the thing we have |
@@ -587,17 +602,20 @@ fn update_file_notifications_on_threadpool( | |||
subscriptions: Vec<FileId>, | |||
) { | |||
log::trace!("updating notifications for {:?}", subscriptions); | |||
let publish_diagnostics = world.feature_flags().get("lsp.diagnostics"); | |||
pool.execute(move || { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't you do a pre-spawn condition check whether this will actually do something? This is in the main thread and loop, and maybe we don't even do something, yet we spawn a useless thread (iterating over files for nothing)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn’t spawn a thread, just sends a closure over a channel, it”s only a constant amount of work, comparable to the work we need to get to this point in the first place.
No description provided.