Skip to content

Conversation

@amatsuda
Copy link
Member

This patch drastically reduces time taken to load the gem in all non-Windows platforms.

Here's a quick benchmark result of requiring the gem before and after this patch on my Mac.

ruby -e "now = Time.now; gem 'bigdecimal', '1.4.4'; require 'bigdecimal'; p '1.4.4' => Time.now - now"
ruby -e "now = Time.now; gem 'bigdecimal', '2.0.0.dev'; require 'bigdecimal'; p '2.0.0.dev' => Time.now - now"
{"1.4.4"=>0.172034}
{"2.0.0.dev"=>0.003902}

This patch was originally written by @nobu for io-console gem, and I'm just copying the solution to this repo, so please read other repos' PRs for more details.
ruby/psych#406
ruby/io-console#4

@kou
Copy link
Member

kou commented Jul 15, 2019

I think that we can just stop fat gem support for the performance improvement.
RubyInstaller provides Ruby that can build an extension with gem install bigdecimal. Because RubyInstaller provides Ruby with MinGW as Devkit.

@nobu
Copy link
Member

nobu commented Jul 16, 2019

Isn't a binary gem also for mswin?
Although mswin binary package may not exist now.

@kou
Copy link
Member

kou commented Jul 16, 2019

Isn't a binary gem also for mswin?

We may be able to use a binary gem for *-mingw32 with *-mswin* because it for *-mingw32 uses non versioned msvcrt.dll. But it's not recommended. *-mswin* users should build their extension library by Visual C++.

And existing binary gems for *-mingw32 aren't installed automatically with *-mswin* Ruby. Because they are different architecture.

@amatsuda
Copy link
Member Author

I think that we can just stop fat gem support for the performance improvement.

I'm totally fine with either solution.

RubyInstaller provides Ruby that can build an extension with gem install bigdecimal. Because RubyInstaller provides Ruby with MinGW as Devkit.

This may work great so far as everything is working. But it may cause some compilation troubles especially for those who are not very much familiar with compiling things, and so the development team may have to spend some time on shooting such troubles.

@kou
Copy link
Member

kou commented Jul 16, 2019

But it may cause some compilation troubles especially for those who are not very much familiar with compiling things, and so the development team may have to spend some time on shooting such troubles.

Providing fat gem spends more time than resolving build error problems in my experience (I stopped fat gem support for Ruby-GNOME2 that has Windows users).

Normally, build error problems aren't occurred while AppVeyor build is green.

Normally, releasing fat gems is forgotten. For example, bigdecimal started fat gem support since 2017 #75 but no fat gems are released: https://rubygems.org/gems/bigdecimal/versions
The latest io-console (0.4.8) is the same situation: https://rubygems.org/gems/io-console/versions

I'm not surprised at this because releasing fat gems is a bother. We need to run more commands (rake build:mingw and gem push pkg/*-mingw32.gem) than rake release and prepare Docker. And we need to update supported Ruby version list when new Ruby is released: https://github.com/ruby/bigdecimal/blob/master/Rakefile#L21

I think that removing fat gem support will make both users (improving require performance) and developers (less maintenance cost) happy.

@amatsuda
Copy link
Member Author

The latest io-console (0.4.8) is the same situation: https://rubygems.org/gems/io-console/versions

Ugh. So sorry about this. I asked nobu to release the gem and I was physically pairing with him, but yeah, we skipped that process...

Anyway, I'm now +1 for omitting the fat gem release. Let's see how it goes, and we can switch back to the fat gems anytime if we're seeing unexpected problems.
I suppose it's actually a very good timing to change the distribution process since the next release is a major version bump, right?

@kou
Copy link
Member

kou commented Jul 17, 2019

We don't need to care about major version bump. Because we have never released fat gems for bigdecimal.

@mrkn What do you think about removing fat gem support? I think that you don't want to care about Windows. So removing fat gem support seems reasonable to you.

@mrkn
Copy link
Member

mrkn commented Jul 17, 2019

I want to drop fat gem support.

@kou
Copy link
Member

kou commented Jul 17, 2019

OK. Let's drop fat gem support.

@amatsuda Do you want to work on it in this pull request or create another pull request?

amatsuda added a commit to amatsuda/bigdecimal that referenced this pull request Jul 17, 2019
"fat gem" release has never actually been done in the past, and we have no plan to do it in the near future.
See GH#148 for the detailed discussion. ruby#148
@amatsuda
Copy link
Member Author

Just made another PR #149 for dropping the whole "fat gem" thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants