Query Editor Memory improvements (#3041)

* working through memory issues

* work in progress

* add missing value

* work in progress

* more work in progress

* various more memory fixes

* additional improvements

* fix imports

* change test that we dispose queries on close not dispose

* update tests

src/sql/base/browser/ui/table/asyncDataView.ts

@@ -3,11 +3,14 @@
  *  Licensed under the Source EULA. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { IDisposableDataProvider } from 'sql/base/browser/ui/table/interfaces';
+
 export interface IObservableCollection<T> {
 	getLength(): number;
 	at(index: number): T;
 	getRange(start: number, end: number): T[];
 	setCollectionChangedCallback(callback: (change: CollectionChange, startIndex: number, count: number) => void): void;
+	dispose(): void;
 }
 
 export interface IGridDataRow {
@@ -24,24 +27,27 @@ class LoadCancellationToken {
 }
 
 class DataWindow<TData> {
-	private _dataSourceLength: number;
 	private _data: TData[];
 	private _length: number = 0;
 	private _offsetFromDataSource: number = -1;
 
-	private loadFunction: (offset: number, count: number) => Thenable<TData[]>;
 	private lastLoadCancellationToken: LoadCancellationToken;
-	private loadCompleteCallback: (start: number, end: number) => void;
-	private placeholderItemGenerator: (index: number) => TData;
 
-	constructor(dataSourceLength: number,
-		loadFunction: (offset: number, count: number) => Thenable<TData[]>,
-		placeholderItemGenerator: (index: number) => TData,
-		loadCompleteCallback: (start: number, end: number) => void) {
-		this._dataSourceLength = dataSourceLength;
-		this.loadFunction = loadFunction;
-		this.placeholderItemGenerator = placeholderItemGenerator;
-		this.loadCompleteCallback = loadCompleteCallback;
+	constructor(
+		private loadFunction: (offset: number, count: number) => Thenable<TData[]>,
+		private placeholderItemGenerator: (index: number) => TData,
+		private loadCompleteCallback: (start: number, end: number) => void
+	) {
+	}
+
+	dispose() {
+		this._data = undefined;
+		this.loadFunction = undefined;
+		this.placeholderItemGenerator = undefined;
+		this.loadCompleteCallback = undefined;
+		if (this.lastLoadCancellationToken) {
+			this.lastLoadCancellationToken.isCancelled = true;
+		}
 	}
 
 	getStartIndex(): number {
@@ -76,10 +82,9 @@ class DataWindow<TData> {
 			return;
 		}
 
-		let cancellationToken = new LoadCancellationToken();
-		this.lastLoadCancellationToken = cancellationToken;
+		this.lastLoadCancellationToken = new LoadCancellationToken();
 		this.loadFunction(offset, length).then(data => {
-			if (!cancellationToken.isCancelled) {
+			if (!this.lastLoadCancellationToken.isCancelled) {
 				this._data = data;
 				this.loadCompleteCallback(this._offsetFromDataSource, this._offsetFromDataSource + this._length);
 			}
@@ -97,10 +102,12 @@ export class VirtualizedCollection<TData> implements IObservableCollection<TData
 
 	private collectionChangedCallback: (change: CollectionChange, startIndex: number, count: number) => void;
 
-	constructor(windowSize: number,
+	constructor(
+		windowSize: number,
 		length: number,
 		loadFn: (offset: number, count: number) => Thenable<TData[]>,
-		private _placeHolderGenerator: (index: number) => TData) {
+		private _placeHolderGenerator: (index: number) => TData
+	) {
 		this._windowSize = windowSize;
 		this._length = length;
 
@@ -110,9 +117,15 @@ export class VirtualizedCollection<TData> implements IObservableCollection<TData
 			}
 		};
 
-		this._bufferWindowBefore = new DataWindow(length, loadFn, _placeHolderGenerator, loadCompleteCallback);
-		this._window = new DataWindow(length, loadFn, _placeHolderGenerator, loadCompleteCallback);
-		this._bufferWindowAfter = new DataWindow(length, loadFn, _placeHolderGenerator, loadCompleteCallback);
+		this._bufferWindowBefore = new DataWindow(loadFn, _placeHolderGenerator, loadCompleteCallback);
+		this._window = new DataWindow(loadFn, _placeHolderGenerator, loadCompleteCallback);
+		this._bufferWindowAfter = new DataWindow(loadFn, _placeHolderGenerator, loadCompleteCallback);
+	}
+
+	dispose() {
+		this._bufferWindowAfter.dispose();
+		this._bufferWindowBefore.dispose();
+		this._window.dispose();
 	}
 
 	setCollectionChangedCallback(callback: (change: CollectionChange, startIndex: number, count: number) => void): void {
@@ -197,7 +210,7 @@ export class VirtualizedCollection<TData> implements IObservableCollection<TData
 	}
 }
 
-export class AsyncDataProvider<TData extends IGridDataRow> implements Slick.DataProvider<TData> {
+export class AsyncDataProvider<TData extends IGridDataRow> implements IDisposableDataProvider<TData> {
 
 	constructor(private dataRows: IObservableCollection<TData>) { }
 
@@ -212,4 +225,8 @@ export class AsyncDataProvider<TData extends IGridDataRow> implements Slick.Data
 	public getRange(start: number, end: number): TData[] {
 		return !this.dataRows ? undefined : this.dataRows.getRange(start, end);
 	}
+
+	dispose() {
+		this.dataRows.dispose();
+	}
 }

src/sql/base/browser/ui/table/interfaces.ts

@@ -0,0 +1,31 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { IListStyles } from 'vs/base/browser/ui/list/listWidget';
+import { Color } from 'vs/base/common/color';
+
+export interface IDisposableDataProvider<T> extends Slick.DataProvider<T> {
+	dispose(): void;
+}
+
+export interface ITableMouseEvent {
+	anchor: HTMLElement | { x: number, y: number };
+	cell?: { row: number, cell: number };
+}
+
+export interface ITableStyles extends IListStyles {
+	tableHeaderBackground?: Color;
+	tableHeaderForeground?: Color;
+}
+
+export interface ITableSorter<T> {
+	sort(args: Slick.OnSortEventArgs<T>);
+}
+
+export interface ITableConfiguration<T> {
+	dataProvider?: IDisposableDataProvider<T> | Array<T>;
+	columns?: Slick.Column<T>[];
+	sorter?: ITableSorter<T>;
+}

src/sql/base/browser/ui/table/table.ts

@@ -5,28 +5,18 @@
 
 import 'vs/css!./media/table';
 import { TableDataView } from './tableDataView';
+import { IDisposableDataProvider, ITableSorter, ITableMouseEvent, ITableConfiguration, ITableStyles } from 'sql/base/browser/ui/table/interfaces';
 
 import { IThemable } from 'vs/platform/theme/common/styler';
-import { IListStyles } from 'vs/base/browser/ui/list/listWidget';
 import * as DOM from 'vs/base/browser/dom';
-import { Color } from 'vs/base/common/color';
 import { mixin } from 'vs/base/common/objects';
-import { IDisposable, dispose } from 'vs/base/common/lifecycle';
+import { IDisposable } from 'vs/base/common/lifecycle';
 import { Orientation } from 'vs/base/browser/ui/splitview/splitview';
 import { Widget } from 'vs/base/browser/ui/widget';
 import { isArray, isBoolean } from 'vs/base/common/types';
 import { Event, Emitter } from 'vs/base/common/event';
 import { range } from 'vs/base/common/arrays';
-
-export interface ITableMouseEvent {
-	anchor: HTMLElement | { x: number, y: number };
-	cell?: { row: number, cell: number };
-}
-
-export interface ITableStyles extends IListStyles {
-	tableHeaderBackground?: Color;
-	tableHeaderForeground?: Color;
-}
+import { $ } from 'vs/base/browser/builder';
 
 function getDefaultOptions<T>(): Slick.GridOptions<T> {
 	return <Slick.GridOptions<T>>{
@@ -35,23 +25,13 @@ function getDefaultOptions<T>(): Slick.GridOptions<T> {
 	};
 }
 
-export interface ITableSorter<T> {
-	sort(args: Slick.OnSortEventArgs<T>);
-}
-
-export interface ITableConfiguration<T> {
-	dataProvider?: Slick.DataProvider<T> | Array<T>;
-	columns?: Slick.Column<T>[];
-	sorter?: ITableSorter<T>;
-}
-
 export class Table<T extends Slick.SlickData> extends Widget implements IThemable, IDisposable {
 	private styleElement: HTMLStyleElement;
 	private idPrefix: string;
 
 	private _grid: Slick.Grid<T>;
 	private _columns: Slick.Column<T>[];
-	private _data: Slick.DataProvider<T>;
+	private _data: IDisposableDataProvider<T>;
 	private _sorter: ITableSorter<T>;
 
 	private _autoscroll: boolean;
@@ -60,8 +40,6 @@ export class Table<T extends Slick.SlickData> extends Widget implements IThemabl
 
 	private _classChangeTimeout: number;
 
-	private _disposables: IDisposable[] = [];
-
 	private _onContextMenu = new Emitter<ITableMouseEvent>();
 	public readonly onContextMenu: Event<ITableMouseEvent> = this._onContextMenu.event;
 
@@ -76,6 +54,8 @@ export class Table<T extends Slick.SlickData> extends Widget implements IThemabl
 			this._data = configuration.dataProvider;
 		}
 
+		this._register(this._data);
+
 		if (configuration && configuration.columns) {
 			this._columns = configuration.columns;
 		} else {
@@ -117,6 +97,12 @@ export class Table<T extends Slick.SlickData> extends Widget implements IThemabl
 			});
 		}
 
+		this._register({
+			dispose: () => {
+				this._grid.destroy();
+			}
+		});
+
 		this.mapMouseEvent(this._grid.onContextMenu, this._onContextMenu);
 		this.mapMouseEvent(this._grid.onClick, this._onClick);
 	}
@@ -131,7 +117,8 @@ export class Table<T extends Slick.SlickData> extends Widget implements IThemabl
 	}
 
 	public dispose() {
-		dispose(this._disposables);
+		$(this._container).dispose();
+		super.dispose();
 	}
 
 	public invalidateRows(rows: number[], keepEditor: boolean) {
@@ -166,7 +153,7 @@ export class Table<T extends Slick.SlickData> extends Widget implements IThemabl
 		this._grid.setData(this._data, true);
 	}
 
-	getData(): Slick.DataProvider<T> {
+	getData(): IDisposableDataProvider<T> {
 		return this._data;
 	}
 

src/sql/base/browser/ui/table/tableDataView.ts

@@ -9,6 +9,8 @@ import { Event, Emitter } from 'vs/base/common/event';
 import { TPromise } from 'vs/base/common/winjs.base';
 import * as types from 'vs/base/common/types';
 
+import { IDisposableDataProvider } from 'sql/base/browser/ui/table/interfaces';
+
 export interface IFindPosition {
 	col: number;
 	row: number;
@@ -20,7 +22,7 @@ function defaultSort<T>(args: Slick.OnSortEventArgs<T>, data: Array<T>): Array<T
 	return data.sort((a, b) => (a[field] === b[field] ? 0 : (a[field] > b[field] ? 1 : -1)) * sign);
 }
 
-export class TableDataView<T extends Slick.SlickData> implements Slick.DataProvider<T> {
+export class TableDataView<T extends Slick.SlickData> implements IDisposableDataProvider<T> {
 	private _data: Array<T>;
 	private _findArray: Array<IFindPosition>;
 	private _findObs: Observable<IFindPosition>;
@@ -154,4 +156,10 @@ export class TableDataView<T extends Slick.SlickData> implements Slick.DataProvi
 	get findCount(): number {
 		return types.isUndefinedOrNull(this._findArray) ? 0 : this._findArray.length;
 	}
+
+	dispose() {
+		this._data = undefined;
+		this._findArray = undefined;
+		this._findObs = undefined;
+	}
 }