Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

debt - cleanup storage leftovers #67837

Merged
merged 1 commit into from
Feb 4, 2019
Merged

debt - cleanup storage leftovers #67837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 0 additions & 21 deletions src/vs/base/node/storage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { ThrottledDelayer, timeout } from 'vs/base/common/async';
import { isUndefinedOrNull } from 'vs/base/common/types';
import { mapToString, setToString } from 'vs/base/common/map';
import { basename } from 'path';
import { mark } from 'vs/base/common/performance';
import { copy, renameIgnoreError, unlink } from 'vs/base/node/pfs';
import { fill } from 'vs/base/common/arrays';

Expand Down Expand Up @@ -326,8 +325,6 @@ export class SQLiteStorageDatabase implements IStorageDatabase {

get onDidChangeItemsExternal(): Event<IStorageItemsChangeEvent> { return Event.None; } // since we are the only client, there can be no external changes

private static measuredRequireDuration: boolean; // TODO@Ben remove me after a while

private static BUSY_OPEN_TIMEOUT = 2000; // timeout in ms to retry when opening DB fails with SQLITE_BUSY
private static MAX_HOST_PARAMETERS = 256; // maximum number of parameters within a statement

Expand Down Expand Up @@ -598,21 +595,8 @@ export class SQLiteStorageDatabase implements IStorageDatabase {
}

private doConnect(path: string): Promise<IDatabaseConnection> {

// TODO@Ben clean up performance markers
return new Promise((resolve, reject) => {
let measureRequireDuration = false;
if (!SQLiteStorageDatabase.measuredRequireDuration) {
SQLiteStorageDatabase.measuredRequireDuration = true;
measureRequireDuration = true;

mark('willRequireSQLite');
}
import('vscode-sqlite3').then(sqlite3 => {
if (measureRequireDuration) {
mark('didRequireSQLite');
}

const connection: IDatabaseConnection = {
db: new (this.logger.isTracing ? sqlite3.verbose().Database : sqlite3.Database)(path, error => {
if (error) {
Expand All @@ -622,17 +606,12 @@ export class SQLiteStorageDatabase implements IStorageDatabase {
// The following exec() statement serves two purposes:
// - create the DB if it does not exist yet
// - validate that the DB is not corrupt (the open() call does not throw otherwise)
mark('willSetupSQLiteSchema');
return this.exec(connection, [
'PRAGMA user_version = 1;',
'CREATE TABLE IF NOT EXISTS ItemTable (key TEXT UNIQUE ON CONFLICT REPLACE, value BLOB)'
].join('')).then(() => {
mark('didSetupSQLiteSchema');

return resolve(connection);
}, error => {
mark('didSetupSQLiteSchema');

return connection.db.close(() => reject(error));
});
}),
Expand Down
34 changes: 4 additions & 30 deletions src/vs/platform/storage/node/storageMainService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import { ILogService, LogLevel } from 'vs/platform/log/common/log';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
import { IStorage, Storage, SQLiteStorageDatabase, ISQLiteStorageDatabaseLoggingOptions, InMemoryStorageDatabase } from 'vs/base/node/storage';
import { join } from 'path';
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { mark, getDuration } from 'vs/base/common/performance';
import { mark } from 'vs/base/common/performance';
import { exists, readdir } from 'vs/base/node/pfs';
import { Database } from 'vscode-sqlite3';
import { endsWith, startsWith } from 'vs/base/common/strings';
Expand Down Expand Up @@ -91,8 +90,7 @@ export class StorageMainService extends Disposable implements IStorageMainServic

constructor(
@ILogService private readonly logService: ILogService,
@IEnvironmentService private readonly environmentService: IEnvironmentService,
@ITelemetryService private readonly telemetryService: ITelemetryService
@IEnvironmentService private readonly environmentService: IEnvironmentService
) {
super();

Expand All @@ -109,27 +107,9 @@ export class StorageMainService extends Disposable implements IStorageMainServic
}

private createLogginOptions(): ISQLiteStorageDatabaseLoggingOptions {
const loggedStorageErrors = new Set<string>();

return {
logTrace: (this.logService.getLevel() === LogLevel.Trace) ? msg => this.logService.trace(msg) : undefined,
logError: error => {
this.logService.error(error);

const errorStr = `${error}`;
if (!loggedStorageErrors.has(errorStr)) {
loggedStorageErrors.add(errorStr);

/* __GDPR__
"sqliteMainStorageError" : {
"storageError": { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
}
*/
this.telemetryService.publicLog('sqliteMainStorageError', {
'storageError': errorStr
});
}
}
logError: error => this.logService.error(error)
} as ISQLiteStorageDatabaseLoggingOptions;
}

Expand Down Expand Up @@ -375,18 +355,12 @@ export class StorageMainService extends Disposable implements IStorageMainServic
}

close(): Promise<void> {
this.logService.trace('StorageMainService#close() - begin');

// Signal as event so that clients can still store data
this._onWillSaveState.fire();

// Do it
mark('main:willCloseGlobalStorage');
return this.storage.close().then(() => {
mark('main:didCloseGlobalStorage');

this.logService.trace(`StorageMainService#close() - finished in ${getDuration('main:willCloseGlobalStorage', 'main:didCloseGlobalStorage')}ms`);
});
return this.storage.close();
}

checkIntegrity(full: boolean): Promise<string> {
Expand Down
53 changes: 5 additions & 48 deletions src/vs/platform/storage/node/storageService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,6 @@ export class StorageService extends Disposable implements IStorageService {
private _onWillSaveState: Emitter<IWillSaveStateEvent> = this._register(new Emitter<IWillSaveStateEvent>());
get onWillSaveState(): Event<IWillSaveStateEvent> { return this._onWillSaveState.event; }

private _hasErrors = false;
get hasErrors(): boolean { return this._hasErrors; }

private bufferedWorkspaceStorageErrors?: Array<string | Error> = [];
private _onWorkspaceStorageError: Emitter<string | Error> = this._register(new Emitter<string | Error>());
get onWorkspaceStorageError(): Event<string | Error> {
if (Array.isArray(this.bufferedWorkspaceStorageErrors)) {
// todo@ben cleanup after a while
if (this.bufferedWorkspaceStorageErrors.length > 0) {
const bufferedStorageErrors = this.bufferedWorkspaceStorageErrors;
setTimeout(() => {
this._onWorkspaceStorageError.fire(`[startup errors] ${bufferedStorageErrors.join('\n')}`);
}, 0);
}

this.bufferedWorkspaceStorageErrors = undefined;
}

return this._onWorkspaceStorageError.event;
}

private globalStorage: IStorage;

private workspaceStoragePath: string;
Expand Down Expand Up @@ -81,15 +60,7 @@ export class StorageService extends Disposable implements IStorageService {
}

private initializeGlobalStorage(): Promise<void> {
mark('willInitGlobalStorage');

return this.globalStorage.init().then(() => {
mark('didInitGlobalStorage');
}, error => {
mark('didInitGlobalStorage');

return Promise.reject(error);
});
return this.globalStorage.init();
}

private initializeWorkspaceStorage(payload: IWorkspaceInitializationPayload): Promise<void> {
Expand Down Expand Up @@ -120,17 +91,7 @@ export class StorageService extends Disposable implements IStorageService {
// Logger for workspace storage
const workspaceLoggingOptions: ISQLiteStorageDatabaseLoggingOptions = {
logTrace: (this.logService.getLevel() === LogLevel.Trace) ? msg => this.logService.trace(msg) : undefined,
logError: error => {
this.logService.error(error);

this._hasErrors = true;

if (Array.isArray(this.bufferedWorkspaceStorageErrors)) {
this.bufferedWorkspaceStorageErrors.push(error);
} else {
this._onWorkspaceStorageError.fire(error);
}
}
logError: error => this.logService.error(error)
};

// Dispose old (if any)
Expand Down Expand Up @@ -219,14 +180,10 @@ export class StorageService extends Disposable implements IStorageService {
this._onWillSaveState.fire({ reason: WillSaveStateReason.SHUTDOWN });

// Do it
mark('willCloseGlobalStorage');
mark('willCloseWorkspaceStorage');
return Promise.all([
this.globalStorage.close().then(() => mark('didCloseGlobalStorage')),
this.workspaceStorage.close().then(() => mark('didCloseWorkspaceStorage'))
]).then(() => {
this.logService.trace(`[storage] closing took ${getDuration('willCloseGlobalStorage', 'didCloseGlobalStorage')}ms global / ${getDuration('willCloseWorkspaceStorage', 'didCloseWorkspaceStorage')}ms workspace`);
});
this.globalStorage.close(),
this.workspaceStorage.close()
]).then(() => undefined);
}

private getStorage(scope: StorageScope): IStorage {
Expand Down
125 changes: 1 addition & 124 deletions src/vs/workbench/electron-browser/shell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ import { IBroadcastService, BroadcastService } from 'vs/platform/broadcast/elect
import { HashService } from 'vs/workbench/services/hash/node/hashService';
import { IHashService } from 'vs/workbench/services/hash/common/hashService';
import { ILogService } from 'vs/platform/log/common/log';
import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage';
import { IStorageService } from 'vs/platform/storage/common/storage';
import { StorageService } from 'vs/platform/storage/node/storageService';
import { Event, Emitter } from 'vs/base/common/event';
import { WORKBENCH_BACKGROUND } from 'vs/workbench/common/theme';
Expand Down Expand Up @@ -215,11 +215,6 @@ export class Shell extends Disposable {

// Startup Telemetry
this.logStartupTelemetry(startupInfos);

// Storage Telemetry (TODO@Ben remove me later, including storage errors)
if (!this.environmentService.extensionTestsPath) {
this.logStorageTelemetry();
}
}, error => handleStartupError(this.logService, error));

return workbench;
Expand Down Expand Up @@ -273,124 +268,6 @@ export class Shell extends Disposable {
perf.mark('didStartWorkbench');
}

private logStorageTelemetry(): void {
const initialStartup = !!this.configuration.isInitialStartup;

const appReadyDuration = initialStartup ? perf.getDuration('main:started', 'main:appReady') : 0;
const workbenchReadyDuration = perf.getDuration(initialStartup ? 'main:started' : 'main:loadWindow', 'didStartWorkbench');
const workspaceStorageRequireDuration = perf.getDuration('willRequireSQLite', 'didRequireSQLite');
const workspaceStorageSchemaDuration = perf.getDuration('willSetupSQLiteSchema', 'didSetupSQLiteSchema');
const globalStorageInitDurationMain = perf.getDuration('main:willInitGlobalStorage', 'main:didInitGlobalStorage');
const globalStorageInitDuratioRenderer = perf.getDuration('willInitGlobalStorage', 'didInitGlobalStorage');
const workspaceStorageInitDuration = perf.getDuration('willInitWorkspaceStorage', 'didInitWorkspaceStorage');
const workbenchLoadDuration = perf.getDuration('willLoadWorkbenchMain', 'didLoadWorkbenchMain');

// Handle errors (avoid duplicates to reduce spam)
const loggedStorageErrors = new Set<string>();
this._register(this.storageService.onWorkspaceStorageError(error => {
const errorStr = `${error}`;

if (!loggedStorageErrors.has(errorStr)) {
loggedStorageErrors.add(errorStr);

/* __GDPR__
"sqliteStorageError5" : {
"appReadyTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workbenchReadyTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceRequireTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceSchemaTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"globalReadTimeMain" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"globalReadTimeRenderer" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceReadTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workbenchRequireTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceKeys" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"startupKind": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"storageError": { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
}
*/

/* __GDPR__
"sqliteStorageError<NUMBER>" : {
"appReadyTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workbenchReadyTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceRequireTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceSchemaTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"globalReadTimeMain" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"globalReadTimeRenderer" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceReadTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workbenchRequireTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceKeys" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"startupKind": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"storageError": { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
}
*/
this.telemetryService.publicLog('sqliteStorageError5', {
'appReadyTime': appReadyDuration,
'workbenchReadyTime': workbenchReadyDuration,
'workspaceRequireTime': workspaceStorageRequireDuration,
'workspaceSchemaTime': workspaceStorageSchemaDuration,
'globalReadTimeMain': globalStorageInitDurationMain,
'globalReadTimeRenderer': globalStorageInitDuratioRenderer,
'workspaceReadTime': workspaceStorageInitDuration,
'workbenchRequireTime': workbenchLoadDuration,
'workspaceKeys': this.storageService.getSize(StorageScope.WORKSPACE),
'startupKind': this.lifecycleService.startupKind,
'storageError': errorStr
});
}
}));


if (this.storageService.hasErrors) {
return; // do not log performance numbers when errors occured
}

if (this.environmentService.verbose) {
return; // do not log when running in verbose mode
}

/* __GDPR__
"sqliteStorageTimers5" : {
"appReadyTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workbenchReadyTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceRequireTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceSchemaTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"globalReadTimeMain" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"globalReadTimeRenderer" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceReadTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workbenchRequireTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceKeys" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"startupKind": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true }
}
*/

/* __GDPR__
"sqliteStorageTimers<NUMBER>" : {
"appReadyTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workbenchReadyTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceRequireTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceSchemaTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"globalReadTimeMain" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"globalReadTimeRenderer" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceReadTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workbenchRequireTime" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"workspaceKeys" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true },
"startupKind": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true }
}
*/
this.telemetryService.publicLog('sqliteStorageTimers5', {
'appReadyTime': appReadyDuration,
'workbenchReadyTime': workbenchReadyDuration,
'workspaceRequireTime': workspaceStorageRequireDuration,
'workspaceSchemaTime': workspaceStorageSchemaDuration,
'globalReadTimeMain': globalStorageInitDurationMain,
'globalReadTimeRenderer': globalStorageInitDuratioRenderer,
'workspaceReadTime': workspaceStorageInitDuration,
'workbenchRequireTime': workbenchLoadDuration,
'workspaceKeys': this.storageService.getSize(StorageScope.WORKSPACE),
'startupKind': this.lifecycleService.startupKind
});
}

private initServiceCollection(container: HTMLElement): [IInstantiationService, ServiceCollection] {
const serviceCollection = new ServiceCollection();
Expand Down
2 changes: 0 additions & 2 deletions src/vs/workbench/parts/performance/electron-browser/perfviewEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,6 @@ class PerfModelContentProvider implements ITextModelContentProvider {
table.push(['require & init global storage', metrics.timers.ellapsedGlobalStorageInitMain, '[main]', `initial startup: ${metrics.initialStartup}`]);
table.push(['window.loadUrl() => begin to require(workbench.main.js)', metrics.timers.ellapsedWindowLoadToRequire, '[main->renderer]', StartupKindToString(metrics.windowKind)]);
table.push(['require(workbench.main.js)', metrics.timers.ellapsedRequire, '[renderer]', `cached data: ${(metrics.didUseCachedData ? 'YES' : 'NO')}${stats ? `, node_modules took ${stats.nodeRequireTotal}ms` : ''}`]);
table.push(['init global storage', metrics.timers.ellapsedGlobalStorageInitRenderer, '[renderer]', undefined]);
table.push(['require workspace storage', metrics.timers.ellapsedWorkspaceStorageRequire, '[renderer]', undefined]);
table.push(['require & init workspace storage', metrics.timers.ellapsedWorkspaceStorageInit, '[renderer]', undefined]);
table.push(['init workspace service', metrics.timers.ellapsedWorkspaceServiceInit, '[renderer]', undefined]);
table.push(['register extensions & spawn extension host', metrics.timers.ellapsedExtensions, '[renderer]', undefined]);
Expand Down