Skip to content

Commit

Permalink
Look more closely at hashdiff's performance issue
Browse files Browse the repository at this point in the history
liufengyun/hashdiff#49
phew, already avoided
  • Loading branch information
odinhb committed Apr 9, 2024
1 parent 0e66e64 commit d644937
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 0 deletions.
5 changes: 5 additions & 0 deletions lib/specdiff/differ/hash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@ def self.diff(a, b)
# array_path: true returns the path as an array, which differentiates
# between symbol keys and string keys in hashes, while the string
# representation does not.

# hmm it really seems like use_lcs: true gives much less human-readable
# (human-comprehensible) output when arrays are involved.

# use_lcs: true may also cause Hashdiff to use a lot of memory when BIG
# arrays are involved: https://github.com/liufengyun/hashdiff/issues/49
# so we might as well avoid that problem altogether.
hashdiff_diff = ::Hashdiff.diff(
a.value, b.value,
array_path: true,
Expand Down
19 changes: 19 additions & 0 deletions spec/diff_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1020,6 +1020,25 @@ def diff(...) = Specdiff.diff(...)
expect(result.to_s).to eq("")
expect(result.types).to eq([:array, :array])
end

# performance test, this makes sure that hashdiff's performance problems
# are not encountered: https://github.com/liufengyun/hashdiff/issues/49
it "really big arrays" do
a = [nil] * 17_000
b = [:nil] * 17_000

result = nil
runtime = measure_runtime do
result = diff(a, b)
end

expect(result.to_s).not_to eq("")
expect(result.to_s.size).to be > (100_000)
expect(result.empty?).to eq(false)
expect(result.types).to eq([:array, :array])

expect(runtime).to be < (0.3) # seconds
end
end

describe "refusing to diff" do
Expand Down
8 changes: 8 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,11 @@ class MyBasicObjectClass < BasicObject
class ConstantForTheSolePurposeOfUndefiningInspect
undef_method :inspect
end

def measure_runtime
t0 = Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second)
yield
t1 = Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second)

t1 - t0
end

0 comments on commit d644937

Please sign in to comment.