diff options
author | Oswaldo Ferreira <oswaldo@gitlab.com> | 2018-09-04 23:06:34 +0300 |
---|---|---|
committer | Oswaldo Ferreira <oswaldo@gitlab.com> | 2018-09-07 16:07:18 +0300 |
commit | cd1d5b244072c6701e77c6bd7be9e9a0d60f8967 (patch) | |
tree | 56dc6c7e96e61cc47b08776152ca21d9e5708c57 /lib | |
parent | d73541d006ce3d0261cf082dac5ae605dfe7a848 (diff) |
Cache diff highlighting upon Merge Request creation (refactors diff caching)
Diffstat (limited to 'lib')
-rw-r--r-- | lib/gitlab/diff/file_collection/base.rb | 10 | ||||
-rw-r--r-- | lib/gitlab/diff/file_collection/merge_request_diff.rb | 63 | ||||
-rw-r--r-- | lib/gitlab/diff/highlight_cache.rb | 68 |
3 files changed, 92 insertions, 49 deletions
diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb index c79d8d3cb21..2acb0e43b69 100644 --- a/lib/gitlab/diff/file_collection/base.rb +++ b/lib/gitlab/diff/file_collection/base.rb @@ -2,7 +2,7 @@ module Gitlab module Diff module FileCollection class Base - attr_reader :project, :diff_options, :diff_refs, :fallback_diff_refs + attr_reader :project, :diff_options, :diff_refs, :fallback_diff_refs, :diffable delegate :count, :size, :real_size, to: :diff_files @@ -33,6 +33,14 @@ module Gitlab diff_files.find { |diff_file| diff_file.new_path == new_path } end + def clear_cache + # No-op + end + + def write_cache + # No-op + end + private def decorate_diff!(diff) diff --git a/lib/gitlab/diff/file_collection/merge_request_diff.rb b/lib/gitlab/diff/file_collection/merge_request_diff.rb index be25e1bab21..0dd073a3a8e 100644 --- a/lib/gitlab/diff/file_collection/merge_request_diff.rb +++ b/lib/gitlab/diff/file_collection/merge_request_diff.rb @@ -2,6 +2,8 @@ module Gitlab module Diff module FileCollection class MergeRequestDiff < Base + extend ::Gitlab::Utils::Override + def initialize(merge_request_diff, diff_options:) @merge_request_diff = merge_request_diff @@ -13,70 +15,35 @@ module Gitlab end def diff_files - # Make sure to _not_ send any method call to Gitlab::Diff::File - # _before_ all of them were collected (`super`). Premature method calls will - # trigger N+1 RPCs to Gitaly through BatchLoader records (Blob.lazy). - # diff_files = super - diff_files.each { |diff_file| cache_highlight!(diff_file) if cacheable?(diff_file) } - store_highlight_cache + diff_files.each { |diff_file| cache.decorate(diff_file) } diff_files end - def real_size - @merge_request_diff.real_size + override :write_cache + def write_cache + cache.write_if_empty end - def clear_cache! - Rails.cache.delete(cache_key) + override :clear_cache + def clear_cache + cache.clear end def cache_key - [@merge_request_diff, 'highlighted-diff-files', Gitlab::Diff::Line::SERIALIZE_KEYS, diff_options] - end - - private - - def highlight_diff_file_from_cache!(diff_file, cache_diff_lines) - diff_file.highlighted_diff_lines = cache_diff_lines.map do |line| - Gitlab::Diff::Line.init_from_hash(line) - end + cache.key end - # - # If we find the highlighted diff files lines on the cache we replace existing diff_files lines (no highlighted) - # for the highlighted ones, so we just skip their execution. - # If the highlighted diff files lines are not cached we calculate and cache them. - # - # The content of the cache is a Hash where the key identifies the file and the values are Arrays of - # hashes that represent serialized diff lines. - # - def cache_highlight!(diff_file) - item_key = diff_file.file_identifier - - if highlight_cache[item_key] - highlight_diff_file_from_cache!(diff_file, highlight_cache[item_key]) - else - highlight_cache[item_key] = diff_file.highlighted_diff_lines.map(&:to_hash) - end - end - - def highlight_cache - return @highlight_cache if defined?(@highlight_cache) - - @highlight_cache = Rails.cache.read(cache_key) || {} - @highlight_cache_was_empty = @highlight_cache.empty? - @highlight_cache + def real_size + @merge_request_diff.real_size end - def store_highlight_cache - Rails.cache.write(cache_key, highlight_cache, expires_in: 1.week) if @highlight_cache_was_empty - end + private - def cacheable?(diff_file) - @merge_request_diff.present? && diff_file.text? && diff_file.diffable? + def cache + @cache ||= Gitlab::Diff::HighlightCache.new(self) end end end diff --git a/lib/gitlab/diff/highlight_cache.rb b/lib/gitlab/diff/highlight_cache.rb new file mode 100644 index 00000000000..e4390771db2 --- /dev/null +++ b/lib/gitlab/diff/highlight_cache.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true +# +module Gitlab + module Diff + class HighlightCache + delegate :diffable, to: :@diff_collection + delegate :diff_options, to: :@diff_collection + + def initialize(diff_collection, backend: Rails.cache) + @backend = backend + @diff_collection = diff_collection + end + + # - Reads from cache + # - Assigns DiffFile#highlighted_diff_lines for cached files + def decorate(diff_file) + if content = read_file(diff_file) + diff_file.highlighted_diff_lines = content.map do |line| + Gitlab::Diff::Line.init_from_hash(line) + end + end + end + + # It populates a Hash in order to submit a single write to the memory + # cache. This avoids excessive IO generated by N+1's (1 writing for + # each highlighted line or file). + def write_if_empty + return if cached_content.present? + + @diff_collection.diff_files.each do |diff_file| + next unless cacheable?(diff_file) + + diff_file_id = diff_file.file_identifier + + cached_content[diff_file_id] = diff_file.highlighted_diff_lines.map(&:to_hash) + end + + cache.write(key, cached_content, expires_in: 1.week) + end + + def clear + cache.delete(key) + end + + def key + [diffable, 'highlighted-diff-files', Gitlab::Diff::Line::SERIALIZE_KEYS, diff_options] + end + + private + + def read_file(diff_file) + cached_content[diff_file.file_identifier] + end + + def cache + @backend + end + + def cached_content + @cached_content ||= cache.read(key) || {} + end + + def cacheable?(diff_file) + diffable.present? && diff_file.text? && diff_file.diffable? + end + end + end +end |