Merge branch 'jej/fix-lfs-changes-laziness' into 'master'

Implement lazy popen so LfsChanges doesn't have to wait for rev-list to complete

See merge request gitlab-org/gitlab-ce!15180

3 files changed, 42 insertions, 27 deletions

diff --git a/lib/gitlab/git/lfs_changes.rb b/lib/gitlab/git/lfs_changes.rb
index 2749e2e69e2..732dd5d998a 100644
--- a/lib/gitlab/git/lfs_changes.rb
+++ b/lib/gitlab/git/lfs_changes.rb
@@ -8,25 +8,22 @@ module Gitlab
 
       def new_pointers(object_limit: nil, not_in: nil)
         @new_pointers ||= begin
-          object_ids = new_objects(not_in: not_in)
-          object_ids = object_ids.take(object_limit) if object_limit
+          rev_list.new_objects(not_in: not_in, require_path: true) do |object_ids|
+            object_ids = object_ids.take(object_limit) if object_limit
 
-          Gitlab::Git::Blob.batch_lfs_pointers(@repository, object_ids)
+            Gitlab::Git::Blob.batch_lfs_pointers(@repository, object_ids)
+          end
         end
       end
 
       def all_pointers
-        object_ids = rev_list.all_objects(require_path: true)
-
-        Gitlab::Git::Blob.batch_lfs_pointers(@repository, object_ids)
+        rev_list.all_objects(require_path: true) do |object_ids|
+          Gitlab::Git::Blob.batch_lfs_pointers(@repository, object_ids)
+        end
       end
 
       private
 
-      def new_objects(not_in:)
-        rev_list.new_objects(require_path: true, lazy: true, not_in: not_in)
-      end
-
       def rev_list
         ::Gitlab::Git::RevList.new(path_to_repo: @repository.path_to_repo,
                                    newrev: @newrev)
diff --git a/lib/gitlab/git/popen.rb b/lib/gitlab/git/popen.rb
index b45da6020ee..d41fe78daa1 100644
--- a/lib/gitlab/git/popen.rb
+++ b/lib/gitlab/git/popen.rb
@@ -7,7 +7,7 @@ module Gitlab
     module Popen
       FAST_GIT_PROCESS_TIMEOUT = 15.seconds
 
-      def popen(cmd, path, vars = {})
+      def popen(cmd, path, vars = {}, lazy_block: nil)
         unless cmd.is_a?(Array)
           raise "System commands must be given as an array of strings"
         end
@@ -22,7 +22,12 @@ module Gitlab
           yield(stdin) if block_given?
           stdin.close
 
-          @cmd_output << stdout.read
+          if lazy_block
+            return lazy_block.call(stdout.lazy)
+          else
+            @cmd_output << stdout.read
+          end
+
           @cmd_output << stderr.read
           @cmd_status = wait_thr.value.exitstatus
         end
diff --git a/lib/gitlab/git/rev_list.rb b/lib/gitlab/git/rev_list.rb
index e0c884aceaa..4974205b8fd 100644
--- a/lib/gitlab/git/rev_list.rb
+++ b/lib/gitlab/git/rev_list.rb
@@ -25,17 +25,18 @@ module Gitlab
       # This skips commit objects and root trees, which might not be needed when
       # looking for blobs
       #
-      # Can return a lazy enumerator to limit work done on megabytes of data
-      def new_objects(require_path: nil, lazy: false, not_in: nil)
-        object_output = execute([*base_args, newrev, *not_in_refs(not_in), '--objects'])
+      # When given a block it will yield objects as a lazy enumerator so
+      # the caller can limit work done instead of processing megabytes of data
+      def new_objects(require_path: nil, not_in: nil, &lazy_block)
+        args = [*base_args, newrev, *not_in_refs(not_in), '--objects']
 
-        objects_from_output(object_output, require_path: require_path, lazy: lazy)
+        get_objects(args, require_path: require_path, &lazy_block)
       end
 
-      def all_objects(require_path: nil)
-        object_output = execute([*base_args, '--all', '--objects'])
+      def all_objects(require_path: nil, &lazy_block)
+        args = [*base_args, '--all', '--objects']
 
-        objects_from_output(object_output, require_path: require_path, lazy: true)
+        get_objects(args, require_path: require_path, &lazy_block)
       end
 
       # This methods returns an array of missed references
@@ -64,6 +65,10 @@ module Gitlab
         output.split("\n")
       end
 
+      def lazy_execute(args, &lazy_block)
+        popen(args, nil, Gitlab::Git::Env.to_env_hash, lazy_block: lazy_block)
+      end
+
       def base_args
         [
           Gitlab.config.git.bin_path,
@@ -72,20 +77,28 @@ module Gitlab
         ]
       end
 
-      def objects_from_output(object_output, require_path: nil, lazy: nil)
-        objects = object_output.lazy.map do |output_line|
+      def get_objects(args, require_path: nil)
+        if block_given?
+          lazy_execute(args) do |lazy_output|
+            objects = objects_from_output(lazy_output, require_path: require_path)
+
+            yield(objects)
+          end
+        else
+          object_output = execute(args)
+
+          objects_from_output(object_output, require_path: require_path)
+        end
+      end
+
+      def objects_from_output(object_output, require_path: nil)
+        object_output.map do |output_line|
           sha, path = output_line.split(' ', 2)
 
           next if require_path && path.blank?
 
           sha
         end.reject(&:nil?)
-
-        if lazy
-          objects
-        else
-          objects.force
-        end
       end
     end
   end