Impove the performance of expanding full diff

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/58597

9 files changed, 193 insertions, 77 deletions

diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue
index 8d09c2a7399..2b3d6d1a3fa 100644
--- a/app/assets/javascripts/diffs/components/diff_content.vue
+++ b/app/assets/javascripts/diffs/components/diff_content.vue
@@ -1,5 +1,6 @@
 <script>
 import { mapActions, mapGetters, mapState } from 'vuex';
+import { GlLoadingIcon } from '@gitlab/ui';
 import diffLineNoteFormMixin from 'ee_else_ce/notes/mixins/diff_line_note_form';
 import draftCommentsMixin from 'ee_else_ce/diffs/mixins/draft_comments';
 import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
@@ -16,6 +17,7 @@ import { diffViewerModes } from '~/ide/constants';
 
 export default {
   components: {
+    GlLoadingIcon,
     InlineDiffView,
     ParallelDiffView,
     DiffViewer,
@@ -108,6 +110,7 @@ export default {
           :diff-lines="diffFile.parallel_diff_lines || []"
           :help-page-path="helpPagePath"
         />
+        <gl-loading-icon v-if="diffFile.renderingLines" size="md" class="mt-3" />
       </template>
       <not-diffable-viewer v-else-if="notDiffable" />
       <no-preview-viewer v-else-if="noPreview" />
diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
index 6709df48637..1281f9b17ef 100644
--- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
@@ -84,8 +84,6 @@ export default {
     },
     shouldShowCommentButton() {
       return (
-        this.isLoggedIn &&
-        this.showCommentButton &&
         this.isHover &&
         !this.isMatchLine &&
         !this.isContextLine &&
@@ -102,6 +100,9 @@ export default {
       }
       return this.showCommentButton && this.hasDiscussions;
     },
+    shouldRenderCommentButton() {
+      return this.isLoggedIn && this.showCommentButton;
+    },
   },
   methods: {
     ...mapActions('diffs', ['loadMoreLines', 'showCommentForm', 'setHighlightedRow']),
@@ -167,6 +168,7 @@ export default {
     >
     <template v-else>
       <button
+        v-if="shouldRenderCommentButton"
         v-show="shouldShowCommentButton"
         type="button"
         class="add-diff-note js-add-diff-note-button qa-diff-comment"
diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue
index d174b13e133..0f3e9208d21 100644
--- a/app/assets/javascripts/diffs/components/diff_table_cell.vue
+++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue
@@ -89,17 +89,19 @@ export default {
     classNameMap() {
       const { type } = this.line;
 
-      return {
-        hll: this.isHighlighted,
-        [type]: type,
-        [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine,
-        [LINE_HOVER_CLASS_NAME]:
-          this.isLoggedIn &&
-          this.isHover &&
-          !this.isMatchLine &&
-          !this.isContextLine &&
-          !this.isMetaLine,
-      };
+      return [
+        type,
+        {
+          hll: this.isHighlighted,
+          [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine,
+          [LINE_HOVER_CLASS_NAME]:
+            this.isLoggedIn &&
+            this.isHover &&
+            !this.isMatchLine &&
+            !this.isContextLine &&
+            !this.isMetaLine,
+        },
+      ];
     },
     lineNumber() {
       return this.lineType === OLD_LINE_TYPE ? this.line.old_line : this.line.new_line;
diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
index c764cbeb8e0..2d5262baeec 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
@@ -1,12 +1,11 @@
 <script>
-import { mapGetters, mapActions, mapState } from 'vuex';
+import { mapActions, mapState } from 'vuex';
 import DiffTableCell from './diff_table_cell.vue';
 import {
   NEW_LINE_TYPE,
   OLD_LINE_TYPE,
   CONTEXT_LINE_TYPE,
   CONTEXT_LINE_CLASS_NAME,
-  PARALLEL_DIFF_VIEW_TYPE,
   LINE_POSITION_LEFT,
   LINE_POSITION_RIGHT,
 } from '../constants';
@@ -45,16 +44,16 @@ export default {
         return this.line.line_code !== null && this.line.line_code === state.diffs.highlightedRow;
       },
     }),
-    ...mapGetters('diffs', ['isInlineView']),
     isContextLine() {
       return this.line.type === CONTEXT_LINE_TYPE;
     },
     classNameMap() {
-      return {
-        [this.line.type]: this.line.type,
-        [CONTEXT_LINE_CLASS_NAME]: this.isContextLine,
-        [PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView,
-      };
+      return [
+        this.line.type,
+        {
+          [CONTEXT_LINE_CLASS_NAME]: this.isContextLine,
+        },
+      ];
     },
     inlineRowId() {
       return this.line.line_code || `${this.fileHash}_${this.line.old_line}_${this.line.new_line}`;
diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js
index 4feb73cfef2..d84e1af11f3 100644
--- a/app/assets/javascripts/diffs/constants.js
+++ b/app/assets/javascripts/diffs/constants.js
@@ -50,3 +50,10 @@ export const LEFT_LINE_KEY = 'left';
 
 export const CENTERED_LIMITED_CONTAINER_CLASSES =
   'container-limited limit-container-width mx-lg-auto px-3';
+
+export const MAX_RENDERING_DIFF_LINES = 500;
+export const MAX_RENDERING_BULK_ROWS = 30;
+export const MIN_RENDERING_MS = 2;
+export const START_RENDERING_INDEX = 200;
+export const INLINE_DIFF_LINES_KEY = 'highlighted_diff_lines';
+export const PARALLEL_DIFF_LINES_KEY = 'parallel_diff_lines';
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index b58ae0d248c..386d08aed2b 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -7,7 +7,12 @@ import { handleLocationHash, historyPushState, scrollToElement } from '~/lib/uti
 import { mergeUrlParams, getLocationHash } from '~/lib/utils/url_utility';
 import TreeWorker from '../workers/tree_worker';
 import eventHub from '../../notes/event_hub';
-import { getDiffPositionByLineCode, getNoteFormData } from './utils';
+import {
+  getDiffPositionByLineCode,
+  getNoteFormData,
+  convertExpandLines,
+  idleCallback,
+} from './utils';
 import * as types from './mutation_types';
 import {
   PARALLEL_DIFF_VIEW_TYPE,
@@ -17,6 +22,16 @@ import {
   TREE_LIST_STORAGE_KEY,
   WHITESPACE_STORAGE_KEY,
   TREE_LIST_WIDTH_STORAGE_KEY,
+  OLD_LINE_KEY,
+  NEW_LINE_KEY,
+  TYPE_KEY,
+  LEFT_LINE_KEY,
+  MAX_RENDERING_DIFF_LINES,
+  MAX_RENDERING_BULK_ROWS,
+  MIN_RENDERING_MS,
+  START_RENDERING_INDEX,
+  INLINE_DIFF_LINES_KEY,
+  PARALLEL_DIFF_LINES_KEY,
 } from '../constants';
 import { diffViewerModes } from '~/ide/constants';
 
@@ -313,13 +328,98 @@ export const cacheTreeListWidth = (_, size) => {
 };
 
 export const requestFullDiff = ({ commit }, filePath) => commit(types.REQUEST_FULL_DIFF, filePath);
-export const receiveFullDiffSucess = ({ commit }, { filePath, data }) =>
-  commit(types.RECEIVE_FULL_DIFF_SUCCESS, { filePath, data });
+export const receiveFullDiffSucess = ({ commit }, { filePath }) =>
+  commit(types.RECEIVE_FULL_DIFF_SUCCESS, { filePath });
 export const receiveFullDiffError = ({ commit }, filePath) => {
   commit(types.RECEIVE_FULL_DIFF_ERROR, filePath);
   createFlash(s__('MergeRequest|Error loading full diff. Please try again.'));
 };
 
+export const setExpandedDiffLines = ({ commit, state }, { file, data }) => {
+  const expandedDiffLines = {
+    highlighted_diff_lines: convertExpandLines({
+      diffLines: file.highlighted_diff_lines,
+      typeKey: TYPE_KEY,
+      oldLineKey: OLD_LINE_KEY,
+      newLineKey: NEW_LINE_KEY,
+      data,
+      mapLine: ({ line, oldLine, newLine }) =>
+        Object.assign(line, {
+          old_line: oldLine,
+          new_line: newLine,
+          line_code: `${file.file_hash}_${oldLine}_${newLine}`,
+        }),
+    }),
+    parallel_diff_lines: convertExpandLines({
+      diffLines: file.parallel_diff_lines,
+      typeKey: [LEFT_LINE_KEY, TYPE_KEY],
+      oldLineKey: [LEFT_LINE_KEY, OLD_LINE_KEY],
+      newLineKey: [LEFT_LINE_KEY, NEW_LINE_KEY],
+      data,
+      mapLine: ({ line, oldLine, newLine }) => ({
+        left: {
+          ...line,
+          old_line: oldLine,
+          line_code: `${file.file_hash}_${oldLine}_${newLine}`,
+        },
+        right: {
+          ...line,
+          new_line: newLine,
+          line_code: `${file.file_hash}_${newLine}_${oldLine}`,
+        },
+      }),
+    }),
+  };
+  const currentDiffLinesKey =
+    state.diffViewType === INLINE_DIFF_VIEW_TYPE ? INLINE_DIFF_LINES_KEY : PARALLEL_DIFF_LINES_KEY;
+  const hiddenDiffLinesKey =
+    state.diffViewType === INLINE_DIFF_VIEW_TYPE ? PARALLEL_DIFF_LINES_KEY : INLINE_DIFF_LINES_KEY;
+
+  commit(types.SET_HIDDEN_VIEW_DIFF_FILE_LINES, {
+    filePath: file.file_path,
+    lines: expandedDiffLines[hiddenDiffLinesKey],
+  });
+
+  if (expandedDiffLines[currentDiffLinesKey].length > MAX_RENDERING_DIFF_LINES) {
+    let index = START_RENDERING_INDEX;
+    commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, {
+      filePath: file.file_path,
+      lines: expandedDiffLines[currentDiffLinesKey].slice(0, index),
+    });
+    commit(types.TOGGLE_DIFF_FILE_RENDERING_MORE, file.file_path);
+
+    const idleCb = t => {
+      const startIndex = index;
+
+      while (
+        t.timeRemaining() >= MIN_RENDERING_MS &&
+        index !== expandedDiffLines[currentDiffLinesKey].length &&
+        index - startIndex !== MAX_RENDERING_BULK_ROWS
+      ) {
+        const line = expandedDiffLines[currentDiffLinesKey][index];
+
+        if (line) {
+          commit(types.ADD_CURRENT_VIEW_DIFF_FILE_LINES, { filePath: file.file_path, line });
+          index += 1;
+        }
+      }
+
+      if (index !== expandedDiffLines[currentDiffLinesKey].length) {
+        idleCallback(idleCb);
+      } else {
+        commit(types.TOGGLE_DIFF_FILE_RENDERING_MORE, file.file_path);
+      }
+    };
+
+    idleCallback(idleCb);
+  } else {
+    commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, {
+      filePath: file.file_path,
+      lines: expandedDiffLines[currentDiffLinesKey],
+    });
+  }
+};
+
 export const fetchFullDiff = ({ dispatch }, file) =>
   axios
     .get(file.context_lines_path, {
@@ -328,8 +428,10 @@ export const fetchFullDiff = ({ dispatch }, file) =>
         from_merge_request: true,
       },
     })
-    .then(({ data }) => dispatch('receiveFullDiffSucess', { filePath: file.file_path, data }))
-    .then(() => scrollToElement(`#${file.file_hash}`))
+    .then(({ data }) => {
+      dispatch('receiveFullDiffSucess', { filePath: file.file_path });
+      dispatch('setExpandedDiffLines', { file, data });
+    })
     .catch(() => dispatch('receiveFullDiffError', file.file_path));
 
 export const toggleFullDiff = ({ dispatch, getters, state }, filePath) => {
@@ -340,7 +442,6 @@ export const toggleFullDiff = ({ dispatch, getters, state }, filePath) => {
   if (file.isShowingFullFile) {
     dispatch('loadCollapsedDiff', file)
       .then(() => dispatch('assignDiscussionsToDiff', getters.getDiffFileDiscussions(file)))
-      .then(() => scrollToElement(`#${file.file_hash}`))
       .catch(() => dispatch('receiveFullDiffError', filePath));
   } else {
     dispatch('fetchFullDiff', file);
diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js
index adf56eba3f8..6bb24c97139 100644
--- a/app/assets/javascripts/diffs/store/mutation_types.js
+++ b/app/assets/javascripts/diffs/store/mutation_types.js
@@ -28,3 +28,8 @@ export const REQUEST_FULL_DIFF = 'REQUEST_FULL_DIFF';
 export const RECEIVE_FULL_DIFF_SUCCESS = 'RECEIVE_FULL_DIFF_SUCCESS';
 export const RECEIVE_FULL_DIFF_ERROR = 'RECEIVE_FULL_DIFF_ERROR';
 export const SET_FILE_COLLAPSED = 'SET_FILE_COLLAPSED';
+
+export const SET_HIDDEN_VIEW_DIFF_FILE_LINES = 'SET_HIDDEN_VIEW_DIFF_FILE_LINES';
+export const SET_CURRENT_VIEW_DIFF_FILE_LINES = 'SET_CURRENT_VIEW_DIFF_FILE_LINES';
+export const ADD_CURRENT_VIEW_DIFF_FILE_LINES = 'ADD_CURRENT_VIEW_DIFF_FILE_LINES';
+export const TOGGLE_DIFF_FILE_RENDERING_MORE = 'TOGGLE_DIFF_FILE_RENDERING_MORE';
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 572fbfb5be4..67bc1724738 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -6,10 +6,8 @@ import {
   addContextLines,
   prepareDiffData,
   isDiscussionApplicableToLine,
-  convertExpandLines,
 } from './utils';
 import * as types from './mutation_types';
-import { OLD_LINE_KEY, NEW_LINE_KEY, TYPE_KEY, LEFT_LINE_KEY } from '../constants';
 
 export default {
   [types.SET_BASE_CONFIG](state, options) {
@@ -265,45 +263,11 @@ export default {
 
     file.isLoadingFullFile = false;
   },
-  [types.RECEIVE_FULL_DIFF_SUCCESS](state, { filePath, data }) {
+  [types.RECEIVE_FULL_DIFF_SUCCESS](state, { filePath }) {
     const file = findDiffFile(state.diffFiles, filePath, 'file_path');
 
     file.isShowingFullFile = true;
     file.isLoadingFullFile = false;
-
-    file.highlighted_diff_lines = convertExpandLines({
-      diffLines: file.highlighted_diff_lines,
-      typeKey: [TYPE_KEY],
-      oldLineKey: [OLD_LINE_KEY],
-      newLineKey: [NEW_LINE_KEY],
-      data,
-      mapLine: ({ line, oldLine, newLine }) => ({
-        ...line,
-        old_line: oldLine,
-        new_line: newLine,
-        line_code: `${file.file_hash}_${oldLine}_${newLine}`,
-      }),
-    });
-
-    file.parallel_diff_lines = convertExpandLines({
-      diffLines: file.parallel_diff_lines,
-      typeKey: [LEFT_LINE_KEY, TYPE_KEY],
-      oldLineKey: [LEFT_LINE_KEY, OLD_LINE_KEY],
-      newLineKey: [LEFT_LINE_KEY, NEW_LINE_KEY],
-      data,
-      mapLine: ({ line, oldLine, newLine }) => ({
-        left: {
-          ...line,
-          old_line: oldLine,
-          line_code: `${file.file_hash}_${oldLine}_${newLine}`,
-        },
-        right: {
-          ...line,
-          new_line: newLine,
-          line_code: `${file.file_hash}_${newLine}_${oldLine}`,
-        },
-      }),
-    });
   },
   [types.SET_FILE_COLLAPSED](state, { filePath, collapsed }) {
     const file = state.diffFiles.find(f => f.file_path === filePath);
@@ -312,4 +276,30 @@ export default {
       file.viewer.collapsed = collapsed;
     }
   },
+  [types.SET_HIDDEN_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) {
+    const file = state.diffFiles.find(f => f.file_path === filePath);
+    const hiddenDiffLinesKey =
+      state.diffViewType === 'inline' ? 'parallel_diff_lines' : 'highlighted_diff_lines';
+
+    file[hiddenDiffLinesKey] = lines;
+  },
+  [types.SET_CURRENT_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) {
+    const file = state.diffFiles.find(f => f.file_path === filePath);
+    const currentDiffLinesKey =
+      state.diffViewType === 'inline' ? 'highlighted_diff_lines' : 'parallel_diff_lines';
+
+    file[currentDiffLinesKey] = lines;
+  },
+  [types.ADD_CURRENT_VIEW_DIFF_FILE_LINES](state, { filePath, line }) {
+    const file = state.diffFiles.find(f => f.file_path === filePath);
+    const currentDiffLinesKey =
+      state.diffViewType === 'inline' ? 'highlighted_diff_lines' : 'parallel_diff_lines';
+
+    file[currentDiffLinesKey].push(line);
+  },
+  [types.TOGGLE_DIFF_FILE_RENDERING_MORE](state, filePath) {
+    const file = state.diffFiles.find(f => f.file_path === filePath);
+
+    file.renderingLines = !file.renderingLines;
+  },
 };
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index 27a79369a24..71956255eef 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -253,6 +253,7 @@ export function prepareDiffData(diffData) {
       isShowingFullFile: false,
       isLoadingFullFile: false,
       discussions: [],
+      renderingLines: false,
     });
   }
 }
@@ -423,27 +424,33 @@ export const convertExpandLines = ({
   mapLine,
 }) => {
   const dataLength = data.length;
+  const lines = [];
+
+  for (let i = 0, diffLinesLength = diffLines.length; i < diffLinesLength; i += 1) {
+    const line = diffLines[i];
 
-  return diffLines.reduce((acc, line, i) => {
     if (_.property(typeKey)(line) === 'match') {
       const beforeLine = diffLines[i - 1];
       const afterLine = diffLines[i + 1];
-      const beforeLineIndex = _.property(newLineKey)(beforeLine) || 0;
-      const afterLineIndex = _.property(newLineKey)(afterLine) - 1 || dataLength;
-
-      acc.push(
-        ...data.slice(beforeLineIndex, afterLineIndex).map((l, index) => ({
-          ...mapLine({
-            line: { ...l, hasForm: false, discussions: [] },
+      const newLineProperty = _.property(newLineKey);
+      const beforeLineIndex = newLineProperty(beforeLine) || 0;
+      const afterLineIndex = newLineProperty(afterLine) - 1 || dataLength;
+
+      lines.push(
+        ...data.slice(beforeLineIndex, afterLineIndex).map((l, index) =>
+          mapLine({
+            line: Object.assign(l, { hasForm: false, discussions: [] }),
             oldLine: (_.property(oldLineKey)(beforeLine) || 0) + index + 1,
-            newLine: (_.property(newLineKey)(beforeLine) || 0) + index + 1,
+            newLine: (newLineProperty(beforeLine) || 0) + index + 1,
           }),
-        })),
+        ),
       );
     } else {
-      acc.push(line);
+      lines.push(line);
     }
+  }
 
-    return acc;
-  }, []);
+  return lines;
 };
+
+export const idleCallback = cb => requestIdleCallback(cb);