Skip to content

Introduce a fast path for comparing Gem::Version #9414

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.

Sign up for GitHub

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

Merged
Edouard-chin merged 1 commit into from
Mar 20, 2026
Merged

Introduce a fast path for comparing Gem::Version #9414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions lib/rubygems/version.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ def initialize(version)
end
@version = -@version
@segments = nil
@sort_key = compute_sort_key
end

##
Expand Down Expand Up @@ -350,6 +351,12 @@ def <=>(other)
end

return unless Gem::Version === other

# Fast path for comparison when available.
if @sort_key && other.sort_key
return @sort_key <=> other.sort_key
end

return 0 if @version == other.version || canonical_segments == other.canonical_segments

lhsegments = canonical_segments
Expand Down Expand Up @@ -415,6 +422,21 @@ def freeze

protected

attr_reader :sort_key # :nodoc:

def compute_sort_key
segments = canonical_segments
return if segments.size > 4 || prerelease? || segments.any? {|segment| segment > 65_000 }

base = 1_000_000_000_000

segments.sum do |segment|
result = segment * base
base /= 10_000

@jhawthorn jhawthorn Mar 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not be the same number (or larger) as used above in segment > 65_000

@Edouard-chin Edouard-chin Mar 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I orginally went with 65_000 (by looking at our own Gemfile.lock) as I wanted to exclude computing a sort key for gems that get released with a timestamp version such as 20180730171000).

We could compute a sort key for those type of gems though. However I need to dig, because I tried now and it seems that the resolver is taking forever to solve the versions (been waiting 3 minutes on our monolith and it still hasn't finished resolving). Let me get back to you with an explanation, as I don't know the reason yet

@Edouard-chin Edouard-chin Mar 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, some private gems changed their version scheme, it used to be

0.0.1.<timestamp>(Where timestamp is 14 digits) , but changed to follow semver later on.

So we end up with a older version that has a larger computed sort key integer.

Gem::Version.new("0.1.96.20210202214054") > Gem::Version.new("1.0.0") #=> true

result
end
Comment on lines +428 to +437

@paracycle paracycle Mar 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I'd much rather prefer this as:

Suggested change
segments = canonical_segments
return if segments.size > 4 || prerelease? || segments.any? {|segment| segment > 65_000 }
base = 1_000_000_000_000
segments.sum do |segment|
result = segment * base
base /= 10_000
result
end
return if prerelease?
base = 1
multiplier = 10_000
canonical_segments.reverse_each.with_index.sum do |segment, index|
return if segment > multiplier || index >= 4
result = segment * base
base *= multiplier
result
end

since I find it to be more readable, we only iterate over the segment list once, and the multiplier bounds check is handled in the same place where the multiplication happens.

@Edouard-chin Edouard-chin Mar 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if your suggestion works.

Gem::Version.new("1.0.0") # Sort key is 1 (Canonical segments is [1])
Gem::Version.new("0.8.2") # Sort key 80002 (Canonical segments is [0, 8, 2])

end

def _segments
# segments is lazy so it can pick up version values that come from
# old marshaled versions, which don't go through marshal_load.
Expand Down
31 changes: 31 additions & 0 deletions test/rubygems/test_gem_version.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,37 @@ def test_semver
assert_less_than "1.0.0-1", "1"
end

def test_sort_key_is_computed_on_regular_release
refute_nil v("9.8.7").send(:sort_key)
end

def test_sort_key_is_computed_on_security_release
refute_nil v("9.8.7.1").send(:sort_key)
end

def test_sort_key_is_not_computed_on_prerelease
assert_nil v("9.8.7.pre1").send(:sort_key)
end

def test_sort_key_is_not_computed_on_version_with_more_segments
assert_nil v("1.1.1.1.1.1.1").send(:sort_key)
end

def test_sort_key_is_not_computed_on_huge_numbers
assert_nil v("2.30.1.250000").send(:sort_key)
end

def test_sort_key_is_used_for_comparison
a = v("18.0.1")
b = v("18.0.2")

# Ensure the slow path isn't getting hit
a.instance_variable_set(:@version, nil)
a.instance_variable_set(:@canonical_segments, nil)

assert_operator(a, :<, b)
end

# modifying the segments of a version should not affect the segments of the cached version object
def test_segments
v("9.8.7").segments[2] += 1
Expand Down
Loading