Remove unused files upon gem install by lloeki · Pull Request #6 · DataDog/datadog-ruby_core_source

lloeki · 2024-10-30T18:42:25Z

Given that:

RubyCoreSource.create_makefile_with_core is the main entry point and computes things from the current runtime information
RubyCoreSource.deduce_packaged_source_dir receives that information

Let's directly make use of that to reduce the gem size post-install ~~by adding a scri...~~

Wait, not post-install. That's too manual. Let's do it zero-step, at install time.

extconf.rb is just a ruby script.
spec.extensions = ['path/to/some/script.rb'] is but a callback mechanism to call the above script.
ext/foo/extconf.rb is a mere convention.

Buuuuut it has to have a Makefile (also convention because hey, it's named extension after all)... so let's add a dummy MakeMakefile.create_makefile with an empty, unused stub C file.

Results:

$ bundle exec rake build
datadog-ruby_core_source 3.3.6 built to pkg/datadog-ruby_core_source-3.3.6.gem.

$ env GEM_HOME=tmp/gem_home gem install pkg/datadog-ruby_core_source-3.3.6.gem
Building native extensions. This could take a while...
Successfully installed datadog-ruby_core_source-3.3.6
Parsing documentation for datadog-ruby_core_source-3.3.6
Installing ri documentation for datadog-ruby_core_source-3.3.6
Done installing documentation for datadog-ruby_core_source after 0 seconds
1 gem installed

$ ls tmp/gem_home/gems/datadog-ruby_core_source-3.3.6/lib/datadog/ruby_core_source
ruby-3.3.5-p100

$ du -h -d 0 tmp/gem_home/gems/datadog-ruby_core_source-3.3.6/lib/datadog/ruby_core_source
1.6M	tmp/gem_home/gems/datadog-ruby_core_source-3.3.6/lib/datadog/ruby_core_source

Yup. 1.6 MB, down from master's 8-ish MB.

Note: I tried to use MakeMakefile.dummy_makefile but it failed so I resorted to the above as a quick hack. Should not be that hard to create a Makefile that does nothing instead?

datadog-datadog-prod-us1 · 2024-10-30T18:43:00Z

+
+dir = Datadog::RubyCoreSource.deduce_packaged_source_dir("ruby-#{RUBY_VERSION}-p#{RUBY_PATCHLEVEL}")
+
+unused = Dir.glob(File.dirname(dir) + '/*') - [dir]


⚪ Code Quality Violation

Consider using string interpolation or formatting instead of concatenation. (...read more)

The rule "Avoid string concatenation" is an important coding practice in Ruby for ensuring efficient and clean code. String concatenation in Ruby using the '+' operator creates a new string object, which can lead to excessive memory usage and slower performance when dealing with large strings or performing the operation multiple times.

Instead, Ruby provides alternatives that are more efficient. The string interpolation syntax #{} allows you to insert variables directly into strings without creating new string objects. This is not only more memory efficient, but also provides cleaner and more readable code.

Another alternative is the format method, which allows you to create a formatted string with placeholders for variables. This method is particularly useful when dealing with more complex strings, as it provides a clear and concise way to format your strings.

By following this rule, you can write more efficient and cleaner Ruby code, leading to better performance and readability.

datadog-datadog-prod-us1 · 2024-10-30T18:43:00Z

+require 'mkmf'
+require 'fileutils'
+
+$LOAD_PATH.push(__dir__ + '/../../lib')


⚪ Code Quality Violation

Consider using string interpolation or formatting instead of concatenation. (...read more)

The rule "Avoid string concatenation" is an important coding practice in Ruby for ensuring efficient and clean code. String concatenation in Ruby using the '+' operator creates a new string object, which can lead to excessive memory usage and slower performance when dealing with large strings or performing the operation multiple times.

Instead, Ruby provides alternatives that are more efficient. The string interpolation syntax #{} allows you to insert variables directly into strings without creating new string objects. This is not only more memory efficient, but also provides cleaner and more readable code.

Another alternative is the format method, which allows you to create a formatted string with placeholders for variables. This method is particularly useful when dealing with more complex strings, as it provides a clear and concise way to format your strings.

By following this rule, you can write more efficient and cleaner Ruby code, leading to better performance and readability.

lloeki · 2024-10-30T18:43:59Z

+
+require 'datadog/ruby_core_source'
+
+dir = Datadog::RubyCoreSource.deduce_packaged_source_dir("ruby-#{RUBY_VERSION}-p#{RUBY_PATCHLEVEL}")


PROBABLY we should move this whole bit of logic here in as well:

datadog-ruby_core_source/lib/datadog/ruby_core_source.rb

Lines 20 to 24 in 4b3cc1c

ruby_dir = if RUBY_PATCHLEVEL < 0

REVISION_MAP[RUBY_REVISION] or "ruby-#{RUBY_VERSION}"

else

"ruby-#{RUBY_VERSION}-p#{RUBY_PATCHLEVEL}"

end

Also, maybe for 3.4 preview we might need to add stuff in here?

datadog-ruby_core_source/lib/datadog/ruby_core_source.rb

Lines 6 to 11 in 4b3cc1c

REVISION_MAP = {

# Add pre-release version here since they do not have patchlevel to refer to.

# Revision can be found at `revision.h` of ruby sources.

# Format of this hash:

# <RUBY_REVISION> => '<sources directory name>', e.g. `61243 => 'ruby-2.5.0-rc1'`

}

marcotc · 2024-10-30T18:55:48Z

I think we need documentation somewhere on why this empty file exists.
Can we add a comment in it? Or maybe in the ext/shave/extconf.rb above?

Sure, I just threw this 5min hackjob out to see if the idea held water.

lloeki · 2024-11-06T12:39:14Z

@ivoanjo found the idea dangerous in face of weird stuff users may be creatively doing. Here's the scenario laid out:

this gem is installed with 3.3.2 runtime
3.3.0 headers gets installed, others removed (notably 3.3.5)
ruby is upgraded to 3.3.5
3.3.0 (via volume, cache, or other filesystem) is reused
it should be using 3.3.5 headers, but they are absent

As is, this could be fixed by gem pristine; the user experience would remain poor though.

Note that:

It always does not apply to cross-minor versions, unless users are reusing an unversioned GEM_HOME (e.g /usr/local/bundle), but then in that case other gems will break just as well, notably those with C extensions or those that have a ruby version dependency constraint that would not be reevaluated. IOW in that case the user would be running afoul of the Ruby and rubygems contract itself.
It sometimes does not apply even to patch versions when the version bump carries some environment changes: e.g variations in dependency shared libraries. This is made extremely obvious e.g by Nix which strictly enforces this binary link, but applies beyond nix as well (as proven by our own CI sometimes failing and requiring cache busting); unless a lot of care is taken, it is good practice to rebuild the bundle from scratch when such a change happens. Still, it may be a trigger into things not working for a customer when they were before.

The issue could be addressed by packaging all patch versions for a given minor version instead of only the one matching, a case that was considered as a requirement (e.g 2.5.x, 3.3.0 and 3.3.5, 3.4.0-preview1 and 3.4.0-preview2) but not yet implemented in order to gather quick feedback with a minimal, early implementation.

Since Ivo remained unconvinced that this could be reliably addressed and this is ultimately solely profiling depending on this gem's behaviour as of today, I am closing this.

Remove unused files upon gem install

5bfb548

datadog-datadog-prod-us1 Bot reviewed Oct 30, 2024

View reviewed changes

lloeki commented Oct 30, 2024

View reviewed changes

marcotc reviewed Oct 30, 2024

View reviewed changes

marcotc approved these changes Oct 30, 2024

View reviewed changes

lloeki mentioned this pull request Oct 30, 2024

[NO-TICKET] Trim unneeded headers from gem #5

Merged

lloeki closed this Nov 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove unused files upon gem install#6

Remove unused files upon gem install#6
lloeki wants to merge 1 commit into
masterfrom
lloeki/shave-on-install

lloeki commented Oct 30, 2024 •

edited

Loading

Uh oh!

datadog-datadog-prod-us1 Bot Oct 30, 2024

Uh oh!

datadog-datadog-prod-us1 Bot Oct 30, 2024

Uh oh!

lloeki Oct 30, 2024

Uh oh!

lloeki Oct 30, 2024

Uh oh!

marcotc Oct 30, 2024

Uh oh!

lloeki Oct 30, 2024 •

edited

Loading

Uh oh!

lloeki commented Nov 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		dir = Datadog::RubyCoreSource.deduce_packaged_source_dir("ruby-#{RUBY_VERSION}-p#{RUBY_PATCHLEVEL}")

		unused = Dir.glob(File.dirname(dir) + '/*') - [dir]


		require 'datadog/ruby_core_source'

		dir = Datadog::RubyCoreSource.deduce_packaged_source_dir("ruby-#{RUBY_VERSION}-p#{RUBY_PATCHLEVEL}")

	ruby_dir = if RUBY_PATCHLEVEL < 0
	REVISION_MAP[RUBY_REVISION] or "ruby-#{RUBY_VERSION}"
	else
	"ruby-#{RUBY_VERSION}-p#{RUBY_PATCHLEVEL}"
	end

	REVISION_MAP = {
	# Add pre-release version here since they do not have patchlevel to refer to.
	# Revision can be found at `revision.h` of ruby sources.
	# Format of this hash:
	# <RUBY_REVISION> => '<sources directory name>', e.g. `61243 => 'ruby-2.5.0-rc1'`
	}

Uh oh!

Conversation

lloeki commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

datadog-datadog-prod-us1 Bot Oct 30, 2024

Choose a reason for hiding this comment

⚪ Code Quality Violation

Uh oh!

datadog-datadog-prod-us1 Bot Oct 30, 2024

Choose a reason for hiding this comment

⚪ Code Quality Violation

Uh oh!

lloeki Oct 30, 2024

Choose a reason for hiding this comment

Uh oh!

lloeki Oct 30, 2024

Choose a reason for hiding this comment

Uh oh!

marcotc Oct 30, 2024

Choose a reason for hiding this comment

Uh oh!

lloeki Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lloeki commented Nov 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lloeki commented Oct 30, 2024 •

edited

Loading

lloeki Oct 30, 2024 •

edited

Loading