-
Notifications
You must be signed in to change notification settings - Fork 186
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
Add sass-embedded to REUSE_AS_BINARY_ON_TRUFFLERUBY #3565
Conversation
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA). To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
Thank you for the PR!
Do you have a log of that or some timings? Locally I wonder what takes so long, https://github.com/sass-contrib/sass-embedded-host-ruby/blob/main/ext/sass/Rakefile looks like a couple downloads which shouldn't be that slow. And a single rake subprocess from what I can see. That seems worth investigating & profiling, cc @andrykonchin but either way this PR seems good to merge. |
Thank you for signing the OCA. |
I tried cloning https://github.com/sass-contrib/sass-embedded-host-ruby and
which looks like an issue of interrupting C ext code to get a backtrace. On a second try it worked though, and took about 20s:
Looks like |
Here is a profile with async-profiler on JVM on 24.0.1: |
I took some time and found this line is where most of the time was spent, and why specs_and_sources, _errors = Gem::SpecFetcher.fetcher.spec_for_dependency(dependency, false) The context here is that, let's say user is installing platform ruby sass-embedded 1.77.0 on aarch64-linux-gnu, the Rakefile will download the rubygems index from rubygems.org and find 1.77.0-aarch64-linux-gnu precompiled gem from the index, then download the gem and extract the pre-built extension from it - this is to avoid rely on external command like |
Ah BTW I have been thinking it might worth adding an option to append custom gems to that list. And that could then be used via |
@ntkme Thank you for looking into it. Do you know why |
I checked this and it's due to This is how
This is how
It turns out |
I was able to bring it down to roughly 4s with sass-contrib/sass-embedded-host-ruby@7f10921 Still, this means installing native gem directly will take 4s, and install platform ruby will take roughly 2 x 4 = 8s. |
Good catch and that explains it clearly, fetching all specs is quite expensive and something that IIRC shouldn't happen anymore with recent RubyGems & Bundler. |
In hindsight I should probably have tried In any case, we'll merge this PR, thank you for the investigation and PR! |
Yes, the two APIs work differently:
As the documentation says: # A Resolver::Specification contains a subset of the information
# contained in a Gem::Specification. Only the information necessary for
# dependency resolution in the resolver is included. |
A new CLI option |
I'm the maintainer of
sass-embedded
gem. This gem has "native extension" that is a Dart VM binary executable, which is completely independent and not linked to libruby. On the platform "ruby", there is aRakefile
based extension that downloads the correct binary for the current platform, and it is extremely slow on TruffleRuby comparing to MRI (likely due toRakefile
extension launching a childrake
process etc). Therefore, it's better to just use the pre-compiled gems that bundles the binary to speed up the installation.