Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try to skip removing list dom nodes when a list is being rapidly scrolled #164340

Merged
merged 1 commit into from Dec 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 20 additions & 11 deletions src/vs/base/browser/ui/list/listView.ts
Expand Up @@ -759,17 +759,19 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
}
}

for (const range of rangesToInsert) {
for (let i = range.start; i < range.end; i++) {
this.insertItemInDOM(i, beforeElement);
this.cache.transact(() => {
for (const range of rangesToRemove) {
for (let i = range.start; i < range.end; i++) {
this.removeItemFromDOM(i);
}
}
}

for (const range of rangesToRemove) {
for (let i = range.start; i < range.end; i++) {
this.removeItemFromDOM(i);
for (const range of rangesToInsert) {
for (let i = range.start; i < range.end; i++) {
this.insertItemInDOM(i, beforeElement);
}
}
}
});

if (renderLeft !== undefined) {
this.rowsContainer.style.left = `-${renderLeft}px`;
Expand All @@ -790,8 +792,15 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
private insertItemInDOM(index: number, beforeElement: HTMLElement | null, row?: IRow): void {
const item = this.items[index];

let isStale = false;
if (!item.row) {
item.row = row ?? this.cache.alloc(item.templateId);
if (row) {
item.row = row;
} else {
const result = this.cache.alloc(item.templateId);
item.row = result.row;
isStale = result.isReusingConnectedDomNode;
}
}

const role = this.accessibilityProvider.getRole(item.element) || 'listitem';
Expand All @@ -807,7 +816,7 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
item.checkedDisposable = checked.onDidChange(update);
}

if (!item.row.domNode.parentElement) {
if (isStale || !item.row.domNode.parentElement) {
if (beforeElement) {
this.rowsContainer.insertBefore(item.row.domNode, beforeElement);
} else {
Expand Down Expand Up @@ -1373,7 +1382,7 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
return item.size - size;
}

const row = this.cache.alloc(item.templateId);
const { row } = this.cache.alloc(item.templateId);
row.domNode.style.height = '';
this.rowsContainer.appendChild(row.domNode);

Expand Down
52 changes: 47 additions & 5 deletions src/vs/base/browser/ui/list/rowCache.ts
Expand Up @@ -25,23 +25,34 @@ export class RowCache<T> implements IDisposable {

private cache = new Map<string, IRow[]>();

private readonly transactionNodesPendingRemoval = new Set<HTMLElement>();
private inTransaction = false;

constructor(private renderers: Map<string, IListRenderer<T, any>>) { }

/**
* Returns a row either by creating a new one or reusing
* a previously released row which shares the same templateId.
*
* @returns A row and `isReusingConnectedDomNode` if the row's node is already in the dom in a stale position.
*/
alloc(templateId: string): IRow {
alloc(templateId: string): { row: IRow; isReusingConnectedDomNode: boolean } {
let result = this.getTemplateCache(templateId).pop();

if (!result) {
let isStale = false;
if (result) {
isStale = this.transactionNodesPendingRemoval.has(result.domNode);
if (isStale) {
this.transactionNodesPendingRemoval.delete(result.domNode);
}
} else {
const domNode = $('.monaco-list-row');
const renderer = this.getRenderer(templateId);
const templateData = renderer.renderTemplate(domNode);
result = { domNode, templateId, templateData };
}

return result;
return { row: result, isReusingConnectedDomNode: isStale };
}

/**
Expand All @@ -55,17 +66,47 @@ export class RowCache<T> implements IDisposable {
this.releaseRow(row);
}

/**
* Begin a set of changes that use the cache. This lets us skip work when a row is removed and then inserted again.
*/
transact(makeChanges: () => void) {
if (this.inTransaction) {
throw new Error('Already in transaction');
}

this.inTransaction = true;

try {
makeChanges();
} finally {
for (const domNode of this.transactionNodesPendingRemoval) {
this.doRemoveNode(domNode);
}

this.transactionNodesPendingRemoval.clear();
this.inTransaction = false;
}
}

private releaseRow(row: IRow): void {
const { domNode, templateId } = row;
if (domNode) {
domNode.classList.remove('scrolling');
removeFromParent(domNode);
if (this.inTransaction) {
this.transactionNodesPendingRemoval.add(domNode);
} else {
this.doRemoveNode(domNode);
}
}

const cache = this.getTemplateCache(templateId);
cache.push(row);
}

private doRemoveNode(domNode: HTMLElement) {
domNode.classList.remove('scrolling');
removeFromParent(domNode);
}

private getTemplateCache(templateId: string): IRow[] {
let result = this.cache.get(templateId);

Expand All @@ -87,6 +128,7 @@ export class RowCache<T> implements IDisposable {
});

this.cache.clear();
this.transactionNodesPendingRemoval.clear();
}

private getRenderer(templateId: string): IListRenderer<T, any> {
Expand Down