Gitlab::DiscussionsDiff::FileCollection#load_highlight does too much work when passed an empty array
Try this script on a production console:
mr = MergeRequest.find(16141050) # https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/7341/discussions.json
mr.note_diff_files.for_commit_or_unresolved.pluck(:id)
#=> []
dd = Gitlab::DiscussionsDiff::FileCollection.new(mr.note_diff_files.to_a)
Benchmark.realtime { dd.load_highlight([]) }
#=> 1.0429975129663944
That's 1 second to load highlights for no discussions
This is a simplified version of what happens from Projects::MergeRequestsController#discussions
-> MergeRequest#preload_discussions_diff_highlight
, which happens on every request to a discussions endpoint.
There are two places in Gitlab::DiscussionsDiff::FileCollection#preload_highlighted_lines
where we can call Gitaly, which I've annotated below:
def preload_highlighted_lines(ids)
cached_content = read_cache(ids)
uncached_ids = ids.select.each_with_index { |_, i| cached_content[i].nil? }
# Gitaly call
mapping = highlighted_lines_by_ids(uncached_ids)
HighlightCache.write_multiple(mapping)
# Gitaly call
diffs = diff_files_indexed_by_id.values_at(*ids)
diffs.zip(cached_content).each do |diff, cached_lines|
next unless diff && cached_lines
diff.highlighted_diff_lines = cached_lines
end
end
I think adding a return if highlightable_ids.empty?
to load_highlight
should take this, but there might also be other cases where we spend time fetching files we don't need.