rbenv-gemset
rbenv-gemset copied to clipboard
jf
improve load times w/ fixed gemset iteration
This PR merges @nrser's work to improve load times, as discussed at https://github.com/jf/rbenv-gemset/issues/84#issue-294145396, with some trivial fixes to an iteration error from my own fork.
@AprilArcus @nrser can we leave out https://github.com/jf/rbenv-gemset/pull/87/commits/5a1b48009732a6cae6facbe6263fd438c48279bd ? Leave the versioning / scheme to me. I'll bump the version when this is done (and I think 3 numbers is enough already).
re https://github.com/jf/rbenv-gemset/pull/87/commits/5a382efb397826d20c45168669ab1f0fbd88a454:
New version adds ~15ms to `ruby -e 'puts "yo"' time :)
sorry, "adds"?
Thanks for adding stuff like rbenv_gemset_debug, btw. For trying to understand what the code does - and I think even for the purpose of code review - it is useful. I will eventually have to remove this in the end before it becomes production.
re the use of rbenv_gemset_lib_loaded: I agree with this approach. I took the same approach for something I wanted to apply to rbenv (you can see https://github.com/rbenv/rbenv/pull/507)
@nrser can you explain your comment in https://github.com/jf/rbenv-gemset/pull/87/commits/43ca1c1db51decfba71a446e268c2607e7faf26a ? What are "lock executables"? and what is "QB"?
@AprilArcus @nrser can we leave out 5a1b480 ? Leave the versioning / scheme to me. I'll bump the version when this is done (and I think 3 numbers is enough already).
Yeah for sure, I just did that so I had a different version number for my fork and could tell the difference from a quick rbenv gemset version (think I had both the master and my fork installed alongside when I was doing the initial work).
The commits you're looking at are the ones I made as I was writing it. I thought I would come back and rebase it up nice afterwards, but once I realized I wasn't going to have time for that any time soon I just shared my branch, figuring that given the magnitude of performance pain & gain people might want to use it regardless of the mess and lack of test.
re 5a382ef:
New version adds ~15ms to `ruby -e 'puts "yo"' time :)
sorry, "adds"?
Oh, that meant that after the changes using Ruby through rbenv/gemsets only added an extra ~15ms spin-up time (compared to just running the system Ruby without rbenv).
Before the changes, the rbenv/gemsets combo was adding around ~700ms over the system installation (testing with simple ruby -e ... commands).
Eventually I think I ended up spending around 40ms for spin-up, having addressed more of the paths and features than I had at the point of 5a382ef
@nrser can you explain your comment in 43ca1c1 ? What are "lock executables"? and what is "QB"?
Like I said previously, I was just committing as I was working along, thinking it would get rebased for publication. That stuff is just notes to self regarding what was broken on my machine.
The "locks" are this:
https://github.com/nrser/rbenv-lock
lil' rbenv plugin I wrote to "lock" certain Gem executables to specific versions of Ruby.
Like, if you use lunchy or travis or whatever CLI apps and you install them as gems, then you either need to install them in every Ruby version you have in rbenv - in which case they might not even support or work across all of them, and even if they do it makes for a horrible setup and maintenance situation - or they will break when you're using a Ruby or gemset that doesn't have them, either through being in a project directory with a .ruby-version, rbenv shell ... set, etc.
I use Ruby for a lot of personal tools and use a few tools distributed through gems and this annoyed the shit out of me, so I wrote a little plugin to try to fix it.
What the comment's talking about is that QB - a tool I wrote to run Ansible roles against localhost from the CLI - is written in Ruby, and uses rbenv-lock to lock on the the Ruby version it's installed for.
The tricky part is that QB may run tasks that invoke executables that are also written in Ruby, and may even use rbenv-lock, so you can get ENV vars set by rbenv/-gemset/-lock trickling down through descendant processes and screwing up Ruby processes that need a different environment to work.
So exporting RBENV_GEMSET_EXEC_ALREADY was causing problems running child Ruby processes, which totally makes sense.
Thanks for adding stuff like
rbenv_gemset_debug, btw. For trying to understand what the code does - and I think even for the purpose of code review - it is useful. I will eventually have to remove this in the end before it becomes production.
rbenv_gemset_debug() only writes output if the $RBENV_GEMSET_DEBUG ENV var is set, so you shouldn't see any debug output unless you turn that guy on, and it's function call, so it's cheap. Do you want to remove it for cosmetic reasons (just don't like seeing/having the calls in the source)?
I don't really do much Bash (and I'm sure it shows), so I'm not sure how projects usually handle logging... but I know at least some tools have CLI options or ENV vars you can use to turn on debug logging in production releases, and I've had it be useful more than once when diagnosing issues.
I see that rbenv just uses their $RBENV_DEBUG flag to set -x, but, man, I've hated wading through the set -x output for anything more than a few-dozen lines of script. Which is why I wrote those logging functions. If that's how it's done though I can get with it.
Thanks for following up, @AprilArcus . This year? I should think so. I need to find time to review this again.
I thought I stated somewhere the version bump implementation should be up to me... but I don't see it here. If I approve this, I will want to do it without the forced version bump.