Skip to content
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

chruby.sh's use of eval is painfully slow with JRuby #369

Open
fallwith opened this issue Sep 14, 2016 · 11 comments
Open

chruby.sh's use of eval is painfully slow with JRuby #369

fallwith opened this issue Sep 14, 2016 · 11 comments
Labels

Comments

@fallwith
Copy link

I'm attempting to get our team of Rubyists to standardize on using chruby. So far everyone testing it has liked it a lot; except for the JRuby users. They reported painful slowness while evaluating chruby.

JRuby shines for web apps, but for CLI tasks the startup time is significant. I've traced chruby's slowness to chruby.sh's eval block. That block launches an instance of JRuby to in order to ultimately set GEM_HOME, GEM_PATH, and PATH.

eval "$(RUBYGEMS_GEMDEPS="" "$RUBY_ROOT/bin/ruby" - <<EOF

Has anyone thought of an alternative to a shell eval running a full blown instance of JRuby in order to set those environment variables? We've thought of conditionally skipping over the eval and hoping that bundle exec helps fill in the gaps. And we've thought of working with the engine and version number specified in a project's .ruby-version file. But before we start down either of these paths, I figured I'd ask if anyone has thought of a way around chruby.sh's eval block.

Any ideas?

@jayjlawrence
Copy link

@havenwood per some chatter on JRuby IRC ... I think you're stuck firing up jruby to get the correct GEM_INFO. The time penalty you're experiencing here is from the creation of a JRuby interpreter in java. If you do a command like "jruby -v" you don't incur this expense and you get a really quick response.

There are two possible routes to mitigate this interpreter creation penalty. One is by using "drip". This will keep a java process with jruby instantiated on standby so you're calling a hot jruby interpreter.

The other idea is a bit of slight of hand. What if you change the perception? This is likely completely unacceptable but fun to mention. We do this at this end where we have costly tasks but want to trick the user into thinking its faster - put the work into the background. So run a background process to obtain the Gem.default_dir.inspect value and set the bash env var when this background process is done. You should get your bash prompt back immediately but the user will still need to wait approx 2.5 seconds (on a fairly fast CPU) to actually set up to use jruby with the GEM_HOME env var set.

Check this post about how to run a command to set an env var in the background (yeah it can be done!) http://unix.stackexchange.com/questions/197186/run-a-command-in-background-that-sets-env-var-or-define-function

Ok after that detour - I think what you should really consider doing is 'caching' the value in your ".gem" directory because this appears to be a directory structure that chruby owns and controls. Since 'jruby -v' is quick you can tie this to the jruby version with little cost / overhead.

HTH

@fallwith
Copy link
Author

Very helpful indeed! Thank you, @jayjlawrence

@headius
Copy link

headius commented Sep 28, 2016

There are two possible routes to mitigate this interpreter creation penalty.

I count three :-)

We could also add information to jruby -v. Currently it already includes information about the JDK that's running, but we might find it useful (potentially VERY useful in bug reports) if -v output provided more info, like:

  • GEM home and/or root (I can never remember which is which)
  • JRuby's home
  • Location of the Java executable being used to run JRuby

Concerns about adding to -v output:

  • If added on the same one-liner, it makes the line even longer. It already wraps on most terminals.
  • If added as additional lines (see example from IBM's J9 JVM below) we would differ from Ruby in that -v emits multiple lines. Anyone parsing -v output could have a bug.

Alternatively, we could add a JRuby-specific flag that dumps out a complete set of configuration properties and environment details, similar to Ant's "ant environment" command.

Any thoughts, @enebo?

@headius
Copy link

headius commented Sep 28, 2016

Another thought related to modifying JRuby: perhaps gem root/home should always come from a configuration property, but that property would default to the env var. There's currently no way to configure a single JRuby instance to have a different gem home other than twiddling the pseudo-ENV var inside that JRuby instance before booting RG.

@enebo
Copy link

enebo commented Sep 28, 2016

@headius I think we can add an option since it is the least risky and it is sort of like having a command for dumping our env.

Property files and envs potentially end up generating weird reports for us when people forget they have them set.

@headius
Copy link

headius commented Sep 28, 2016

I vote for jruby --environment.

@headius
Copy link

headius commented Sep 28, 2016

This might also be a good feature for MRI.

@enebo
Copy link

enebo commented Sep 28, 2016

@headius one other question is should this work with all the options or is it an either or? I was thinking for reporting purposes a jruby --environment my_buggy_script.rb might be nice (sorry for spamming chruby issue :) ).

fallwith added a commit to fallwith/chruby that referenced this issue Sep 29, 2016
`chruby_use()`'s `PATH`, `GEM_HOME`, and `GEM_PATH` environment variable
setting functionality has been relocated to a new `chruby_set_paths()`
function. This new function will cache to disk the results of the
`eval` call which invokes Ruby to determine the values to use. The
cached results will be looked for and sourced if found in lieu of
invoking the `eval()` call. This logic is one potential way to address
[issue 369](postmodern#369).
@skull-squadron
Copy link
Contributor

skull-squadron commented Nov 11, 2016

@fallwith This is a great patch, I just independently implemented a very similar solution. Currently, chruby jruby wastes a lot of many people's time unnecessarily. There is no sense in calling Ruby for every invocation and regenerating the wheel when it's always the same shape. Please make a PR, because it's useful now. Other future solutions can refactor things later.

@fallwith
Copy link
Author

Thanks, @steakknife. Unfortunately the commit I added to my fork ended up introducing some issues for my MRI usage in the name of helping with the JRuby side of things. I haven't had the time to dig into why that was. I also haven't implemented any tests yet to describe / cover my changes.

@skull-squadron
Copy link
Contributor

I have a working MRI patch, I can PR that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants