Commit c49d4a49 authored by Paul Slaughter's avatar Paul Slaughter Committed by Phil Hughes

Improve files_decorator performance

**How?**
Previously the files_decorator inserted parent folders inefficiently.
It started at the first part and ensured each was inserted.

Since the file entries are given to use in alphabetical order, we can
start at the last part of the blob's parents. If this exists, we can
short circuit and be done inserting parents.

**What else?**
- Improve performance of decorateData. The object spread operator is
not needed because the object is brand new.
parent 80fea82f
import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils';
import { decorateData, sortTree } from '../stores/utils';
export const splitParent = path => {
const idx = path.lastIndexOf('/');
return {
parent: idx >= 0 ? path.substring(0, idx) : null,
name: idx >= 0 ? path.substring(idx + 1) : path,
};
};
/**
* Create file objects from a list of file paths.
*/
export const decorateFiles = ({
data,
projectId,
branchId,
tempFile = false,
content = '',
base64 = false,
}) => {
const treeList = [];
const entries = {};
// These mutable variable references end up being exported and used by `createTempEntry`
let file;
let parentPath;
const insertParent = path => {
if (!path) {
return null;
} else if (entries[path]) {
return entries[path];
}
const { parent, name } = splitParent(path);
const parentFolder = parent && insertParent(parent);
parentPath = parentFolder && parentFolder.path;
const tree = decorateData({
projectId,
branchId,
id: path,
name,
path,
url: `/${projectId}/tree/${branchId}/-/${path}/`,
type: 'tree',
parentTreeUrl: parentFolder ? parentFolder.url : `/${projectId}/tree/${branchId}/`,
tempFile,
changed: tempFile,
opened: tempFile,
parentPath,
});
Object.assign(entries, {
[path]: tree,
});
if (parentFolder) {
parentFolder.tree.push(tree);
} else {
treeList.push(tree);
}
return tree;
};
data.forEach(path => {
const { parent, name } = splitParent(path);
const fileFolder = parent && insertParent(parent);
if (name) {
parentPath = fileFolder && fileFolder.path;
file = decorateData({
projectId,
branchId,
id: path,
name,
path,
url: `/${projectId}/blob/${branchId}/-/${path}`,
type: 'blob',
parentTreeUrl: fileFolder ? fileFolder.url : `/${projectId}/blob/${branchId}`,
tempFile,
changed: tempFile,
content,
base64,
previewMode: viewerInformationForPath(name),
parentPath,
});
Object.assign(entries, {
[path]: file,
});
if (fileFolder) {
fileFolder.tree.push(file);
} else {
treeList.push(file);
}
}
});
return {
entries,
treeList: sortTree(treeList),
file,
parentPath,
};
};
...@@ -3,7 +3,7 @@ import Vue from 'vue'; ...@@ -3,7 +3,7 @@ import Vue from 'vue';
import { visitUrl } from '~/lib/utils/url_utility'; import { visitUrl } from '~/lib/utils/url_utility';
import flash from '~/flash'; import flash from '~/flash';
import * as types from './mutation_types'; import * as types from './mutation_types';
import FilesDecoratorWorker from './workers/files_decorator_worker'; import { decorateFiles } from '../lib/files';
import { stageKeys } from '../constants'; import { stageKeys } from '../constants';
export const redirectToUrl = (_, url) => visitUrl(url); export const redirectToUrl = (_, url) => visitUrl(url);
...@@ -56,7 +56,6 @@ export const createTempEntry = ( ...@@ -56,7 +56,6 @@ export const createTempEntry = (
{ name, type, content = '', base64 = false }, { name, type, content = '', base64 = false },
) => ) =>
new Promise(resolve => { new Promise(resolve => {
const worker = new FilesDecoratorWorker();
const fullName = name.slice(-1) !== '/' && type === 'tree' ? `${name}/` : name; const fullName = name.slice(-1) !== '/' && type === 'tree' ? `${name}/` : name;
if (state.entries[name]) { if (state.entries[name]) {
...@@ -74,11 +73,17 @@ export const createTempEntry = ( ...@@ -74,11 +73,17 @@ export const createTempEntry = (
return null; return null;
} }
worker.addEventListener('message', ({ data }) => { const data = decorateFiles({
data: [fullName],
projectId: state.currentProjectId,
branchId: state.currentBranchId,
type,
tempFile: true,
base64,
content,
});
const { file, parentPath } = data; const { file, parentPath } = data;
worker.terminate();
commit(types.CREATE_TMP_ENTRY, { commit(types.CREATE_TMP_ENTRY, {
data, data,
projectId: state.currentProjectId, projectId: state.currentProjectId,
...@@ -96,17 +101,6 @@ export const createTempEntry = ( ...@@ -96,17 +101,6 @@ export const createTempEntry = (
} }
resolve(file); resolve(file);
});
worker.postMessage({
data: [fullName],
projectId: state.currentProjectId,
branchId: state.currentBranchId,
type,
tempFile: true,
base64,
content,
});
return null; return null;
}); });
......
import _ from 'underscore';
import { __ } from '../../../locale'; import { __ } from '../../../locale';
import service from '../../services'; import service from '../../services';
import * as types from '../mutation_types'; import * as types from '../mutation_types';
import FilesDecoratorWorker from '../workers/files_decorator_worker'; import { decorateFiles } from '../../lib/files';
export const toggleTreeOpen = ({ commit }, path) => { export const toggleTreeOpen = ({ commit }, path) => {
commit(types.TOGGLE_TREE_OPEN, path); commit(types.TOGGLE_TREE_OPEN, path);
...@@ -32,6 +33,19 @@ export const handleTreeEntryAction = ({ commit, dispatch }, row) => { ...@@ -32,6 +33,19 @@ export const handleTreeEntryAction = ({ commit, dispatch }, row) => {
dispatch('showTreeEntry', row.path); dispatch('showTreeEntry', row.path);
}; };
export const setDirectoryData = ({ state, commit }, { projectId, branchId, treeList }) => {
const selectedTree = state.trees[`${projectId}/${branchId}`];
commit(types.SET_DIRECTORY_DATA, {
treePath: `${projectId}/${branchId}`,
data: treeList,
});
commit(types.TOGGLE_LOADING, {
entry: selectedTree,
forceValue: false,
});
};
export const getFiles = ({ state, commit, dispatch }, { projectId, branchId } = {}) => export const getFiles = ({ state, commit, dispatch }, { projectId, branchId } = {}) =>
new Promise((resolve, reject) => { new Promise((resolve, reject) => {
if ( if (
...@@ -45,31 +59,19 @@ export const getFiles = ({ state, commit, dispatch }, { projectId, branchId } = ...@@ -45,31 +59,19 @@ export const getFiles = ({ state, commit, dispatch }, { projectId, branchId } =
service service
.getFiles(selectedProject.web_url, branchId) .getFiles(selectedProject.web_url, branchId)
.then(({ data }) => { .then(({ data }) => {
const worker = new FilesDecoratorWorker(); const { entries, treeList } = decorateFiles({
worker.addEventListener('message', e => { data,
const { entries, treeList } = e.data; projectId,
const selectedTree = state.trees[`${projectId}/${branchId}`]; branchId,
});
commit(types.SET_ENTRIES, entries); commit(types.SET_ENTRIES, entries);
commit(types.SET_DIRECTORY_DATA, {
treePath: `${projectId}/${branchId}`,
data: treeList,
});
commit(types.TOGGLE_LOADING, {
entry: selectedTree,
forceValue: false,
});
worker.terminate(); // Defer setting the directory data because this triggers some intense rendering.
// The entries is all we need to load the file editor.
_.defer(() => dispatch('setDirectoryData', { projectId, branchId, treeList }));
resolve(); resolve();
});
worker.postMessage({
data,
projectId,
branchId,
});
}) })
.catch(e => { .catch(e => {
if (e.response.status === 404) { if (e.response.status === 404) {
......
...@@ -75,8 +75,7 @@ export const decorateData = entity => { ...@@ -75,8 +75,7 @@ export const decorateData = entity => {
parentPath = '', parentPath = '',
} = entity; } = entity;
return { return Object.assign(dataStructure(), {
...dataStructure(),
id, id,
projectId, projectId,
branchId, branchId,
...@@ -97,7 +96,7 @@ export const decorateData = entity => { ...@@ -97,7 +96,7 @@ export const decorateData = entity => {
file_lock, file_lock,
html, html,
parentPath, parentPath,
}; });
}; };
export const findEntry = (tree, type, name, prop = 'name') => export const findEntry = (tree, type, name, prop = 'name') =>
......
import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils';
import { decorateData, sortTree } from '../utils';
// eslint-disable-next-line no-restricted-globals
self.addEventListener('message', e => {
const { data, projectId, branchId, tempFile = false, content = '', base64 = false } = e.data;
const treeList = [];
let file;
let parentPath;
const entries = data.reduce((acc, path) => {
const pathSplit = path.split('/');
const blobName = pathSplit.pop().trim();
if (pathSplit.length > 0) {
pathSplit.reduce((pathAcc, folderName) => {
const parentFolder = acc[pathAcc[pathAcc.length - 1]];
const folderPath = `${parentFolder ? `${parentFolder.path}/` : ''}${folderName}`;
const foundEntry = acc[folderPath];
if (!foundEntry) {
parentPath = parentFolder ? parentFolder.path : null;
const tree = decorateData({
projectId,
branchId,
id: folderPath,
name: folderName,
path: folderPath,
url: `/${projectId}/tree/${branchId}/-/${folderPath}/`,
type: 'tree',
parentTreeUrl: parentFolder ? parentFolder.url : `/${projectId}/tree/${branchId}/`,
tempFile,
changed: tempFile,
opened: tempFile,
parentPath,
});
Object.assign(acc, {
[folderPath]: tree,
});
if (parentFolder) {
parentFolder.tree.push(tree);
} else {
treeList.push(tree);
}
pathAcc.push(tree.path);
} else {
pathAcc.push(foundEntry.path);
}
return pathAcc;
}, []);
}
if (blobName !== '') {
const fileFolder = acc[pathSplit.join('/')];
parentPath = fileFolder ? fileFolder.path : null;
file = decorateData({
projectId,
branchId,
id: path,
name: blobName,
path,
url: `/${projectId}/blob/${branchId}/-/${path}`,
type: 'blob',
parentTreeUrl: fileFolder ? fileFolder.url : `/${projectId}/blob/${branchId}`,
tempFile,
changed: tempFile,
content,
base64,
previewMode: viewerInformationForPath(blobName),
parentPath,
});
Object.assign(acc, {
[path]: file,
});
if (fileFolder) {
fileFolder.tree.push(file);
} else {
treeList.push(file);
}
}
return acc;
}, {});
// eslint-disable-next-line no-restricted-globals
self.postMessage({
entries,
treeList: sortTree(treeList),
file,
parentPath,
});
});
---
title: Improve Web IDE launch performance
merge_request: 25700
author:
type: performance
import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils';
import { decorateFiles, splitParent } from '~/ide/lib/files';
import { decorateData } from '~/ide/stores/utils';
const TEST_BRANCH_ID = 'lorem-ipsum';
const TEST_PROJECT_ID = 10;
const createEntries = paths => {
const createEntry = (acc, { path, type, children }) => {
// Sometimes we need to end the url with a '/'
const createUrl = base => (type === 'tree' ? `${base}/` : base);
const { name, parent } = splitParent(path);
const parentEntry = acc[parent];
acc[path] = {
...decorateData({
projectId: TEST_PROJECT_ID,
branchId: TEST_BRANCH_ID,
id: path,
name,
path,
url: createUrl(`/${TEST_PROJECT_ID}/${type}/${TEST_BRANCH_ID}/-/${path}`),
type,
previewMode: viewerInformationForPath(path),
parentPath: parent,
parentTreeUrl: parentEntry
? parentEntry.url
: createUrl(`/${TEST_PROJECT_ID}/${type}/${TEST_BRANCH_ID}`),
}),
tree: children.map(childName => jasmine.objectContaining({ name: childName })),
};
return acc;
};
const entries = paths.reduce(createEntry, {});
// Wrap entries in jasmine.objectContaining.
// We couldn't do this earlier because we still need to select properties from parent entries.
return Object.keys(entries).reduce((acc, key) => {
acc[key] = jasmine.objectContaining(entries[key]);
return acc;
}, {});
};
describe('IDE lib decorate files', () => {
it('creates entries and treeList', () => {
const data = ['app/assets/apples/foo.js', 'app/bugs.js', 'README.md'];
const expectedEntries = createEntries([
{ path: 'app', type: 'tree', children: ['assets', 'bugs.js'] },
{ path: 'app/assets', type: 'tree', children: ['apples'] },
{ path: 'app/assets/apples', type: 'tree', children: ['foo.js'] },
{ path: 'app/assets/apples/foo.js', type: 'blob', children: [] },
{ path: 'app/bugs.js', type: 'blob', children: [] },
{ path: 'README.md', type: 'blob', children: [] },
]);
const { entries, treeList } = decorateFiles({
data,
branchId: TEST_BRANCH_ID,
projectId: TEST_PROJECT_ID,
});
// Here we test the keys and then each key/value individually because `expect(entries).toEqual(expectedEntries)`
// was taking a very long time for some reason. Probably due to large objects and nested `jasmine.objectContaining`.
const entryKeys = Object.keys(entries);
expect(entryKeys).toEqual(Object.keys(expectedEntries));
entryKeys.forEach(key => {
expect(entries[key]).toEqual(expectedEntries[key]);
});
expect(treeList).toEqual([expectedEntries.app, expectedEntries['README.md']]);
});
});
...@@ -20,6 +20,7 @@ describe('Multi-file store tree actions', () => { ...@@ -20,6 +20,7 @@ describe('Multi-file store tree actions', () => {
}; };
beforeEach(() => { beforeEach(() => {
jasmine.clock().install();
spyOn(router, 'push'); spyOn(router, 'push');
mock = new MockAdapter(axios); mock = new MockAdapter(axios);
...@@ -37,6 +38,7 @@ describe('Multi-file store tree actions', () => { ...@@ -37,6 +38,7 @@ describe('Multi-file store tree actions', () => {
}); });
afterEach(() => { afterEach(() => {
jasmine.clock().uninstall();
mock.restore(); mock.restore();
resetStore(store); resetStore(store);
}); });
...@@ -69,6 +71,11 @@ describe('Multi-file store tree actions', () => { ...@@ -69,6 +71,11 @@ describe('Multi-file store tree actions', () => {
it('adds data into tree', done => { it('adds data into tree', done => {
store store
.dispatch('getFiles', basicCallParameters) .dispatch('getFiles', basicCallParameters)
.then(() => {
// The populating of the tree is deferred for performance reasons.
// See this merge request for details: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/25700
jasmine.clock().tick(1);
})
.then(() => { .then(() => {
projectTree = store.state.trees['abcproject/master']; projectTree = store.state.trees['abcproject/master'];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment