Skip to content

Commit

Permalink
Fewer allocations in gem installation
Browse files Browse the repository at this point in the history
For now, on a small rails app I have hanging around:

```
==> memprof.after.txt <==
Total allocated: 872.51 MB (465330 objects)
Total retained:  40.48 kB (326 objects)

==> memprof.before.txt <==
Total allocated: 890.79 MB (1494026 objects)
Total retained:  40.40 kB (328 objects)
```

Not a huge difference in memory usage, but it's a drastic improvement
in total number of allocations.

Additionally, this will pay huge dividends once
ruby/zlib#61 is merged, as it will allow us to
completely avoid allocations in the repeated calls to readpartial,
which currently accounts for most of the memory usage shown above.
  • Loading branch information
segiddins committed Dec 11, 2023
1 parent 5fc69f5 commit f78d45d
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 31 deletions.
6 changes: 3 additions & 3 deletions lib/rubygems/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,11 @@ def check_executable_overwrite(filename) # :nodoc:

File.open generated_bin, "rb" do |io|
line = io.gets
shebang = /^#!.*ruby/
shebang = /^#!.*ruby/o

# TruffleRuby uses a bash prelude in default launchers
if load_relative_enabled? || RUBY_ENGINE == "truffleruby"
until line.nil? || line =~ shebang do
until line.nil? || shebang.match?(line) do
line = io.gets
end
end
Expand All @@ -239,7 +239,7 @@ def check_executable_overwrite(filename) # :nodoc:

# TODO: detect a specially formatted comment instead of trying
# to find a string inside Ruby code.
next unless io.gets.to_s.include?("This file was generated by RubyGems")
next unless io.gets&.include?("This file was generated by RubyGems")

ruby_executable = true
existing = io.read.slice(/
Expand Down
24 changes: 13 additions & 11 deletions lib/rubygems/package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -357,18 +357,21 @@ def contents

def digest(entry) # :nodoc:
algorithms = if @checksums
@checksums.keys
else
[Gem::Security::DIGEST_NAME].compact
@checksums.to_h {|algorithm, _| [algorithm, Gem::Security.create_digest(algorithm)] }
elsif Gem::Security::DIGEST_NAME
{ Gem::Security::DIGEST_NAME => Gem::Security.create_digest(Gem::Security::DIGEST_NAME) }
end

algorithms.each do |algorithm|
digester = Gem::Security.create_digest(algorithm)

digester << entry.readpartial(16_384) until entry.eof?
return @digests if algorithms.nil? || algorithms.empty?

entry.rewind
buf = String.new(capacity: 16_384, encoding: Encoding::BINARY)
until entry.eof?
entry.readpartial(16_384, buf)
algorithms.each_value {|digester| digester << buf }
end
entry.rewind

algorithms.each do |algorithm, digester|
@digests[algorithm][entry.full_name] = digester
end

Expand Down Expand Up @@ -437,8 +440,6 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc:

FileUtils.rm_rf destination

mkdir_options = {}
mkdir_options[:mode] = dir_mode ? 0o755 : (entry.header.mode if entry.directory?)
mkdir =
if entry.directory?
destination
Expand All @@ -447,7 +448,7 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc:
end

unless directories.include?(mkdir)
FileUtils.mkdir_p mkdir, **mkdir_options
FileUtils.mkdir_p mkdir, mode: dir_mode ? 0o755 : (entry.header.mode if entry.directory?)
directories << mkdir
end

Expand Down Expand Up @@ -707,6 +708,7 @@ def verify_files(gem)

def verify_gz(entry) # :nodoc:
Zlib::GzipReader.wrap entry do |gzio|
# TODO: read into a buffer once zlib supports it
gzio.read 16_384 until gzio.eof? # gzip checksum verification
end
rescue Zlib::GzipFile::Error => e
Expand Down
38 changes: 21 additions & 17 deletions lib/rubygems/package/tar_header.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ def self.from(stream)
end

def self.strict_oct(str)
return str.strip.oct if /\A[0-7]*\z/.match?(str.strip)
str.strip!
return str.oct if /\A[0-7]*\z/.match?(str)

raise ArgumentError, "#{str.inspect} is not an octal string"
end
Expand All @@ -137,7 +138,8 @@ def self.oct_or_256based(str)
# \ff flags a negative 256-based number
# In case we have a match, parse it as a signed binary value
# in big-endian order, except that the high-order bit is ignored.
return str.unpack("N2").last if /\A[\x80\xff]/n.match?(str)

return str.unpack1("@4N") if /\A[\x80\xff]/n.match?(str)
strict_oct(str)
end

Expand All @@ -149,21 +151,23 @@ def initialize(vals)
raise ArgumentError, ":name, :size, :prefix and :mode required"
end

vals[:uid] ||= 0
vals[:gid] ||= 0
vals[:mtime] ||= 0
vals[:checksum] ||= ""
vals[:typeflag] = "0" if vals[:typeflag].nil? || vals[:typeflag].empty?
vals[:magic] ||= "ustar"
vals[:version] ||= "00"
vals[:uname] ||= "wheel"
vals[:gname] ||= "wheel"
vals[:devmajor] ||= 0
vals[:devminor] ||= 0

FIELDS.each do |name|
instance_variable_set "@#{name}", vals[name]
end
@checksum = vals[:checksum] || ""
@devmajor = vals[:devmajor] || 0
@devminor = vals[:devminor] || 0
@gid = vals[:gid] || 0
@gname = vals[:gname] || "wheel"
@linkname = vals[:linkname]
@magic = vals[:magic] || "ustar"
@mode = vals[:mode]
@mtime = vals[:mtime] || 0
@name = vals[:name]
@prefix = vals[:prefix]
@size = vals[:size]
@typeflag = vals[:typeflag]
@typeflag = "0" if @typeflag.nil? || @typeflag.empty?
@uid = vals[:uid] || 0
@uname = vals[:uname] || "wheel"
@version = vals[:version] || "00"

@empty = vals[:empty]
end
Expand Down

0 comments on commit f78d45d

Please sign in to comment.