diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index a800cc8edc8131edeebcff1c82e1ff17875fe98f..9d8d184a3f614b86e23ff61b6a6327c7c4590aaa 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -20,6 +20,8 @@ import HiddenFilesWarning from './hidden_files_warning.vue';
import MergeConflictWarning from './merge_conflict_warning.vue';
import CollapsedFilesWarning from './collapsed_files_warning.vue';
+import { diffsApp } from '../utils/performance';
+
import {
TREE_LIST_WIDTH_STORAGE_KEY,
INITIAL_TREE_WIDTH,
@@ -272,8 +274,12 @@ export default {
);
}
},
+ beforeCreate() {
+ diffsApp.instrument();
+ },
created() {
this.adjustView();
+
eventHub.$once('fetchDiffData', this.fetchData);
eventHub.$on('refetchDiffData', this.refetchDiffData);
this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES;
@@ -294,6 +300,8 @@ export default {
);
},
beforeDestroy() {
+ diffsApp.deinstrument();
+
eventHub.$off('fetchDiffData', this.fetchData);
eventHub.$off('refetchDiffData', this.refetchDiffData);
this.removeEventListeners();
@@ -487,9 +495,11 @@ export default {
{
+ eventHub.$emit(event);
+ });
+ },
handleToggle() {
const currentCollapsedFlag = this.isCollapsed;
@@ -197,7 +231,8 @@ export default {
})
.then(() => {
requestIdleCallback(
- () => {
+ async () => {
+ await this.postRender();
this.assignDiscussionsToDiff(this.getDiffFileDiscussions(this.file));
},
{ timeout: 1000 },
diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js
index 709bfe693e63b03dfd7783cd3a9928046e1272ca..79f8c08e389d01e1689aac07052789e357914ea7 100644
--- a/app/assets/javascripts/diffs/constants.js
+++ b/app/assets/javascripts/diffs/constants.js
@@ -98,3 +98,8 @@ export const RENAMED_DIFF_TRANSITIONS = {
// MR Diffs known events
export const EVT_EXPAND_ALL_FILES = 'mr:diffs:expandAllFiles';
+export const EVT_PERF_MARK_FILE_TREE_START = 'mr:diffs:perf:fileTreeStart';
+export const EVT_PERF_MARK_FILE_TREE_END = 'mr:diffs:perf:fileTreeEnd';
+export const EVT_PERF_MARK_DIFF_FILES_START = 'mr:diffs:perf:filesStart';
+export const EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN = 'mr:diffs:perf:firstFileShown';
+export const EVT_PERF_MARK_DIFF_FILES_END = 'mr:diffs:perf:filesEnd';
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index 5a3d836a1584edf33ab398b853f8eaf40a915289..72b99ca84865fc788eb75f6676d4430efabbc9dd 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -8,7 +8,8 @@ import { __, s__ } from '~/locale';
import { handleLocationHash, historyPushState, scrollToElement } from '~/lib/utils/common_utils';
import { mergeUrlParams, getLocationHash } from '~/lib/utils/url_utility';
import TreeWorker from '../workers/tree_worker';
-import eventHub from '../../notes/event_hub';
+import notesEventHub from '../../notes/event_hub';
+import eventHub from '../event_hub';
import {
getDiffPositionByLineCode,
getNoteFormData,
@@ -42,6 +43,9 @@ import {
NO_SHOW_WHITESPACE,
DIFF_FILE_MANUAL_COLLAPSE,
DIFF_FILE_AUTOMATIC_COLLAPSE,
+ EVT_PERF_MARK_FILE_TREE_START,
+ EVT_PERF_MARK_FILE_TREE_END,
+ EVT_PERF_MARK_DIFF_FILES_START,
} from '../constants';
import { diffViewerModes } from '~/ide/constants';
import { isCollapsed } from '../diff_file';
@@ -78,6 +82,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
commit(types.SET_BATCH_LOADING, true);
commit(types.SET_RETRIEVING_BATCHES, true);
+ eventHub.$emit(EVT_PERF_MARK_DIFF_FILES_START);
const getBatch = (page = 1) =>
axios
@@ -139,9 +144,11 @@ export const fetchDiffFilesMeta = ({ commit, state }) => {
};
commit(types.SET_LOADING, true);
+ eventHub.$emit(EVT_PERF_MARK_FILE_TREE_START);
worker.addEventListener('message', ({ data }) => {
commit(types.SET_TREE_DATA, data);
+ eventHub.$emit(EVT_PERF_MARK_FILE_TREE_END);
worker.terminate();
});
@@ -215,7 +222,7 @@ export const assignDiscussionsToDiff = (
}
Vue.nextTick(() => {
- eventHub.$emit('scrollToDiscussion');
+ notesEventHub.$emit('scrollToDiscussion');
});
};
@@ -240,7 +247,7 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi
}
if (file.viewer.automaticallyCollapsed) {
- eventHub.$emit(`loadCollapsedDiff/${file.file_hash}`);
+ notesEventHub.$emit(`loadCollapsedDiff/${file.file_hash}`);
scrollToElement(document.getElementById(file.file_hash));
} else if (file.viewer.manuallyCollapsed) {
commit(types.SET_FILE_COLLAPSED, {
@@ -248,9 +255,9 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi
collapsed: false,
trigger: DIFF_FILE_AUTOMATIC_COLLAPSE,
});
- eventHub.$emit('scrollToDiscussion');
+ notesEventHub.$emit('scrollToDiscussion');
} else {
- eventHub.$emit('scrollToDiscussion');
+ notesEventHub.$emit('scrollToDiscussion');
}
}
}
@@ -485,7 +492,7 @@ export const setShowWhitespace = ({ commit }, { showWhitespace, pushState = fals
historyPushState(mergeUrlParams({ w }, window.location.href));
}
- eventHub.$emit('refetchDiffData');
+ notesEventHub.$emit('refetchDiffData');
};
export const toggleFileFinder = ({ commit }, visible) => {
diff --git a/app/assets/javascripts/diffs/utils/performance.js b/app/assets/javascripts/diffs/utils/performance.js
new file mode 100644
index 0000000000000000000000000000000000000000..dcde6f4ecc49bdcaa5a4d50da7131f973be3a321
--- /dev/null
+++ b/app/assets/javascripts/diffs/utils/performance.js
@@ -0,0 +1,80 @@
+import { performanceMarkAndMeasure } from '~/performance/utils';
+import {
+ MR_DIFFS_MARK_FILE_TREE_START,
+ MR_DIFFS_MARK_FILE_TREE_END,
+ MR_DIFFS_MARK_DIFF_FILES_START,
+ MR_DIFFS_MARK_FIRST_DIFF_FILE_SHOWN,
+ MR_DIFFS_MARK_DIFF_FILES_END,
+ MR_DIFFS_MEASURE_FILE_TREE_DONE,
+ MR_DIFFS_MEASURE_DIFF_FILES_DONE,
+} from '../../performance/constants';
+
+import eventHub from '../event_hub';
+import {
+ EVT_PERF_MARK_FILE_TREE_START,
+ EVT_PERF_MARK_FILE_TREE_END,
+ EVT_PERF_MARK_DIFF_FILES_START,
+ EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN,
+ EVT_PERF_MARK_DIFF_FILES_END,
+} from '../constants';
+
+function treeStart() {
+ performanceMarkAndMeasure({
+ mark: MR_DIFFS_MARK_FILE_TREE_START,
+ });
+}
+
+function treeEnd() {
+ performanceMarkAndMeasure({
+ mark: MR_DIFFS_MARK_FILE_TREE_END,
+ measures: [
+ {
+ name: MR_DIFFS_MEASURE_FILE_TREE_DONE,
+ start: MR_DIFFS_MARK_FILE_TREE_START,
+ end: MR_DIFFS_MARK_FILE_TREE_END,
+ },
+ ],
+ });
+}
+
+function filesStart() {
+ performanceMarkAndMeasure({
+ mark: MR_DIFFS_MARK_DIFF_FILES_START,
+ });
+}
+
+function filesEnd() {
+ performanceMarkAndMeasure({
+ mark: MR_DIFFS_MARK_DIFF_FILES_END,
+ measures: [
+ {
+ name: MR_DIFFS_MEASURE_DIFF_FILES_DONE,
+ start: MR_DIFFS_MARK_DIFF_FILES_START,
+ end: MR_DIFFS_MARK_DIFF_FILES_END,
+ },
+ ],
+ });
+}
+
+function firstFile() {
+ performanceMarkAndMeasure({
+ mark: MR_DIFFS_MARK_FIRST_DIFF_FILE_SHOWN,
+ });
+}
+
+export const diffsApp = {
+ instrument() {
+ eventHub.$on(EVT_PERF_MARK_FILE_TREE_START, treeStart);
+ eventHub.$on(EVT_PERF_MARK_FILE_TREE_END, treeEnd);
+ eventHub.$on(EVT_PERF_MARK_DIFF_FILES_START, filesStart);
+ eventHub.$on(EVT_PERF_MARK_DIFF_FILES_END, filesEnd);
+ eventHub.$on(EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN, firstFile);
+ },
+ deinstrument() {
+ eventHub.$off(EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN, firstFile);
+ eventHub.$off(EVT_PERF_MARK_DIFF_FILES_END, filesEnd);
+ eventHub.$off(EVT_PERF_MARK_DIFF_FILES_START, filesStart);
+ eventHub.$off(EVT_PERF_MARK_FILE_TREE_END, treeEnd);
+ eventHub.$off(EVT_PERF_MARK_FILE_TREE_START, treeStart);
+ },
+};
diff --git a/app/assets/javascripts/performance/constants.js b/app/assets/javascripts/performance/constants.js
index 6b6b6f1da40f4f296e4b8f5bda2c8862ba2bafc3..816eb9b3a66cf9f81627454fd7e0fb10621ef1f4 100644
--- a/app/assets/javascripts/performance/constants.js
+++ b/app/assets/javascripts/performance/constants.js
@@ -29,3 +29,17 @@ export const WEBIDE_MARK_FILE_FINISH = 'webide-file-finished';
export const WEBIDE_MEASURE_TREE_FROM_REQUEST = 'webide-tree-loading-from-request';
export const WEBIDE_MEASURE_FILE_FROM_REQUEST = 'webide-file-loading-from-request';
export const WEBIDE_MEASURE_FILE_AFTER_INTERACTION = 'webide-file-loading-after-interaction';
+
+//
+// MR Diffs namespace
+
+// Marks
+export const MR_DIFFS_MARK_FILE_TREE_START = 'mr-diffs-mark-file-tree-start';
+export const MR_DIFFS_MARK_FILE_TREE_END = 'mr-diffs-mark-file-tree-end';
+export const MR_DIFFS_MARK_DIFF_FILES_START = 'mr-diffs-mark-diff-files-start';
+export const MR_DIFFS_MARK_FIRST_DIFF_FILE_SHOWN = 'mr-diffs-mark-first-diff-file-shown';
+export const MR_DIFFS_MARK_DIFF_FILES_END = 'mr-diffs-mark-diff-files-end';
+
+// Measures
+export const MR_DIFFS_MEASURE_FILE_TREE_DONE = 'mr-diffs-measure-file-tree-done';
+export const MR_DIFFS_MEASURE_DIFF_FILES_DONE = 'mr-diffs-measure-diff-files-done';
diff --git a/changelogs/unreleased/feature-mr-diffs-performance-marks.yml b/changelogs/unreleased/feature-mr-diffs-performance-marks.yml
new file mode 100644
index 0000000000000000000000000000000000000000..914991b2a91c49d98ba426bf8555810786d6b7c6
--- /dev/null
+++ b/changelogs/unreleased/feature-mr-diffs-performance-marks.yml
@@ -0,0 +1,5 @@
+---
+title: Add performance marks and measures to the MR Diffs app at critical moments
+merge_request: 46434
+author:
+type: other
diff --git a/spec/frontend/diffs/components/diff_file_spec.js b/spec/frontend/diffs/components/diff_file_spec.js
index bd21252eb5a3845e5283d1cfd46dced257c7e48e..71e0ffd176f46fdafd7b963005fbd78d92676db9 100644
--- a/spec/frontend/diffs/components/diff_file_spec.js
+++ b/spec/frontend/diffs/components/diff_file_spec.js
@@ -2,6 +2,7 @@ import Vuex from 'vuex';
import { shallowMount, createLocalVue } from '@vue/test-utils';
import createDiffsStore from '~/diffs/store/modules';
+import createNotesStore from '~/notes/stores/modules';
import diffFileMockDataReadable from '../mock_data/diff_file';
import diffFileMockDataUnreadable from '../mock_data/diff_file_unreadable';
@@ -10,9 +11,13 @@ import DiffFileHeaderComponent from '~/diffs/components/diff_file_header.vue';
import DiffContentComponent from '~/diffs/components/diff_content.vue';
import eventHub from '~/diffs/event_hub';
+import {
+ EVT_EXPAND_ALL_FILES,
+ EVT_PERF_MARK_DIFF_FILES_END,
+ EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN,
+} from '~/diffs/constants';
import { diffViewerModes, diffViewerErrors } from '~/ide/constants';
-import { EVT_EXPAND_ALL_FILES } from '~/diffs/constants';
function changeViewer(store, index, { automaticallyCollapsed, manuallyCollapsed, name }) {
const file = store.state.diffs.diffFiles[index];
@@ -58,12 +63,13 @@ function markFileToBeRendered(store, index = 0) {
});
}
-function createComponent({ file }) {
+function createComponent({ file, first = false, last = false }) {
const localVue = createLocalVue();
localVue.use(Vuex);
const store = new Vuex.Store({
+ ...createNotesStore(),
modules: {
diffs: createDiffsStore(),
},
@@ -78,6 +84,8 @@ function createComponent({ file }) {
file,
canCurrentUserFork: false,
viewDiffsFileByFile: false,
+ isFirstFile: first,
+ isLastFile: last,
},
});
@@ -117,6 +125,72 @@ describe('DiffFile', () => {
afterEach(() => {
wrapper.destroy();
+ wrapper = null;
+ });
+
+ describe('bus events', () => {
+ beforeEach(() => {
+ jest.spyOn(eventHub, '$emit').mockImplementation(() => {});
+ });
+
+ describe('during mount', () => {
+ it.each`
+ first | last | events | file
+ ${false} | ${false} | ${[]} | ${{ inlineLines: [], parallelLines: [], readableText: true }}
+ ${true} | ${true} | ${[]} | ${{ inlineLines: [], parallelLines: [], readableText: true }}
+ ${true} | ${false} | ${[EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN]} | ${false}
+ ${false} | ${true} | ${[EVT_PERF_MARK_DIFF_FILES_END]} | ${false}
+ ${true} | ${true} | ${[EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN, EVT_PERF_MARK_DIFF_FILES_END]} | ${false}
+ `(
+ 'emits the events $events based on the file and its position ({ first: $first, last: $last }) among all files',
+ async ({ file, first, last, events }) => {
+ if (file) {
+ forceHasDiff({ store, ...file });
+ }
+
+ ({ wrapper, store } = createComponent({
+ file: store.state.diffs.diffFiles[0],
+ first,
+ last,
+ }));
+
+ await wrapper.vm.$nextTick();
+
+ expect(eventHub.$emit).toHaveBeenCalledTimes(events.length);
+ events.forEach(event => {
+ expect(eventHub.$emit).toHaveBeenCalledWith(event);
+ });
+ },
+ );
+ });
+
+ describe('after loading the diff', () => {
+ it('indicates that it loaded the file', async () => {
+ forceHasDiff({ store, inlineLines: [], parallelLines: [], readableText: true });
+ ({ wrapper, store } = createComponent({
+ file: store.state.diffs.diffFiles[0],
+ first: true,
+ last: true,
+ }));
+
+ jest.spyOn(wrapper.vm, 'loadCollapsedDiff').mockResolvedValue(getReadableFile());
+ jest.spyOn(window, 'requestIdleCallback').mockImplementation(fn => fn());
+
+ makeFileAutomaticallyCollapsed(store);
+
+ await wrapper.vm.$nextTick(); // Wait for store updates to flow into the component
+
+ toggleFile(wrapper);
+
+ await wrapper.vm.$nextTick(); // Wait for the load to resolve
+ await wrapper.vm.$nextTick(); // Wait for the idleCallback
+ await wrapper.vm.$nextTick(); // Wait for nextTick inside postRender
+
+ expect(eventHub.$emit).toHaveBeenCalledTimes(2);
+ expect(eventHub.$emit).toHaveBeenCalledWith(EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN);
+ expect(eventHub.$emit).toHaveBeenCalledWith(EVT_PERF_MARK_DIFF_FILES_END);
+ });
+ });
});
describe('template', () => {