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

Removed unf dependency for ruby > 2.2 #11

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tayler1
Copy link

@tayler1 tayler1 commented Feb 4, 2017

ref #3

@tayler1
Copy link
Author

tayler1 commented Feb 5, 2017

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
Copy link

@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
Copy link
Owner

knu commented Mar 29, 2017

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
Copy link
Owner

knu commented Mar 29, 2017

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

@tayler1
Copy link
Author

tayler1 commented Mar 29, 2017

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
Copy link
Owner

knu commented Mar 29, 2017

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
Copy link
Author

tayler1 commented Mar 29, 2017

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

@cwjohnston
Copy link

@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
Copy link
Author

tayler1 commented Mar 29, 2017

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

@sandstrom
Copy link

sandstrom commented Nov 21, 2018

friendly ping @knu 😄

@Mijyuoon
Copy link

Any updates on this?

@ydakuka
Copy link

ydakuka commented Sep 17, 2023

@knu ping again, sorry to disturb :)

@tisba
Copy link
Contributor

tisba commented Dec 26, 2023

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

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.

7 participants