Skip to content

Commit

Permalink
#7443 remove DataGrid memory leaks (#7476)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mariusz Jurowicz authored and scottdraves committed Jun 5, 2018
1 parent 8b00d10 commit 8d9e59d
Show file tree
Hide file tree
Showing 36 changed files with 419 additions and 228 deletions.
112 changes: 0 additions & 112 deletions js/notebook/src/TableDisplay.js

This file was deleted.

115 changes: 115 additions & 0 deletions js/notebook/src/TableDisplay.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/*
* Copyright 2017 TWO SIGMA OPEN SOURCE, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as widgets from './widgets';
import { DataGridScope } from './tableDisplay/dataGrid';

import './tableDisplay/css/datatables.scss';

class TableDisplayModel extends widgets.DOMWidgetModel {
defaults() {
return {
...super.defaults(),
_model_name: 'TableDisplayModel',
_view_name: 'TableDisplayView',
_model_module: 'beakerx',
_view_module: 'beakerx',
_model_module_version: BEAKERX_MODULE_VERSION,
_view_module_version: BEAKERX_MODULE_VERSION
};
}
}

// Custom View. Renders the widget model.
class TableDisplayView extends widgets.DOMWidgetView {
private _currentScope: DataGridScope;

render() {
this._currentScope = null;
this.$el.addClass('beaker-table-display');

this.displayed.then(() => {
const tableModel = this.model.get('model');

if (tableModel.tooManyRows) {
this.showWarning(tableModel);
}

this.initDataGridTable(tableModel);

this.listenTo(this.model, 'beakerx-tabSelected', () => {
this._currentScope && this._currentScope.setInitialSize();
});

this.listenTo(this.model, 'change:updateData', this.handleUpdateData);
this.listenTo(this.model, 'change:model', this.handleModellUpdate);
});
}

handleModellUpdate() {
this._currentScope.updateModelData(this.model.get('model'));
this._currentScope.doResetAll();
}

handleUpdateData() {
const change = this.model.get('updateData');
const currentModel = this.model.get('model');

this.model.set('model', { ...currentModel, ...change }, { updated_view: this });
this.handleModellUpdate();
}

showWarning(data) {
const rowLength = data.rowLength;
const rowLimit = data.rowLimit;
const modal = document.createElement('div');

modal.setAttribute('id', this.wrapperId);
modal.innerHTML = `<p class="ansired">Note: table is too big to display.
The limit is ${rowLimit} rows, but this table has ${rowLength} rows.
The first 10000 rows are displayed as a preview.</p>`;

this.el.appendChild(modal);
}

initDataGridTable(data) {
this._currentScope = new DataGridScope({
element: this.el,
data: data,
widgetModel: this.model,
widgetView: this
});

this._currentScope.render();
}

remove() {
this._currentScope && this._currentScope.doDestroy();

if (this.pWidget) {
this.pWidget.dispose();
}

setTimeout(() => { this._currentScope = null; });

return super.remove.call(this);
}
}

export default {
TableDisplayModel: TableDisplayModel,
TableDisplayView: TableDisplayView
};
4 changes: 4 additions & 0 deletions js/notebook/src/contextMenu/BkoContextMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,5 +182,9 @@ export default abstract class BkoContextMenu implements MenuInterface {

unbind(): void {
this.scope.element[0].removeEventListener('contextmenu', this.handleContextMenu);

setTimeout(() => {
this.scope = null;
});
}
}
2 changes: 1 addition & 1 deletion js/notebook/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ module.exports = {};

var loadedModules = [
require("./Plot"),
require("./TableDisplay"),
require("./EasyForm"),
require("./TabView"),
require("./GridView"),
require("./CyclingDisplayBox"),
require("./TableDisplay").default,
require("./SparkUI").default,
require("./SparkStateProgress").default,
require("./SparkConfiguration").default,
Expand Down
35 changes: 29 additions & 6 deletions js/notebook/src/tableDisplay/dataGrid/BeakerXDataGrid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export class BeakerXDataGrid extends DataGrid {
}

resize(args?: any): void {
this.dataGridResize.resize();
this.dataGridResize && this.dataGridResize.resize();
}

setFocus(focus: boolean) {
Expand Down Expand Up @@ -186,10 +186,34 @@ export class BeakerXDataGrid extends DataGrid {
}

destroy() {
this.model && this.model.destroy();
this.eventManager.destroy();
this.columnManager.destroy();
this.columnPosition.destroy();
this.cellFocusManager.destroy();
this.cellManager.destroy();
this.cellSelectionManager.destroy();
this.cellTooltipManager.destroy();
this.highlighterManager.destroy();
this.dataGridResize.destroy();
this.rowManager.destroy();
this.dispose();

Signal.disconnectAll(this);

setTimeout(() => {
this.cellSelectionManager = null;
this.cellTooltipManager = null;
this.highlighterManager = null;
this.cellFocusManager = null;
this.dataGridResize = null;
this.columnPosition = null;
this.columnManager = null;
this.eventManager = null;
this.cellManager = null;
this.rowManager = null;
this.store = null;
});
}

onAfterAttach(msg) {
Expand Down Expand Up @@ -243,14 +267,13 @@ export class BeakerXDataGrid extends DataGrid {
}

private addCellRenderers() {
let cellRendererFactory = new CellRendererFactory(this);
let defaultRenderer = cellRendererFactory.getRenderer();
let headerCellRenderer = cellRendererFactory.getHeaderRenderer();
let defaultRenderer = CellRendererFactory.getRenderer(this);
let headerCellRenderer = CellRendererFactory.getHeaderRenderer(this);

this.cellRenderers.set(
'body',
{ dataType: ALL_TYPES[ALL_TYPES.html] },
cellRendererFactory.getRenderer(ALL_TYPES.html)
CellRendererFactory.getRenderer(this, ALL_TYPES.html)
);
this.cellRenderers.set('body', {}, defaultRenderer);
this.cellRenderers.set('column-header', {}, headerCellRenderer);
Expand All @@ -259,6 +282,6 @@ export class BeakerXDataGrid extends DataGrid {
}

private handleStateChanged() {
this.model.reset();
this.model && this.model.reset();
}
}
4 changes: 4 additions & 0 deletions js/notebook/src/tableDisplay/dataGrid/DataFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ export class DataFormatter {
this.html = this.html.bind(this);
}

destroy(): void {
this.store = null;
}

get stringFormatForColumn() {
return selectStringFormatForColumn(this.store.state);
}
Expand Down
8 changes: 6 additions & 2 deletions js/notebook/src/tableDisplay/dataGrid/DataGridResize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ export class DataGridResize {
this.installMessageHook();
}

destroy(): void {
this.dataGrid = null;
}

setInitialSize(): void {
this.setBaseRowSize();
this.resizeHeader();
Expand Down Expand Up @@ -354,13 +358,13 @@ export class DataGridResize {
}

private viewportResizeMessageHook(handler, msg) {
if (handler !== this.dataGrid.viewport) {
if (!this.dataGrid || handler !== this.dataGrid.viewport) {
return true;
}

if (msg.type === 'resize') {
setTimeout(() => {
this.dataGrid['_syncViewport']();
this.dataGrid && this.dataGrid['_syncViewport']();
});
}

Expand Down
17 changes: 13 additions & 4 deletions js/notebook/src/tableDisplay/dataGrid/DataGridScope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ import BeakerXThemeHelper from "../../BeakerXThemeHelper";
export class DataGridScope {
contextMenu: DataGridContextMenu;

readonly dataGrid: BeakerXDataGrid;
readonly element: HTMLElement;
readonly store: BeakerXDataStore;
private element: HTMLElement;
private store: BeakerXDataStore;
private dataGrid: BeakerXDataGrid;
private tableDisplayModel: any;
private tableDisplayView: any;

Expand Down Expand Up @@ -61,9 +61,14 @@ export class DataGridScope {
Widget.attach(this.dataGrid, this.element);
}

doDestroy() {
doDestroy(): void {
this.dataGrid.destroy();
this.contextMenu.destroy();

setTimeout(() => {
this.dataGrid = null;
this.store = null;
});
}

updateModelData(newData) {
Expand Down Expand Up @@ -93,4 +98,8 @@ export class DataGridScope {
initColumnLimitModal() {
return new ColumnLimitModal(this.dataGrid, this.element);
}

setInitialSize() {
this.dataGrid.setInitialSize();
}
}
Loading

0 comments on commit 8d9e59d

Please sign in to comment.