Skip to content

Commit bd9545d

Browse files
authored
Load storage while loading renderer, not before (microsoft#68967)
* Load storage while loading renderer, not before (fixes microsoft#68400) * storage - allow multiple init() calls
1 parent 5c049a9 commit bd9545d

7 files changed

Lines changed: 85 additions & 67 deletions

File tree

src/vs/code/electron-main/app.ts

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ import { ResolvedAuthority } from 'vs/platform/remote/common/remoteAuthorityReso
7272
import { SnapUpdateService } from 'vs/platform/update/electron-main/updateService.snap';
7373
import { IStorageMainService, StorageMainService } from 'vs/platform/storage/node/storageMainService';
7474
import { GlobalStorageDatabaseChannel } from 'vs/platform/storage/node/storageIpc';
75-
import { generateUuid } from 'vs/base/common/uuid';
7675
import { startsWith } from 'vs/base/common/strings';
7776
import { BackupMainService } from 'vs/platform/backup/electron-main/backupMainService';
7877
import { IBackupMainService } from 'vs/platform/backup/common/backup';
@@ -460,6 +459,7 @@ export class CodeApplication extends Disposable {
460459

461460
const appInstantiationService = this.instantiationService.createChild(services);
462461

462+
// Init services that require it
463463
return appInstantiationService.invokeFunction(accessor => Promise.all([
464464
this.initStorageService(accessor),
465465
this.initBackupService(accessor)
@@ -472,35 +472,8 @@ export class CodeApplication extends Disposable {
472472
// Ensure to close storage on shutdown
473473
this.lifecycleService.onWillShutdown(e => e.join(storageMainService.close()));
474474

475-
// Initialize storage service
476-
return storageMainService.initialize().then(undefined, error => {
477-
errors.onUnexpectedError(error);
478-
this.logService.error(error);
479-
}).then(() => {
480-
481-
// Apply global telemetry values as part of the initialization
482-
// These are global across all windows and thereby should be
483-
// written from the main process once.
484-
485-
const telemetryInstanceId = 'telemetry.instanceId';
486-
const instanceId = storageMainService.get(telemetryInstanceId, undefined);
487-
if (instanceId === undefined) {
488-
storageMainService.store(telemetryInstanceId, generateUuid());
489-
}
475+
return Promise.resolve();
490476

491-
const telemetryFirstSessionDate = 'telemetry.firstSessionDate';
492-
const firstSessionDate = storageMainService.get(telemetryFirstSessionDate, undefined);
493-
if (firstSessionDate === undefined) {
494-
storageMainService.store(telemetryFirstSessionDate, new Date().toUTCString());
495-
}
496-
497-
const telemetryCurrentSessionDate = 'telemetry.currentSessionDate';
498-
const telemetryLastSessionDate = 'telemetry.lastSessionDate';
499-
const lastSessionDate = storageMainService.get(telemetryCurrentSessionDate, undefined); // previous session date was the "current" one at that time
500-
const currentSessionDate = new Date().toUTCString(); // current session date is "now"
501-
storageMainService.store(telemetryLastSessionDate, typeof lastSessionDate === 'undefined' ? null : lastSessionDate);
502-
storageMainService.store(telemetryCurrentSessionDate, currentSessionDate);
503-
});
504477
}
505478

506479
private initBackupService(accessor: ServicesAccessor): Promise<void> {
@@ -544,7 +517,7 @@ export class CodeApplication extends Disposable {
544517
this.electronIpcServer.registerChannel('url', urlChannel);
545518

546519
const storageMainService = accessor.get(IStorageMainService);
547-
const storageChannel = this._register(new GlobalStorageDatabaseChannel(storageMainService as StorageMainService));
520+
const storageChannel = this._register(new GlobalStorageDatabaseChannel(this.logService, storageMainService as StorageMainService));
548521
this.electronIpcServer.registerChannel('storage', storageChannel);
549522

550523
// Log level management

src/vs/platform/storage/node/storageIpc.ts

Lines changed: 56 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ import { StorageMainService, IStorageChangeEvent } from 'vs/platform/storage/nod
99
import { IUpdateRequest, IStorageDatabase, IStorageItemsChangeEvent } from 'vs/base/node/storage';
1010
import { mapToSerializable, serializableToMap, values } from 'vs/base/common/map';
1111
import { Disposable, IDisposable, dispose } from 'vs/base/common/lifecycle';
12+
import { onUnexpectedError } from 'vs/base/common/errors';
13+
import { ILogService } from 'vs/platform/log/common/log';
14+
import { generateUuid } from 'vs/base/common/uuid';
15+
import { instanceStorageKey, firstSessionDateStorageKey, lastSessionDateStorageKey, currentSessionDateStorageKey } from 'vs/platform/telemetry/node/workbenchCommonProperties';
1216

1317
type Key = string;
1418
type Value = string;
@@ -30,10 +34,48 @@ export class GlobalStorageDatabaseChannel extends Disposable implements IServerC
3034
private readonly _onDidChangeItems: Emitter<ISerializableItemsChangeEvent> = this._register(new Emitter<ISerializableItemsChangeEvent>());
3135
get onDidChangeItems(): Event<ISerializableItemsChangeEvent> { return this._onDidChangeItems.event; }
3236

33-
constructor(private storageMainService: StorageMainService) {
37+
private whenReady: Promise<void>;
38+
39+
constructor(
40+
private logService: ILogService,
41+
private storageMainService: StorageMainService
42+
) {
3443
super();
3544

36-
this.registerListeners();
45+
this.whenReady = this.init();
46+
}
47+
48+
private init(): Promise<void> {
49+
return this.storageMainService.initialize().then(undefined, error => {
50+
onUnexpectedError(error);
51+
this.logService.error(error);
52+
}).then(() => {
53+
54+
// Apply global telemetry values as part of the initialization
55+
// These are global across all windows and thereby should be
56+
// written from the main process once.
57+
this.initTelemetry();
58+
59+
// Setup storage change listeners
60+
this.registerListeners();
61+
});
62+
}
63+
64+
private initTelemetry(): void {
65+
const instanceId = this.storageMainService.get(instanceStorageKey, undefined);
66+
if (instanceId === undefined) {
67+
this.storageMainService.store(instanceStorageKey, generateUuid());
68+
}
69+
70+
const firstSessionDate = this.storageMainService.get(firstSessionDateStorageKey, undefined);
71+
if (firstSessionDate === undefined) {
72+
this.storageMainService.store(firstSessionDateStorageKey, new Date().toUTCString());
73+
}
74+
75+
const lastSessionDate = this.storageMainService.get(currentSessionDateStorageKey, undefined); // previous session date was the "current" one at that time
76+
const currentSessionDate = new Date().toUTCString(); // current session date is "now"
77+
this.storageMainService.store(lastSessionDateStorageKey, typeof lastSessionDate === 'undefined' ? null : lastSessionDate);
78+
this.storageMainService.store(currentSessionDateStorageKey, currentSessionDate);
3779
}
3880

3981
private registerListeners(): void {
@@ -73,26 +115,26 @@ export class GlobalStorageDatabaseChannel extends Disposable implements IServerC
73115
call(_, command: string, arg?: any): Promise<any> {
74116
switch (command) {
75117
case 'getItems': {
76-
return Promise.resolve(mapToSerializable(this.storageMainService.items));
118+
return this.whenReady.then(() => mapToSerializable(this.storageMainService.items));
77119
}
78120

79121
case 'updateItems': {
80-
const items = arg as ISerializableUpdateRequest;
81-
if (items.insert) {
82-
for (const [key, value] of items.insert) {
83-
this.storageMainService.store(key, value);
122+
return this.whenReady.then(() => {
123+
const items = arg as ISerializableUpdateRequest;
124+
if (items.insert) {
125+
for (const [key, value] of items.insert) {
126+
this.storageMainService.store(key, value);
127+
}
84128
}
85-
}
86-
87-
if (items.delete) {
88-
items.delete.forEach(key => this.storageMainService.remove(key));
89-
}
90129

91-
return Promise.resolve(); // do not wait for modifications to complete
130+
if (items.delete) {
131+
items.delete.forEach(key => this.storageMainService.remove(key));
132+
}
133+
});
92134
}
93135

94136
case 'checkIntegrity': {
95-
return this.storageMainService.checkIntegrity(arg);
137+
return this.whenReady.then(() => this.storageMainService.checkIntegrity(arg));
96138
}
97139
}
98140

src/vs/platform/storage/node/storageMainService.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { ILogService, LogLevel } from 'vs/platform/log/common/log';
1010
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
1111
import { IStorage, Storage, SQLiteStorageDatabase, ISQLiteStorageDatabaseLoggingOptions, InMemoryStorageDatabase } from 'vs/base/node/storage';
1212
import { join } from 'vs/base/common/path';
13-
import { mark } from 'vs/base/common/performance';
1413
import { exists, readdir } from 'vs/base/node/pfs';
1514
import { Database } from 'vscode-sqlite3';
1615
import { endsWith, startsWith } from 'vs/base/common/strings';
@@ -88,6 +87,8 @@ export class StorageMainService extends Disposable implements IStorageMainServic
8887

8988
private storage: IStorage;
9089

90+
private initializePromise: Promise<void>;
91+
9192
constructor(
9293
@ILogService private readonly logService: ILogService,
9394
@IEnvironmentService private readonly environmentService: IEnvironmentService
@@ -114,6 +115,14 @@ export class StorageMainService extends Disposable implements IStorageMainServic
114115
}
115116

116117
initialize(): Promise<void> {
118+
if (!this.initializePromise) {
119+
this.initializePromise = this.doInitialize();
120+
}
121+
122+
return this.initializePromise;
123+
}
124+
125+
private doInitialize(): Promise<void> {
117126
const useInMemoryStorage = this.storagePath === SQLiteStorageDatabase.IN_MEMORY_PATH;
118127

119128
let globalStorageExists: Promise<boolean>;
@@ -131,14 +140,7 @@ export class StorageMainService extends Disposable implements IStorageMainServic
131140

132141
this._register(this.storage.onDidChangeStorage(key => this._onDidChangeStorage.fire({ key })));
133142

134-
mark('main:willInitGlobalStorage');
135143
return this.storage.init().then(() => {
136-
mark('main:didInitGlobalStorage');
137-
}, error => {
138-
mark('main:didInitGlobalStorage');
139-
140-
return Promise.reject(error);
141-
}).then(() => {
142144

143145
// Migrate storage if this is the first start and we are not using in-memory
144146
let migrationPromise: Promise<void>;

src/vs/platform/storage/node/storageService.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ export class StorageService extends Disposable implements IStorageService {
3636
private workspaceStorage: IStorage;
3737
private workspaceStorageListener: IDisposable;
3838

39+
private initializePromise: Promise<void>;
40+
3941
constructor(
4042
globalStorageDatabase: IStorageDatabase,
4143
@ILogService private readonly logService: ILogService,
@@ -53,6 +55,14 @@ export class StorageService extends Disposable implements IStorageService {
5355
}
5456

5557
initialize(payload: IWorkspaceInitializationPayload): Promise<void> {
58+
if (!this.initializePromise) {
59+
this.initializePromise = this.doInitialize(payload);
60+
}
61+
62+
return this.initializePromise;
63+
}
64+
65+
private doInitialize(payload: IWorkspaceInitializationPayload): Promise<void> {
5666
return Promise.all([
5767
this.initializeGlobalStorage(),
5868
this.initializeWorkspaceStorage(payload)
@@ -227,7 +237,7 @@ export class StorageService extends Disposable implements IStorageService {
227237
workspaceItemsParsed.set(key, safeParse(value));
228238
});
229239

230-
console.group(`Storage: Global (integrity: ${result[2]}, load: ${getDuration('main:willInitGlobalStorage', 'main:didInitGlobalStorage')}, path: ${this.environmentService.globalStorageHome})`);
240+
console.group(`Storage: Global (integrity: ${result[2]}, path: ${this.environmentService.globalStorageHome})`);
231241
let globalValues: { key: string, value: string }[] = [];
232242
globalItems.forEach((value, key) => {
233243
globalValues.push({ key, value });

src/vs/platform/telemetry/node/workbenchCommonProperties.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@
66
import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage';
77
import { resolveCommonProperties } from 'vs/platform/telemetry/node/commonProperties';
88

9+
export const instanceStorageKey = 'telemetry.instanceId';
10+
export const currentSessionDateStorageKey = 'telemetry.currentSessionDate';
11+
export const firstSessionDateStorageKey = 'telemetry.firstSessionDate';
912
export const lastSessionDateStorageKey = 'telemetry.lastSessionDate';
1013

1114
export function resolveWorkbenchCommonProperties(storageService: IStorageService, commit: string, version: string, machineId: string, installSourcePath: string): Promise<{ [name: string]: string | undefined }> {
1215
return resolveCommonProperties(commit, version, machineId, installSourcePath).then(result => {
13-
const instanceId = storageService.get('telemetry.instanceId', StorageScope.GLOBAL)!;
14-
const firstSessionDate = storageService.get('telemetry.firstSessionDate', StorageScope.GLOBAL)!;
16+
const instanceId = storageService.get(instanceStorageKey, StorageScope.GLOBAL)!;
17+
const firstSessionDate = storageService.get(firstSessionDateStorageKey, StorageScope.GLOBAL)!;
1518
const lastSessionDate = storageService.get(lastSessionDateStorageKey, StorageScope.GLOBAL)!;
1619

1720
// __GDPR__COMMON__ "common.version.shell" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }

src/vs/workbench/contrib/performance/electron-browser/perfviewEditor.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ class PerfModelContentProvider implements ITextModelContentProvider {
153153
table.push(['nls:start => nls:end', metrics.timers.ellapsedNlsGeneration, '[main]', `initial startup: ${metrics.initialStartup}`]);
154154
table.push(['require(main.bundle.js)', metrics.initialStartup ? perf.getDuration('willLoadMainBundle', 'didLoadMainBundle') : undefined, '[main]', `initial startup: ${metrics.initialStartup}`]);
155155
table.push(['app.isReady => window.loadUrl()', metrics.timers.ellapsedWindowLoad, '[main]', `initial startup: ${metrics.initialStartup}`]);
156-
table.push(['require & init global storage', metrics.timers.ellapsedGlobalStorageInitMain, '[main]', `initial startup: ${metrics.initialStartup}`]);
157156
table.push(['window.loadUrl() => begin to require(workbench.main.js)', metrics.timers.ellapsedWindowLoadToRequire, '[main->renderer]', StartupKindToString(metrics.windowKind)]);
158157
table.push(['require(workbench.main.js)', metrics.timers.ellapsedRequire, '[renderer]', `cached data: ${(metrics.didUseCachedData ? 'YES' : 'NO')}${stats ? `, node_modules took ${stats.nodeRequireTotal}ms` : ''}`]);
159158
table.push(['require & init workspace storage', metrics.timers.ellapsedWorkspaceStorageInit, '[renderer]', undefined]);

src/vs/workbench/services/timer/electron-browser/timerService.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ export interface IMemoryInfo {
5353
"timers.ellapsedExtensions" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true },
5454
"timers.ellapsedExtensionsReady" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true },
5555
"timers.ellapsedRequire" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true },
56-
"timers.ellapsedGlobalStorageInitMain" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true },
5756
"timers.ellapsedWorkspaceStorageInit" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true },
5857
"timers.ellapsedWorkspaceServiceInit" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true },
5958
"timers.ellapsedViewletRestore" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true },
@@ -195,15 +194,6 @@ export interface IStartupMetrics {
195194
*/
196195
readonly ellapsedWindowLoadToRequire: number;
197196

198-
/**
199-
* The time it took to require the global storage DB, connect to it
200-
* and load the initial set of values.
201-
*
202-
* * Happens in the main-process
203-
* * Measured with the `main:willInitGlobalStorage` and `main:didInitGlobalStorage` performance marks.
204-
*/
205-
readonly ellapsedGlobalStorageInitMain: number;
206-
207197
/**
208198
* The time it took to require the workspace storage DB, connect to it
209199
* and load the initial set of values.
@@ -399,7 +389,6 @@ class TimerService implements ITimerService {
399389
ellapsedWindowLoad: initialStartup ? perf.getDuration('main:appReady', 'main:loadWindow') : undefined,
400390
ellapsedWindowLoadToRequire: perf.getDuration('main:loadWindow', 'willLoadWorkbenchMain'),
401391
ellapsedRequire: perf.getDuration('willLoadWorkbenchMain', 'didLoadWorkbenchMain'),
402-
ellapsedGlobalStorageInitMain: perf.getDuration('main:willInitGlobalStorage', 'main:didInitGlobalStorage'),
403392
ellapsedWorkspaceStorageInit: perf.getDuration('willInitWorkspaceStorage', 'didInitWorkspaceStorage'),
404393
ellapsedWorkspaceServiceInit: perf.getDuration('willInitWorkspaceService', 'didInitWorkspaceService'),
405394
ellapsedExtensions: perf.getDuration('willLoadExtensions', 'didLoadExtensions'),

0 commit comments

Comments
 (0)