ruby / stringio

Pseudo `IO` class from/to `String`.
BSD 2-Clause "Simplified" License
33 stars 26 forks source link

Add version.rb and load it from stringio.rb #58

Closed headius closed 1 year ago

headius commented 1 year ago

Fixes #57

This probably will need some discussion. There are no shared .rb files between the C extension gem and the JRuby extension gem. There's no easy way to inject a value into the javac compilation, so it seemed like the best answer is to move the version to a common stringio/version.rb file and start shipping the stringio.rb that loads correctly for JRuby or non-JRuby.

I am open to suggestions on a better way to solve this without having the version string in more than one place.

headius commented 1 year ago

I'm not sure what caused the slow down in those gems you linked, but these days adding the stub stringio.rb and version.rb files really should not make much of a difference.

Here's a test of stock 3.1 stringio versus my modified stringio:

[] stringio $ (chruby 3.1 ; time ruby -Ilib:lib/3.1.4/arm64-darwin22 -e 'require "stringio"')
ruby -Ilib:lib/3.1.4/arm64-darwin22 -e 'require "stringio"'  0.03s user 0.01s system 94% cpu 0.038 total
[] stringio $ (chruby 3.1 ; time ruby -e 'require "stringio"')                         
ruby -e 'require "stringio"'  0.03s user 0.01s system 93% cpu 0.038 total

Activating stringio as a gem shows no impact either:

[] stringio $ (chruby 3.1 ; time ruby -e 'gem "stringio", "3.0.1"; require "stringio"')
ruby -e 'gem "stringio", "3.0.1"; require "stringio"'  0.03s user 0.01s system 92% cpu 0.038 total
[] stringio $ (chruby 3.1 ; time ruby -e 'gem "stringio", "3.0.6"; require "stringio"')
ruby -e 'gem "stringio", "3.0.6"; require "stringio"'  0.03s user 0.01s system 94% cpu 0.038 total

Can you show me how the extra .rb files slow down loading stringio?

headius commented 1 year ago

We can update version information in them automatically

As long as this is acceptable and happens consistently, it would be fine with me. It will be a step anyone doing a release has to remember: bump the version, then run rake to update sources. Do you have an example gem that does this today?

headius commented 1 year ago

bump the version, then run rake to update sources

... and commit the results

kou commented 1 year ago

You need to have many installed gems. In https://github.com/ruby/io-console/pull/4 case, there are 656 gems:

just requiring 'io/console' adds extra 0.1+ sec on my MacBook Pro having 656 gems installed.


It will be a step anyone doing a release has to remember: bump the version, then run rake to update sources.

We always bump the version after a new release. This is a normal release step. And it's already done by rake bump or its family such as rake bump:minor. Using rake isn't a new step.

Do you have an example gem that does this today?

Some repositories in ruby/ use this style. See also: https://github.com/search?q=org%3Aruby%20version.rake&type=code

... and commit the results

rake bump and its family do it automatically.

headius commented 1 year ago

You need to have many installed gems. In https://github.com/ruby/io-console/pull/4 case, there are 656 gems

I am not convinced. I have 150 gems installed on this 3.1 and there's basically no difference between my way and the old way:

[] stringio $ time ruby -e 'gem "stringio", "3.0.1"; require "stringio"'
ruby -e 'gem "stringio", "3.0.1"; require "stringio"'  0.05s user 0.02s system 88% cpu 0.076 total
[] stringio $ time ruby -e 'gem "stringio", "3.0.1"; require "stringio"'
ruby -e 'gem "stringio", "3.0.1"; require "stringio"'  0.05s user 0.02s system 94% cpu 0.071 total
[] stringio $ time ruby -e 'gem "stringio", "3.0.1"; require "stringio"'
ruby -e 'gem "stringio", "3.0.1"; require "stringio"'  0.05s user 0.02s system 94% cpu 0.070 total
[] stringio $ time ruby -e 'gem "stringio", "3.0.1"; require "stringio"'
ruby -e 'gem "stringio", "3.0.1"; require "stringio"'  0.05s user 0.02s system 94% cpu 0.071 total
[] stringio $ time ruby -e 'gem "stringio", "3.0.6"; require "stringio"'
ruby -e 'gem "stringio", "3.0.6"; require "stringio"'  0.05s user 0.02s system 94% cpu 0.071 total
[] stringio $ time ruby -e 'gem "stringio", "3.0.6"; require "stringio"'
ruby -e 'gem "stringio", "3.0.6"; require "stringio"'  0.05s user 0.02s system 93% cpu 0.073 total
[] stringio $ time ruby -e 'gem "stringio", "3.0.6"; require "stringio"'
ruby -e 'gem "stringio", "3.0.6"; require "stringio"'  0.05s user 0.02s system 94% cpu 0.071 total
[] stringio $ time ruby -e 'gem "stringio", "3.0.6"; require "stringio"'
ruby -e 'gem "stringio", "3.0.6"; require "stringio"'  0.05s user 0.02s system 94% cpu 0.071 total

As I mentioned, I believe improvements in RubyGems and Ruby itself have greatly reduced the cost of loading files from gems, and we're talking about a couple extra files out of thousands required by a typical app. I also believe those stub files were doing a LOT more than this, trying several platforms and failing with LoadError until it finds the right one, which is extremely expensive on any impl.

Plus I cannot show it slowing down at all on 3.1.

We always bump the version after a new release.

If that is a standard thing for other core gems then I'm fine with it.

At the moment we have been forced to exclude all StringIO specs because of the version checks. I just want this fixed as soon as possible.

headius commented 1 year ago

there's basically no difference

Keep in mind the issues you linked to had 4-5x as many gems with a performance hit of 0.1s or more; my example shows that any slowdown with 150 gems (if I could even show it) would be in the 1-2 millisecond range (8-10ms if it scales with number of gems).

I don't understand the exact cause in those issues, but it doesn't seem to be a problem here.

kou commented 1 year ago

Ah, sorry. We need one more condition for this problem:

begin
  require "always-failed"
rescue LoadError
  require "stringio.so"
end

The require "always-failed" is slow when many gems installed.

We don't have the condition in this case. Sorry.

kou commented 1 year ago

Anyway, I still think that the non version.rb approach is simpler.

kou commented 1 year ago

Another approach: #59

headius commented 1 year ago

@kou Your PR looks good to me!

kou commented 1 year ago

Close this in favor of #59.