Skip to content

Commit bf71b0e

Browse files
segiddinsmatzbot
authored andcommitted
[rubygems/rubygems] Optimize allocations in Gem::Version
From running in a random rails app I have locally, here are the changes 1) for `bundle lock --update --bundler` (forcing Bundler to go through dependency resolution) ``` ==> memprof.after.txt <== Total allocated: 2.98 MB (48307 objects) Total retained: 1.21 MB (16507 objects) ==> memprof.before.txt <== Total allocated: 12.62 MB (198506 objects) Total retained: 1.30 MB (23133 objects) ``` 2) for `bin/rails runner true` (essentially only bundler/setup) ``` ==> memprof.after.txt <== Total allocated: 59.50 kB (1017 objects) Total retained: 25.08 kB (362 objects) ==> memprof.before.txt <== Total allocated: 561.82 kB (8575 objects) Total retained: 27.28 kB (513 objects) ``` rubygems/rubygems@35c8ed2cb8
1 parent 5810304 commit bf71b0e

File tree

2 files changed

+40
-20
lines changed

2 files changed

+40
-20
lines changed

lib/rubygems/query_utils.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,8 @@ def spec_loaded_from(entry, spec, specs)
311311
label = "Installed at"
312312
specs.each do |s|
313313
version = s.version.to_s
314-
version << ", default" if s.default_gem?
315-
entry << "\n" << " #{label} (#{version}): #{s.base_dir}"
314+
default = ", default" if s.default_gem?
315+
entry << "\n" << " #{label} (#{version}#{default}): #{s.base_dir}"
316316
label = " " * label.length
317317
end
318318
end

lib/rubygems/version.rb

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ class Gem::Version
162162
# A string representation of this Version.
163163

164164
def version
165-
@version.dup
165+
@version
166166
end
167167

168168
alias_method :to_s, :version
@@ -173,7 +173,7 @@ def version
173173
def self.correct?(version)
174174
nil_versions_are_discouraged! if version.nil?
175175

176-
!!(version.to_s =~ ANCHORED_VERSION_PATTERN)
176+
ANCHORED_VERSION_PATTERN.match?(version.to_s)
177177
end
178178

179179
##
@@ -224,9 +224,17 @@ def initialize(version)
224224
end
225225

226226
# If version is an empty string convert it to 0
227-
version = 0 if version.is_a?(String) && version =~ /\A\s*\Z/
227+
version = 0 if version.is_a?(String) && /\A\s*\Z/.match?(version)
228+
229+
@version = version.to_s
228230

229-
@version = version.to_s.strip.gsub("-",".pre.")
231+
# optimization to avoid allocation when given an integer, since we know
232+
# it's to_s won't have any spaces or dashes
233+
unless version.is_a?(Integer)
234+
@version = @version.strip
235+
@version.gsub!("-",".pre.")
236+
end
237+
@version = -@version
230238
@segments = nil
231239
end
232240

@@ -252,7 +260,7 @@ def bump
252260
# same precision. Version "1.0" is not the same as version "1".
253261

254262
def eql?(other)
255-
self.class === other && @version == other._version
263+
self.class === other && @version == other.version
256264
end
257265

258266
def hash # :nodoc:
@@ -284,7 +292,7 @@ def marshal_load(array)
284292
end
285293

286294
def yaml_initialize(tag, map) # :nodoc:
287-
@version = map["version"]
295+
@version = -map["version"]
288296
@segments = nil
289297
@hash = nil
290298
end
@@ -302,7 +310,7 @@ def encode_with(coder) # :nodoc:
302310

303311
def prerelease?
304312
unless instance_variable_defined? :@prerelease
305-
@prerelease = !(@version =~ /[a-zA-Z]/).nil?
313+
@prerelease = /[a-zA-Z]/.match?(version)
306314
end
307315
@prerelease
308316
end
@@ -354,7 +362,7 @@ def <=>(other)
354362
return self <=> self.class.new(other) if (String === other) && self.class.correct?(other)
355363

356364
return unless Gem::Version === other
357-
return 0 if @version == other._version || canonical_segments == other.canonical_segments
365+
return 0 if @version == other.version || canonical_segments == other.canonical_segments
358366

359367
lhsegments = canonical_segments
360368
rhsegments = other.canonical_segments
@@ -381,10 +389,26 @@ def <=>(other)
381389
end
382390

383391
def canonical_segments
384-
@canonical_segments ||=
385-
_split_segments.map! do |segments|
386-
segments.reverse_each.drop_while {|s| s == 0 }.reverse
387-
end.reduce(&:concat)
392+
@canonical_segments ||= begin
393+
numeric_segments, string_segments = _split_segments
394+
canonical_segments = []
395+
396+
seen_non_zero = false
397+
string_segments.reverse_each do |segment|
398+
if seen_non_zero || (seen_non_zero = (segment != 0))
399+
canonical_segments << segment
400+
end
401+
end
402+
seen_non_zero = false
403+
numeric_segments.reverse_each do |segment|
404+
if seen_non_zero || (seen_non_zero = (segment != 0))
405+
canonical_segments << segment
406+
end
407+
end
408+
409+
canonical_segments.reverse!
410+
canonical_segments.freeze
411+
end
388412
end
389413

390414
def freeze
@@ -395,17 +419,13 @@ def freeze
395419

396420
protected
397421

398-
def _version
399-
@version
400-
end
401-
402422
def _segments
403423
# segments is lazy so it can pick up version values that come from
404424
# old marshaled versions, which don't go through marshal_load.
405425
# since this version object is cached in @@all, its @segments should be frozen
406426

407-
@segments ||= @version.scan(/[0-9]+|[a-z]+/i).map do |s|
408-
/^\d+$/.match?(s) ? s.to_i : s
427+
@segments ||= @version.scan(/[0-9]+|[a-z]+/i).map! do |s|
428+
/^\d+$/.match?(s) ? s.to_i : -s
409429
end.freeze
410430
end
411431

0 commit comments

Comments
 (0)