Skip to content

Conversation

@SimonSiefke
Copy link
Contributor

Fixes a memory leak in notebookEditorWidget.

Details

In notebookEditorWidget, when the list height changes, the scrollheight is updated. And each time the list changes height, a new disposable is added to this._localStore, making the number of disposables grow by one each time the list content height changes:

this._localStore.add(this._list.onDidChangeContentHeight(() => {
		this._localStore.add(DOM.scheduleAtNextAnimationFrame(DOM.getWindow(this.getDomNode()), () => {
			hasPendingChangeContentHeight = false;
			this._updateScrollHeight();
		}, 100));
}));

Change

The change uses a MutableDisposable so that there can be at most one disposable for the animation frame registered

const renderScrollHeightDisposable = this._localStore.add(new MutableDisposable());

this._localStore.add(this._list.onDidChangeContentHeight(() => {
		renderScrollHeightDisposable.value = DOM.scheduleAtNextAnimationFrame(DOM.getWindow(this.getDomNode()), () => {
			hasPendingChangeContentHeight = false;
			this._updateScrollHeight();
		}, 100);
}));

Before

When executing a notebook cell 37, the number of functions in AnimationFrameQueueItem and NotebookEditorWidget seems to grow by one each time:

Untitled

After

When executing a notebook cell 37, the number of AnimationFrameQueueItem stays constant (only some things link UndoGroup and StackOperations grow, which seems fine):

notebook-execute-cell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants