Skip to content
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

Speed up the ContainExactly matcher #1333

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 43 additions & 5 deletions lib/rspec/matchers/built_in/contain_exactly.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,55 @@ def best_solution
@best_solution ||= pairings_maximizer.find_best_solution
end

def pairings_maximizer
# This implementation is complicated but it facilitates an enormous speedup.
# The previous implementation never finished when comparing arrays of 50
# random integers between 1 and 10 (I stopped the execution after a full day).
# This implementation finishes that task in .03s on my laptop.
# This represents a speedup of over 259,000x for that task! The gap widens
# exponentially as the array size increases:
# Assuming the old implementation takes a day to compare arrays of 50 random
# integers, comparing arrays of 1000 elements would take 10^2486 billion years.
# The new implementation finishes this task in 1 second!
def pairings_maximizer # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
@pairings_maximizer ||= begin
expected_matches = Hash[Array.new(expected.size) { |i| [i, []] }]
actual_matches = Hash[Array.new(actual.size) { |i| [i, []] }]

expected.each_with_index do |e, ei|
actual.each_with_index do |a, ai|
# Set the reciprocal pairings for matching elements
value_buckets = Hash.new { |hash, key| hash[key] = [] }
expected.each_with_index { |v, i| value_buckets[[v, :expected]] << i }
actual.each_with_index { |v, i| value_buckets[[v, :actual]] << i }
value_buckets.each do |key, indexes|
value, source = key
next unless source == :expected
next unless value_buckets.key? [value, :actual]

actual_indexes = value_buckets[[value, :actual]]
indexes.zip(actual_indexes).each do |expected_index, actual_index|
break unless actual_index

expected_matches[expected_index] << actual_index
actual_matches[actual_index] << expected_index
end
end

# Set the remaining elements to be paired
filtered_expected = []
filtered_actual = []
expected.each_with_index do |e, expected_index|
filtered_expected << [e, expected_index] if expected_matches[expected_index].empty?
end
actual.each_with_index do |a, actual_index|
filtered_actual << [a, actual_index] if actual_matches[actual_index].empty?
end

# Set the pairing combinations of the remaining elements
filtered_expected.each do |e, expected_index|
filtered_actual.each do |a, actual_index|
next unless values_match?(e, a)

expected_matches[ei] << ai
actual_matches[ai] << ei
expected_matches[expected_index] << actual_index
actual_matches[actual_index] << expected_index
end
end

Expand Down
80 changes: 68 additions & 12 deletions spec/rspec/matchers/built_in/contain_exactly_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -184,22 +184,78 @@ def array.send; :sent; end
MESSAGE
end

def timeout_if_not_debugging(time)
in_sub_process_if_possible do
require 'timeout'
return yield if defined?(::Debugger)
Timeout.timeout(time) { yield }
context "speed checks" do
def timeout_if_not_debugging(time)
in_sub_process_if_possible do
require 'timeout'
return yield if defined?(::Debugger)
Timeout.timeout(time) { yield }
end
end
end

it 'fails a match of 11 items with duplicates in a reasonable amount of time' do
timeout_if_not_debugging(0.1) do
expected = [0, 1, 1, 3, 3, 3, 4, 4, 8, 8, 9 ]
actual = [ 1, 2, 3, 3, 3, 3, 7, 8, 8, 9, 9]
shared_examples "for succeeding quickly" do
it do
timeout_if_not_debugging(max_runtime) do
subject
pirj marked this conversation as resolved.
Show resolved Hide resolved
end
end
end

expect {
shared_examples "for failing fast with" do |failure_msg|
it do
timeout_if_not_debugging(max_runtime) do
expect {
subject
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
subject
expect_actual_to_contain_exactly_expected

Copy link
Member

Choose a reason for hiding this comment

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

subject is a substitute for expect_actual_to_contain_exactly_expected or expect_actual_not_to_contain_exactly_expected depending on the including example group.

}.to fail_with(/#{Regexp.quote(failure_msg)}/)
end
end
end

let(:max_runtime) { 0.5 }
let(:actual) { Array.new(1000) { rand(10) } }

context "with a positive expectation" do
subject(:expect_actual_to_contain_exactly_expected) do
expect(actual).to contain_exactly(*expected)
}.to fail_including("the missing elements were: [0, 1, 4, 4]")
end

context "that is valid" do
let(:expected) { actual.shuffle }

it "matches" do
expect_actual_to_contain_exactly_expected
end

include_examples "for succeeding quickly"
end

context "that is not valid" do
let(:expected) { Array.new(1000) { rand(10) } }

include_examples "for failing fast with", "expected collection contained"
end
end

context "with a negative expectation" do
subject(:expect_actual_not_to_contain_exactly_expected) do
expect(actual).not_to contain_exactly(*expected)
end

context "that is valid" do
let(:expected) { Array.new(1000) { rand(10) } }

it "does not match" do
expect_actual_not_to_contain_exactly_expected
end

include_examples "for succeeding quickly"
end

context "that is not valid" do
let(:expected) { actual.shuffle }

include_examples "for failing fast with", "not to contain exactly"
end
end
end

Expand Down