Skip to content
This repository was archived by the owner on May 30, 2024. It is now read-only.

Delete GC tuning variables from the environment#386

Merged
vinistock merged 1 commit intomainfrom
vs/delete_gc_tuning_variables
Feb 13, 2023
Merged

Delete GC tuning variables from the environment#386
vinistock merged 1 commit intomainfrom
vs/delete_gc_tuning_variables

Conversation

@vinistock
Copy link
Copy Markdown
Member

Sometimes developers may use Ruby's GC tuning environment variables to calibrate their application's performance. However, this is often not adequate for the Ruby LSP and usually results in spending an insane amount of time doing garbage collection.

There might be an opportunity to tune these specifically for the Ruby LSP to output better performance, but for the time being we should at least remove those and use defaults, which greatly improves performance where GC has been tuned.

@vinistock vinistock self-assigned this Feb 10, 2023
@vinistock vinistock requested a review from a team as a code owner February 10, 2023 21:08
Copy link
Copy Markdown
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

I think we may want to delete RUBYOPT too as it doesn't make sense for the LSP server to take it anyway.
For example, ruby/debug users could inject RUBYOPT into their shell config following the official recommendation. This means ruby-lsp would require that prelude.rb as well. Even though in this case loading that file shouldn't break ruby-lsp, it still highlights the risk of taking RUBYOPT.

@vinistock
Copy link
Copy Markdown
Member Author

I agree, but that might require a more complex check. If I'm not mistaken bundler itself adds some requires using RUBYOPT and getting rid of those could be problematic. I created an issue to investigate it separately Shopify/ruby-lsp#1532.

@vinistock vinistock merged commit 54958a2 into main Feb 13, 2023
@vinistock vinistock deleted the vs/delete_gc_tuning_variables branch February 13, 2023 14:15
Comment thread src/client.ts
"RUBY_GC_MALLOC_LIMIT",
"RUBY_GC_HEAP_GROWTH_MAX_SLOTS",
"RUBY_GC_OLDMALLOC_LIMIT",
"RUBY_GC_HEAP_INIT_SLOTS",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Other than looking at the Ruby source, is there somewhere that documents all the available variables? Google isn't returning anything useful.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The most sensible thing might be to remove all the env var that start with RUBY_GC_*, since we introduced new ones in Ruby 3.3.0-dev already, it would be more future proof.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Comment thread src/client.ts
"RUBY_GC_HEAP_OLDOBJECT_LIMIT_FACTOR",
"RUBY_GC_HEAP_FREE_SLOTS",
"RUBY_GC_HEAP_SLOTS_GROWTH_FACTOR",
"RUBY_GLOBAL_METHOD_CACHE_SIZE",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

So this env var has no longer any effect since Ruby 3.0, you can ignore it. I also recently removed it from core.

jayanth-kumar-morem pushed a commit to jayanth-kumar-morem/vscode-ruby-lsp that referenced this pull request Apr 26, 2023
…p-minitest-0.25.0

Bump rubocop-minitest from 0.24.0 to 0.25.0
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants