Skip to content

Commit f161d3f

Browse files
mjbvzbpasero
mjbvz
authored and
bpasero
committed
Optimize parseStorage (microsoft#62805)
* For microsoft#62733 **Problem** `parseStorage` computes a lot of data that we then immediately discard. **Fix** Try to only compute the data that we really will use by splitting `parseStorage` into different implementations per storage type (empty, multiRoot, ...). Also optimizes some of these implementations * Fixing tests * Clean up interface and optimize a bit further - Don't create big maps that we pick one from; Just target the workspace we are interested in directly. * Add logging for verifying that parseStorage extracts the correct values For insiders, we want to test that the changes to parseStorage are doing the right thing. For this, always run parseStorage and then check against migrated results. Log a warning if the data sets do not match * perf - remove check for workspace identifier for empty and workspace storage These keys will never have the workspace identifier. * revert, looks like we now store workspace identifier also for empty and multi-root * perf - make migration of empty, root and no workspace easier * remove verification code and check for keys length * properly migrate empty workspaces (changed from empty: to just the ID unfortunately)
1 parent 15ab767 commit f161d3f

3 files changed

Lines changed: 164 additions & 205 deletions

File tree

‎src/vs/platform/storage/common/storageLegacyMigration.ts‎

Lines changed: 80 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
import { StorageLegacyService, IStorageLegacy } from 'vs/platform/storage/common/storageLegacyService';
77
import { endsWith, startsWith, rtrim } from 'vs/base/common/strings';
8-
import { URI } from 'vs/base/common/uri';
98

109
/**
1110
* We currently store local storage with the following format:
@@ -28,154 +27,130 @@ import { URI } from 'vs/base/common/uri';
2827
* => no longer being used (used for empty workspaces previously)
2928
*/
3029

31-
const EMPTY_WORKSPACE_PREFIX = `${StorageLegacyService.COMMON_PREFIX}workspace/empty:`;
32-
const MULTI_ROOT_WORKSPACE_PREFIX = `${StorageLegacyService.COMMON_PREFIX}workspace/root:`;
30+
const COMMON_WORKSPACE_PREFIX = `${StorageLegacyService.COMMON_PREFIX}workspace/`;
3331
const NO_WORKSPACE_PREFIX = 'storage://workspace/__$noWorkspace__';
3432

3533
export type StorageObject = { [key: string]: string };
3634

3735
export interface IParsedStorage {
38-
global: Map<string, string>;
3936
multiRoot: Map<string, StorageObject>;
4037
folder: Map<string, StorageObject>;
4138
empty: Map<string, StorageObject>;
4239
noWorkspace: StorageObject;
4340
}
4441

45-
/**
46-
* Parses the local storage implementation into global, multi root, folder and empty storage.
47-
*/
48-
export function parseStorage(storage: IStorageLegacy): IParsedStorage {
49-
const globalStorage = new Map<string, string>();
50-
const noWorkspaceStorage: StorageObject = Object.create(null);
51-
const folderWorkspacesStorage = new Map<string /* workspace file resource */, StorageObject>();
52-
const emptyWorkspacesStorage = new Map<string /* empty workspace id */, StorageObject>();
53-
const multiRootWorkspacesStorage = new Map<string /* multi root workspace id */, StorageObject>();
42+
export function parseFolderStorage(storage: IStorageLegacy, folderId: string): StorageObject {
5443

5544
const workspaces: { prefix: string; resource: string; }[] = [];
45+
const activeKeys = new Set<string>();
46+
5647
for (let i = 0; i < storage.length; i++) {
5748
const key = storage.key(i);
5849

5950
// Workspace Storage (storage://workspace/)
60-
if (startsWith(key, StorageLegacyService.WORKSPACE_PREFIX)) {
61-
62-
// No Workspace key is for extension development windows
63-
if (key.indexOf('__$noWorkspace__') > 0) {
64-
65-
// storage://workspace/__$noWorkspace__someKey => someKey
66-
const noWorkspaceStorageKey = key.substr(NO_WORKSPACE_PREFIX.length);
51+
if (!startsWith(key, StorageLegacyService.WORKSPACE_PREFIX)) {
52+
continue;
53+
}
6754

68-
noWorkspaceStorage[noWorkspaceStorageKey] = storage.getItem(key);
69-
}
55+
activeKeys.add(key);
7056

71-
// We are looking for key: storage://workspace/<folder>/workspaceIdentifier to be able to find all folder
72-
// paths that are known to the storage. is the only way how to parse all folder paths known in storage.
73-
else if (endsWith(key, StorageLegacyService.WORKSPACE_IDENTIFIER)) {
57+
// We are looking for key: storage://workspace/<folder>/workspaceIdentifier to be able to find all folder
58+
// paths that are known to the storage. is the only way how to parse all folder paths known in storage.
59+
if (endsWith(key, StorageLegacyService.WORKSPACE_IDENTIFIER)) {
7460

75-
// storage://workspace/<folder>/workspaceIdentifier => <folder>/
76-
let workspace = key.substring(StorageLegacyService.WORKSPACE_PREFIX.length, key.length - StorageLegacyService.WORKSPACE_IDENTIFIER.length);
61+
// storage://workspace/<folder>/workspaceIdentifier => <folder>/
62+
let workspace = key.substring(StorageLegacyService.WORKSPACE_PREFIX.length, key.length - StorageLegacyService.WORKSPACE_IDENTIFIER.length);
7763

78-
// macOS/Unix: Users/name/folder/
79-
// Windows: c%3A/Users/name/folder/
80-
if (!startsWith(workspace, 'file:')) {
81-
workspace = `file:///${rtrim(workspace, '/')}`;
82-
}
64+
// macOS/Unix: Users/name/folder/
65+
// Windows: c%3A/Users/name/folder/
66+
if (!startsWith(workspace, 'file:')) {
67+
workspace = `file:///${rtrim(workspace, '/')}`;
68+
}
8369

84-
// Windows UNC path: file://localhost/c%3A/Users/name/folder/
85-
else {
86-
workspace = rtrim(workspace, '/');
87-
}
70+
// Windows UNC path: file://localhost/c%3A/Users/name/folder/
71+
else {
72+
workspace = rtrim(workspace, '/');
73+
}
8874

89-
// storage://workspace/<folder>/workspaceIdentifier => storage://workspace/<folder>/
90-
const prefix = key.substr(0, key.length - StorageLegacyService.WORKSPACE_IDENTIFIER.length);
75+
// storage://workspace/<folder>/workspaceIdentifier => storage://workspace/<folder>/
76+
const prefix = key.substr(0, key.length - StorageLegacyService.WORKSPACE_IDENTIFIER.length);
77+
if (startsWith(workspace, folderId)) {
9178
workspaces.push({ prefix, resource: workspace });
9279
}
80+
}
81+
}
9382

94-
// Empty workspace key: storage://workspace/empty:<id>/<key>
95-
else if (startsWith(key, EMPTY_WORKSPACE_PREFIX)) {
83+
// With all the folder paths known we can now extract storage for each path. We have to go through all workspaces
84+
// from the longest path first to reliably extract the storage. The reason is that one folder path can be a parent
85+
// of another folder path and as such a simple indexOf check is not enough.
86+
const workspacesByLength = workspaces.sort((w1, w2) => w1.prefix.length >= w2.prefix.length ? -1 : 1);
9687

97-
// storage://workspace/empty:<id>/<key> => <id>
98-
const emptyWorkspaceId = key.substring(EMPTY_WORKSPACE_PREFIX.length, key.indexOf('/', EMPTY_WORKSPACE_PREFIX.length));
99-
const emptyWorkspaceResource = URI.from({ path: emptyWorkspaceId, scheme: 'empty' }).toString();
88+
const folderWorkspaceStorage: StorageObject = Object.create(null);
10089

101-
let emptyWorkspaceStorage = emptyWorkspacesStorage.get(emptyWorkspaceResource);
102-
if (!emptyWorkspaceStorage) {
103-
emptyWorkspaceStorage = Object.create(null);
104-
emptyWorkspacesStorage.set(emptyWorkspaceResource, emptyWorkspaceStorage);
105-
}
90+
workspacesByLength.forEach(workspace => {
91+
activeKeys.forEach(key => {
92+
if (!startsWith(key, workspace.prefix)) {
93+
return; // not part of workspace prefix or already handled
94+
}
10695

107-
// storage://workspace/empty:<id>/someKey => someKey
108-
const storageKey = key.substr(EMPTY_WORKSPACE_PREFIX.length + emptyWorkspaceId.length + 1 /* trailing / */);
96+
activeKeys.delete(key);
10997

110-
emptyWorkspaceStorage[storageKey] = storage.getItem(key);
98+
if (workspace.resource === folderId) {
99+
// storage://workspace/<folder>/someKey => someKey
100+
const storageKey = key.substr(workspace.prefix.length);
101+
folderWorkspaceStorage[storageKey] = storage.getItem(key);
111102
}
103+
});
104+
});
112105

113-
// Multi root workspace key: storage://workspace/root:<id>/<key>
114-
else if (startsWith(key, MULTI_ROOT_WORKSPACE_PREFIX)) {
115-
116-
// storage://workspace/root:<id>/<key> => <id>
117-
const multiRootWorkspaceId = key.substring(MULTI_ROOT_WORKSPACE_PREFIX.length, key.indexOf('/', MULTI_ROOT_WORKSPACE_PREFIX.length));
118-
const multiRootWorkspaceResource = URI.from({ path: multiRootWorkspaceId, scheme: 'root' }).toString();
106+
return folderWorkspaceStorage;
107+
}
119108

120-
let multiRootWorkspaceStorage = multiRootWorkspacesStorage.get(multiRootWorkspaceResource);
121-
if (!multiRootWorkspaceStorage) {
122-
multiRootWorkspaceStorage = Object.create(null);
123-
multiRootWorkspacesStorage.set(multiRootWorkspaceResource, multiRootWorkspaceStorage);
124-
}
109+
export function parseNoWorkspaceStorage(storage: IStorageLegacy) {
110+
const noWorkspacePrefix = `${StorageLegacyService.WORKSPACE_PREFIX}__$noWorkspace__`;
125111

126-
// storage://workspace/root:<id>/someKey => someKey
127-
const storageKey = key.substr(MULTI_ROOT_WORKSPACE_PREFIX.length + multiRootWorkspaceId.length + 1 /* trailing / */);
112+
const noWorkspaceStorage: StorageObject = Object.create(null);
113+
for (let i = 0; i < storage.length; i++) {
114+
const key = storage.key(i);
128115

129-
multiRootWorkspaceStorage[storageKey] = storage.getItem(key);
130-
}
116+
// No Workspace key is for extension development windows
117+
if (startsWith(key, noWorkspacePrefix) && !endsWith(key, StorageLegacyService.WORKSPACE_IDENTIFIER)) {
118+
// storage://workspace/__$noWorkspace__someKey => someKey
119+
noWorkspaceStorage[key.substr(NO_WORKSPACE_PREFIX.length)] = storage.getItem(key);
131120
}
121+
}
132122

133-
// Global Storage (storage://global)
134-
else if (startsWith(key, StorageLegacyService.GLOBAL_PREFIX)) {
123+
return noWorkspaceStorage;
124+
}
135125

136-
// storage://global/someKey => someKey
137-
const globalStorageKey = key.substr(StorageLegacyService.GLOBAL_PREFIX.length);
138-
if (startsWith(globalStorageKey, StorageLegacyService.COMMON_PREFIX)) {
139-
continue; // filter out faulty keys that have the form storage://something/storage://
140-
}
126+
export function parseEmptyStorage(storage: IStorageLegacy, targetWorkspaceId: string): StorageObject {
127+
const emptyStoragePrefix = `${COMMON_WORKSPACE_PREFIX}${targetWorkspaceId}/`;
128+
129+
const emptyWorkspaceStorage: StorageObject = Object.create(null);
130+
for (let i = 0; i < storage.length; i++) {
131+
const key = storage.key(i);
141132

142-
globalStorage.set(globalStorageKey, storage.getItem(key));
133+
if (startsWith(key, emptyStoragePrefix) && !endsWith(key, StorageLegacyService.WORKSPACE_IDENTIFIER)) {
134+
// storage://workspace/empty:<id>/someKey => someKey
135+
emptyWorkspaceStorage[key.substr(emptyStoragePrefix.length)] = storage.getItem(key);
143136
}
144137
}
145138

146-
// With all the folder paths known we can now extract storage for each path. We have to go through all workspaces
147-
// from the longest path first to reliably extract the storage. The reason is that one folder path can be a parent
148-
// of another folder path and as such a simple indexOf check is not enough.
149-
const workspacesByLength = workspaces.sort((w1, w2) => w1.prefix.length >= w2.prefix.length ? -1 : 1);
150-
const handledKeys = new Map<string, boolean>();
151-
workspacesByLength.forEach(workspace => {
152-
for (let i = 0; i < storage.length; i++) {
153-
const key = storage.key(i);
154-
155-
if (handledKeys.has(key) || !startsWith(key, workspace.prefix)) {
156-
continue; // not part of workspace prefix or already handled
157-
}
158-
159-
handledKeys.set(key, true);
139+
return emptyWorkspaceStorage;
140+
}
160141

161-
let folderWorkspaceStorage = folderWorkspacesStorage.get(workspace.resource);
162-
if (!folderWorkspaceStorage) {
163-
folderWorkspaceStorage = Object.create(null);
164-
folderWorkspacesStorage.set(workspace.resource, folderWorkspaceStorage);
165-
}
142+
export function parseMultiRootStorage(storage: IStorageLegacy, targetWorkspaceId: string): StorageObject {
143+
const multiRootStoragePrefix = `${COMMON_WORKSPACE_PREFIX}${targetWorkspaceId}/`;
166144

167-
// storage://workspace/<folder>/someKey => someKey
168-
const storageKey = key.substr(workspace.prefix.length);
145+
const multiRootWorkspaceStorage: StorageObject = Object.create(null);
146+
for (let i = 0; i < storage.length; i++) {
147+
const key = storage.key(i);
169148

170-
folderWorkspaceStorage[storageKey] = storage.getItem(key);
149+
if (startsWith(key, multiRootStoragePrefix) && !endsWith(key, StorageLegacyService.WORKSPACE_IDENTIFIER)) {
150+
// storage://workspace/root:<id>/someKey => someKey
151+
multiRootWorkspaceStorage[key.substr(multiRootStoragePrefix.length)] = storage.getItem(key);
171152
}
172-
});
153+
}
173154

174-
return {
175-
global: globalStorage,
176-
multiRoot: multiRootWorkspacesStorage,
177-
folder: folderWorkspacesStorage,
178-
empty: emptyWorkspacesStorage,
179-
noWorkspace: noWorkspaceStorage
180-
};
181-
}
155+
return multiRootWorkspaceStorage;
156+
}

‎src/vs/platform/storage/test/browser/storageLegacyMigration.test.ts‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import * as assert from 'assert';
77
import { StorageLegacyScope, StorageLegacyService } from 'vs/platform/storage/common/storageLegacyService';
8-
import { parseStorage } from 'vs/platform/storage/common/storageLegacyMigration';
8+
import { parseEmptyStorage, parseMultiRootStorage, parseFolderStorage } from 'vs/platform/storage/common/storageLegacyMigration';
99
import { URI } from 'vs/base/common/uri';
1010
import { startsWith } from 'vs/base/common/strings';
1111

@@ -20,18 +20,6 @@ suite('Storage Migration', () => {
2020
storage.clear();
2121
});
2222

23-
test('Parse Storage (Global)', () => {
24-
const service = createService();
25-
26-
const parsed = parseStorage(storage);
27-
28-
assert.equal(parsed.global.size, 4);
29-
assert.equal(parsed.global.get('key1'), service.get('key1'));
30-
assert.equal(parsed.global.get('key2.something'), service.get('key2.something'));
31-
assert.equal(parsed.global.get('key3/special'), service.get('key3/special'));
32-
assert.equal(parsed.global.get('key4 space'), service.get('key4 space'));
33-
});
34-
3523
test('Parse Storage (mixed)', () => {
3624

3725
// Fill the storage with multiple workspaces of all kinds (empty, root, folders)
@@ -74,23 +62,21 @@ suite('Storage Migration', () => {
7462

7563
const services = workspaceIds.map(id => createService(id));
7664

77-
const parsed = parseStorage(storage);
78-
7965
services.forEach((service, index) => {
8066
let expectedKeyCount = 4;
8167
let storageToTest;
8268

8369
const workspaceId = workspaceIds[index];
8470
if (startsWith(workspaceId, 'file:')) {
85-
storageToTest = parsed.folder.get(workspaceId);
71+
storageToTest = parseFolderStorage(storage, workspaceId);
8672
expectedKeyCount++; // workspaceIdentifier gets added!
8773
} else if (startsWith(workspaceId, 'empty:')) {
88-
storageToTest = parsed.empty.get(workspaceId);
74+
storageToTest = parseEmptyStorage(storage, workspaceId);
8975
} else if (startsWith(workspaceId, 'root:')) {
90-
storageToTest = parsed.multiRoot.get(workspaceId);
76+
storageToTest = parseMultiRootStorage(storage, workspaceId);
9177
}
9278

93-
assert.equal(Object.keys(storageToTest).length, expectedKeyCount);
79+
assert.equal(Object.keys(storageToTest).length, expectedKeyCount, 's');
9480
assert.equal(storageToTest['key1'], service.get('key1', StorageLegacyScope.WORKSPACE));
9581
assert.equal(storageToTest['key2.something'], service.get('key2.something', StorageLegacyScope.WORKSPACE));
9682
assert.equal(storageToTest['key3/special'], service.get('key3/special', StorageLegacyScope.WORKSPACE));
@@ -115,16 +101,15 @@ suite('Storage Migration', () => {
115101
s2.store('s2key3/special', true, StorageLegacyScope.WORKSPACE);
116102
s2.store('s2key4 space', 4, StorageLegacyScope.WORKSPACE);
117103

118-
const parsed = parseStorage(storage);
119104

120-
const s1Storage = parsed.folder.get(ws1);
105+
const s1Storage = parseFolderStorage(storage, ws1);
121106
assert.equal(Object.keys(s1Storage).length, 5);
122107
assert.equal(s1Storage['s1key1'], s1.get('s1key1', StorageLegacyScope.WORKSPACE));
123108
assert.equal(s1Storage['s1key2.something'], s1.get('s1key2.something', StorageLegacyScope.WORKSPACE));
124109
assert.equal(s1Storage['s1key3/special'], s1.get('s1key3/special', StorageLegacyScope.WORKSPACE));
125110
assert.equal(s1Storage['s1key4 space'], s1.get('s1key4 space', StorageLegacyScope.WORKSPACE));
126111

127-
const s2Storage = parsed.folder.get(ws2);
112+
const s2Storage = parseFolderStorage(storage, ws2);
128113
assert.equal(Object.keys(s2Storage).length, 5);
129114
assert.equal(s2Storage['s2key1'], s2.get('s2key1', StorageLegacyScope.WORKSPACE));
130115
assert.equal(s2Storage['s2key2.something'], s2.get('s2key2.something', StorageLegacyScope.WORKSPACE));

0 commit comments

Comments
 (0)