knu / ruby-domain_name

Domain Name manipulation library for Ruby
Other
106 stars 23 forks source link

Removed unf dependency for ruby > 2.2 #11

Open tayler1 opened 7 years ago

tayler1 commented 7 years ago

ref #3

tayler1 commented 7 years ago

ruby 2.4 / derailed bundle:mem

current master

TOP: 3.9102 MiB
  domain_name: 2.5078 MiB
    domain_name/etld_data: 2.3203 MiB
  rake: 1.2734 MiB
    rake/rake_module: 0.9727 MiB
      rake/application: 0.9531 MiB (Also required by: rake)
        rake/file_list: 0.5234 MiB (Also required by: rake)

without unf

TOP: 6.8047 MiB
  domain_name: 5.9375 MiB
    unicode_normalize/normalize: 3.4414 MiB
      unicode_normalize/tables.rb: 3.3789 MiB
    domain_name/etld_data: 2.2813 MiB
  rake: 0.8281 MiB
    rake/rake_module: 0.7422 MiB
      rake/application: 0.7344 MiB (Also required by: rake)
        rake/thread_pool: 0.4297 MiB

ruby 2.4 / test.rb

current master

Total allocated: 87820 bytes (676 objects)
Total retained:  4289 bytes (27 objects)

allocated memory by gem
-----------------------------------
     59509  unf-0.1.4
     17038  other
      4800  ruby-domain_name-16bf27a2b3e7
      4401  unf_ext-0.0.7.2
      2072  ruby-2.4.0/lib

without unf

Total allocated: 5632 bytes (93 objects)
Total retained:  368 bytes (2 objects)

allocated memory by gem
-----------------------------------
      4880  ruby-domain_name/lib
       672  other
        80  ruby-2.4.0/lib

ruby 2.4 / test-100k.rb

current master

Total allocated: 185688228 bytes (3000679 objects)
Total retained:  4657 bytes (29 objects)

allocated memory by gem
-----------------------------------
 168804800  ruby-domain_name-16bf27a2b3e7
  12817446  other
   4059509  unf-0.1.4
      4401  unf_ext-0.0.7.2
      2072  ruby-2.4.0/lib

without unf

Total allocated: 181605344 bytes (2900087 objects)
Total retained:  368 bytes (2 objects)

allocated memory by gem
-----------------------------------
 164804760  ruby-domain_name/lib
  12800544  other
   4000040  ruby-2.4.0/lib

ruby 2.4 / bench.rb

current master

                       user     system      total        real
new int            0.420000   0.000000   0.420000 (  0.426173)
new etld           0.060000   0.000000   0.060000 (  0.061709)
ascii              0.060000   0.000000   0.060000 (  0.058985)
decode             0.910000   0.000000   0.910000 (  0.907784)

without unf

                       user     system      total        real
new int            0.460000   0.000000   0.460000 (  0.462737)
new etld           0.090000   0.000000   0.090000 (  0.093066)
ascii              0.090000   0.000000   0.090000 (  0.085603)
decode             0.930000   0.000000   0.930000 (  0.930268)

ruby 2.4 / test.rb process size

current master Activity: 20.4 MB without unf Activity: 23.5 MB

cwjohnston commented 7 years ago

@knu I'm curious to know if this change could land in a release any time soon?

I have found that a large number of projects under the Sensu Plugins organization require native extensions to install, and in many cases this is the result of a dependency on rest-client, which pulls in this library and subsequently unf.

I believe making the dependency on unf conditional would have a major positive impact on such projects, making them usable by a wider audience who can't or won't install a compiler toolchain on their systems.

knu commented 7 years ago

You are free and good to use this PR branch in your Gemfile (gem "domain_name", github: "tayler1/ruby-domain_name", branch: "unicode-normalize"), but when it comes to merging this, there's a problem. If I merged this and built a gem using Ruby >=2.2, the built gem wouldn't have a dependency on unf, and it couldn't be used for Ruby < 2.2.

knu commented 7 years ago

So I think I'll just completely drop unf in domain_name 1.0.0 and release it for newer rubies exclusively.

tayler1 commented 7 years ago

I saved back compatibility with ruby prior 2.2 Please review commit changes. But yes, I agree that this changes probably targets to new "major" version.

knu commented 7 years ago

Yeah, perhaps I could merge this, bump the major version, and announce that users of old rubies should pull this gem from the repository instead of using the released gems. Let me think about it. Thanks for your contribution!

tayler1 commented 7 years ago

Actually it shouldn't be any special notes. All tests are passed for all rubies.

cwjohnston commented 7 years ago

@tayler1 I think the concern is that the conditional logic in the gemspec isn't reflected in the built gem artifact. I believe the artifact will reflect dependencies of whichever Ruby version is used to build it.

tayler1 commented 7 years ago

It reflects. There are no artefacts built for ruby > 2.2

sandstrom commented 5 years ago

friendly ping @knu 😄

Mijyuoon commented 3 years ago

Any updates on this?

ydakuka commented 1 year ago

@knu ping again, sorry to disturb :)

tisba commented 10 months ago

I think this PR can be closed, unf has been removed.