Commit 324af4ac authored by Douwe Maan's avatar Douwe Maan

Merge branch 'diffcollection-no-restarts' into 'master'

Fix buffering in DiffCollection

See merge request !11659
parents a59165e7 16168b5b
...@@ -19,22 +19,19 @@ module Gitlab ...@@ -19,22 +19,19 @@ module Gitlab
@line_count = 0 @line_count = 0
@byte_count = 0 @byte_count = 0
@overflow = false @overflow = false
@empty = true
@array = Array.new @array = Array.new
end end
def each(&block) def each(&block)
if @populated Gitlab::GitalyClient.migrate(:commit_raw_diffs) do
# @iterator.each is slower than just iterating the array in place each_patch(&block)
@array.each(&block)
else
Gitlab::GitalyClient.migrate(:commit_raw_diffs) do
each_patch(&block)
end
end end
end end
def empty? def empty?
!@iterator.any? any? # Make sure the iterator has been exercised
@empty
end end
def overflow? def overflow?
...@@ -60,7 +57,6 @@ module Gitlab ...@@ -60,7 +57,6 @@ module Gitlab
collection = each_with_index do |element, i| collection = each_with_index do |element, i|
@array[i] = yield(element) @array[i] = yield(element)
end end
@populated = true
collection collection
end end
...@@ -72,7 +68,6 @@ module Gitlab ...@@ -72,7 +68,6 @@ module Gitlab
return if @populated return if @populated
each { nil } # force a loop through all diffs each { nil } # force a loop through all diffs
@populated = true
nil nil
end end
...@@ -81,15 +76,17 @@ module Gitlab ...@@ -81,15 +76,17 @@ module Gitlab
end end
def each_patch def each_patch
@iterator.each_with_index do |raw, i| i = 0
# First yield cached Diff instances from @array @array.each do |diff|
if @array[i] yield diff
yield @array[i] i += 1
next end
end
return if @overflow
return if @iterator.nil?
# We have exhausted @array, time to create new Diff instances or stop. @iterator.each do |raw|
break if @overflow @empty = false
if !@all_diffs && i >= @max_files if !@all_diffs && i >= @max_files
@overflow = true @overflow = true
...@@ -115,7 +112,13 @@ module Gitlab ...@@ -115,7 +112,13 @@ module Gitlab
end end
yield @array[i] = diff yield @array[i] = diff
i += 1
end end
@populated = true
# Allow iterator to be garbage-collected. It cannot be reused anyway.
@iterator = nil
end end
end end
end end
......
...@@ -10,7 +10,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do ...@@ -10,7 +10,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do
no_collapse: no_collapse no_collapse: no_collapse
) )
end end
let(:iterator) { Array.new(file_count, fake_diff(line_length, line_count)) } let(:iterator) { MutatingConstantIterator.new(file_count, fake_diff(line_length, line_count)) }
let(:file_count) { 0 } let(:file_count) { 0 }
let(:line_length) { 1 } let(:line_length) { 1 }
let(:line_count) { 1 } let(:line_count) { 1 }
...@@ -64,7 +64,15 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do ...@@ -64,7 +64,15 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do
subject { super().real_size } subject { super().real_size }
it { is_expected.to eq('3') } it { is_expected.to eq('3') }
end end
it { expect(subject.size).to eq(3) }
describe '#size' do
it { expect(subject.size).to eq(3) }
it 'does not change after peeking' do
subject.any?
expect(subject.size).to eq(3)
end
end
context 'when limiting is disabled' do context 'when limiting is disabled' do
let(:all_diffs) { true } let(:all_diffs) { true }
...@@ -83,7 +91,15 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do ...@@ -83,7 +91,15 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do
subject { super().real_size } subject { super().real_size }
it { is_expected.to eq('3') } it { is_expected.to eq('3') }
end end
it { expect(subject.size).to eq(3) }
describe '#size' do
it { expect(subject.size).to eq(3) }
it 'does not change after peeking' do
subject.any?
expect(subject.size).to eq(3)
end
end
end end
end end
...@@ -457,4 +473,22 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do ...@@ -457,4 +473,22 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do
def fake_diff(line_length, line_count) def fake_diff(line_length, line_count)
{ 'diff' => "#{'a' * line_length}\n" * line_count } { 'diff' => "#{'a' * line_length}\n" * line_count }
end end
class MutatingConstantIterator
include Enumerable
def initialize(count, value)
@count = count
@value = value
end
def each
loop do
break if @count.zero?
# It is critical to decrement before yielding. We may never reach the lines after 'yield'.
@count -= 1
yield @value
end
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment