Skip to content

Speed up clearing all markers, mass event listener dispose#5066

Merged
Tyriar merged 1 commit intoxtermjs:masterfrom
Tyriar:tyriar/vscode_213174
Jun 3, 2024
Merged

Speed up clearing all markers, mass event listener dispose#5066
Tyriar merged 1 commit intoxtermjs:masterfrom
Tyriar:tyriar/vscode_213174

Conversation

@Tyriar
Copy link
Copy Markdown
Member

@Tyriar Tyriar commented Jun 3, 2024

Part of microsoft/vscode#213174
Part of microsoft/vscode#213304

Test: 1000 scrollback, decorations stress test

Before:

Screenshot 2024-06-03 at 10 59 34 AM

After:

Screenshot 2024-06-03 at 11 42 41 AM

@Tyriar Tyriar added this to the 5.6.0 milestone Jun 3, 2024
@Tyriar Tyriar self-assigned this Jun 3, 2024
@Tyriar Tyriar enabled auto-merge June 3, 2024 18:44
@Tyriar Tyriar merged commit c73793f into xtermjs:master Jun 3, 2024

export class EventEmitter<T, U = void> implements IEventEmitter<T, U> {
private _listeners: IListener<T, U>[] = [];
private _listeners: Set<IListener<T, U>> = new Set();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Tyriar Just a remark, I think this could theoretically introduce a bug: if you add the same listener function more than once. The Set will filter out duplicate references, so registering the same listener callback function twice will now only fire the listener once. I don't think this will be a problem in practice because listener callbacks are most likely passed as anonymous callback functions anyway, but yeah. Just saying :D

@Tyriar Tyriar modified the milestones: 5.6.0, 6.0.0 Jul 14, 2024
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.

2 participants