Skip to content

Conversation

@SimonSiefke
Copy link
Contributor

Fixes a two memory in chat widget related to registering animation frames.

Details

When setVisible is called, an animation Frame Disposable gets registered using this._register(dom.scheduleAtNextAnimationFrame)), making the number of registered disposables grow over time

setVisible(visible: boolean): void {
  this._register(dom.scheduleAtNextAnimationFrame(dom.getWindow(this.listContainer), () => {
	  this._onDidShow.fire();
  }));
}

Similarly, for onDidChangeTreeContentHeight, a disposable also gets registered each time:

private onDidChangeTreeContentHeight(): void {
  this._register(dom.scheduleAtNextAnimationFrame(dom.getWindow(this.listContainer), () => {
	// Can't set scrollTop during this event listener, the list might overwrite the change

	this.scrollToEnd();
  }, 0));
}

Fix

The fix uses a MutableDisposable so that there can be at most one animation frame disposable registered.

Before

When sending a chat message and clearing all chat messages 37 times, the number of functions in ChatWidget.onDidChangeTreeContentHeight and AnimationFrameQueueItem seems to grow by 1 each time.

chat-editor-send-message-with-link-original

After

When sending a chat message and clearing all chat messages 37 times, the number of functions in ChatWidget.onDidChangeTreeContentHeight and AnimationFrameQueueItem stays constant:

chat-editor-send-message-with-link

Copy link
Member

@roblourens roblourens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch thanks!

}, 0);

this._register(dom.scheduleAtNextAnimationFrame(dom.getWindow(this.listContainer), () => {
this.visibilityAnimationFrameDisposable.value = dom.scheduleAtNextAnimationFrame(dom.getWindow(this.listContainer), () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be merged with the one above but I can clean that up

@roblourens roblourens enabled auto-merge (squash) December 19, 2025 01:03
@vs-code-engineering vs-code-engineering bot added this to the December / January 2026 milestone Dec 19, 2025
@roblourens roblourens merged commit 8fec28c into microsoft:main Dec 19, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants