Skip to content

Commit f5fdf67

Browse files
mjbvzmjbvz
authored
Re-use disposable store while rendering scm lists (microsoft#163479)
The `renderElement` functions for scm currently create new `DisposableStore` every time they are invoked. For performance, it is better to have a single `DisposableStore` for each template, and then re-use this across renderElement calls
1 parent 7318c90 commit f5fdf67

File tree

2 files changed

+32
-58
lines changed

2 files changed

+32
-58
lines changed

‎src/vs/workbench/contrib/scm/browser/scmRepositoryRenderer.ts

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import 'vs/css!./media/scm';
7-
import { IDisposable, Disposable, DisposableStore, combinedDisposable, toDisposable } from 'vs/base/common/lifecycle';
7+
import { IDisposable, DisposableStore, combinedDisposable, toDisposable } from 'vs/base/common/lifecycle';
88
import { append, $ } from 'vs/base/browser/dom';
99
import { ISCMRepository, ISCMViewService } from 'vs/workbench/contrib/scm/common/scm';
1010
import { CountBadge } from 'vs/base/browser/ui/countBadge/countBadge';
@@ -30,7 +30,7 @@ interface RepositoryTemplate {
3030
readonly countContainer: HTMLElement;
3131
readonly count: CountBadge;
3232
readonly toolBar: ToolBar;
33-
disposable: IDisposable;
33+
readonly elementDisposables: DisposableStore;
3434
readonly templateDisposable: IDisposable;
3535
}
3636

@@ -65,16 +65,12 @@ export class RepositoryRenderer implements ICompressibleTreeRenderer<ISCMReposit
6565
const badgeStyler = attachBadgeStyler(count, this.themeService);
6666
const visibilityDisposable = toolBar.onDidChangeDropdownVisibility(e => provider.classList.toggle('active', e));
6767

68-
const disposable = Disposable.None;
6968
const templateDisposable = combinedDisposable(visibilityDisposable, toolBar, badgeStyler);
7069

71-
return { label, name, description, countContainer, count, toolBar, disposable, templateDisposable };
70+
return { label, name, description, countContainer, count, toolBar, elementDisposables: new DisposableStore(), templateDisposable };
7271
}
7372

7473
renderElement(arg: ISCMRepository | ITreeNode<ISCMRepository, FuzzyScore>, index: number, templateData: RepositoryTemplate, height: number | undefined): void {
75-
templateData.disposable.dispose();
76-
77-
const disposables = new DisposableStore();
7874
const repository = isSCMRepository(arg) ? arg : arg.element;
7975

8076
if (repository.provider.rootUri) {
@@ -113,8 +109,8 @@ export class RepositoryRenderer implements ICompressibleTreeRenderer<ISCMReposit
113109

114110
// TODO@joao TODO@lszomoru
115111
let disposed = false;
116-
disposables.add(toDisposable(() => disposed = true));
117-
disposables.add(repository.provider.onDidChange(() => {
112+
templateData.elementDisposables.add(toDisposable(() => disposed = true));
113+
templateData.elementDisposables.add(repository.provider.onDidChange(() => {
118114
if (disposed) {
119115
return;
120116
}
@@ -125,26 +121,24 @@ export class RepositoryRenderer implements ICompressibleTreeRenderer<ISCMReposit
125121
onDidChangeProvider();
126122

127123
const menus = this.scmViewService.menus.getRepositoryMenus(repository.provider);
128-
disposables.add(connectPrimaryMenu(menus.titleMenu.menu, (primary, secondary) => {
124+
templateData.elementDisposables.add(connectPrimaryMenu(menus.titleMenu.menu, (primary, secondary) => {
129125
menuPrimaryActions = primary;
130126
menuSecondaryActions = secondary;
131127
updateToolbar();
132128
}));
133129
templateData.toolBar.context = repository.provider;
134-
135-
templateData.disposable = disposables;
136130
}
137131

138132
renderCompressedElements(): void {
139133
throw new Error('Should never happen since node is incompressible');
140134
}
141135

142136
disposeElement(group: ISCMRepository | ITreeNode<ISCMRepository, FuzzyScore>, index: number, template: RepositoryTemplate): void {
143-
template.disposable.dispose();
137+
template.elementDisposables.clear();
144138
}
145139

146140
disposeTemplate(templateData: RepositoryTemplate): void {
147-
templateData.disposable.dispose();
141+
templateData.elementDisposables.dispose();
148142
templateData.templateDisposable.dispose();
149143
}
150144
}

‎src/vs/workbench/contrib/scm/browser/scmViewPane.ts

Lines changed: 24 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ class ActionButtonRenderer implements ICompressibleTreeRenderer<ISCMActionButton
165165

166166
interface InputTemplate {
167167
readonly inputWidget: SCMInputWidget;
168-
disposable: IDisposable;
168+
readonly elementDisposables: DisposableStore;
169169
readonly templateDisposable: IDisposable;
170170
}
171171

@@ -194,24 +194,21 @@ class InputRenderer implements ICompressibleTreeRenderer<ISCMInput, FuzzyScore,
194194
// Disable hover for list item
195195
container.parentElement!.parentElement!.classList.add('force-no-hover');
196196

197-
const disposables = new DisposableStore();
197+
const templateDisposable = new DisposableStore();
198198
const inputElement = append(container, $('.scm-input'));
199199
const inputWidget = this.instantiationService.createInstance(SCMInputWidget, inputElement, this.overflowWidgetsDomNode);
200-
disposables.add(inputWidget);
200+
templateDisposable.add(inputWidget);
201201

202-
return { inputWidget, disposable: Disposable.None, templateDisposable: disposables };
202+
return { inputWidget, elementDisposables: templateDisposable.add(new DisposableStore()), templateDisposable };
203203
}
204204

205205
renderElement(node: ITreeNode<ISCMInput, FuzzyScore>, index: number, templateData: InputTemplate): void {
206-
templateData.disposable.dispose();
207-
208-
const disposables = new DisposableStore();
209206
const input = node.element;
210207
templateData.inputWidget.input = input;
211208

212209
// Remember widget
213210
this.inputWidgets.set(input, templateData.inputWidget);
214-
disposables.add({ dispose: () => this.inputWidgets.delete(input) });
211+
templateData.elementDisposables.add({ dispose: () => this.inputWidgets.delete(input) });
215212

216213
// Widget cursor selections
217214
const selections = this.editorSelections.get(input);
@@ -220,7 +217,7 @@ class InputRenderer implements ICompressibleTreeRenderer<ISCMInput, FuzzyScore,
220217
templateData.inputWidget.selections = selections;
221218
}
222219

223-
disposables.add(toDisposable(() => {
220+
templateData.elementDisposables.add(toDisposable(() => {
224221
const selections = templateData.inputWidget.selections;
225222

226223
if (selections) {
@@ -241,32 +238,29 @@ class InputRenderer implements ICompressibleTreeRenderer<ISCMInput, FuzzyScore,
241238
};
242239

243240
const startListeningContentHeightChange = () => {
244-
disposables.add(templateData.inputWidget.onDidChangeContentHeight(onDidChangeContentHeight));
241+
templateData.elementDisposables.add(templateData.inputWidget.onDidChangeContentHeight(onDidChangeContentHeight));
245242
onDidChangeContentHeight();
246243
};
247244

248245
// Setup height change listener on next tick
249246
const timeout = disposableTimeout(startListeningContentHeightChange, 0);
250-
disposables.add(timeout);
247+
templateData.elementDisposables.add(timeout);
251248

252249
// Layout the editor whenever the outer layout happens
253250
const layoutEditor = () => templateData.inputWidget.layout();
254-
disposables.add(this.outerLayout.onDidChange(layoutEditor));
251+
templateData.elementDisposables.add(this.outerLayout.onDidChange(layoutEditor));
255252
layoutEditor();
256-
257-
templateData.disposable = disposables;
258253
}
259254

260255
renderCompressedElements(): void {
261256
throw new Error('Should never happen since node is incompressible');
262257
}
263258

264259
disposeElement(group: ITreeNode<ISCMInput, FuzzyScore>, index: number, template: InputTemplate): void {
265-
template.disposable.dispose();
260+
template.elementDisposables.clear();
266261
}
267262

268263
disposeTemplate(templateData: InputTemplate): void {
269-
templateData.disposable.dispose();
270264
templateData.templateDisposable.dispose();
271265
}
272266

@@ -299,7 +293,7 @@ interface ResourceGroupTemplate {
299293
readonly name: HTMLElement;
300294
readonly count: CountBadge;
301295
readonly actionBar: ActionBar;
302-
elementDisposables: IDisposable;
296+
readonly elementDisposables: DisposableStore;
303297
readonly disposables: IDisposable;
304298
}
305299

@@ -325,34 +319,28 @@ class ResourceGroupRenderer implements ICompressibleTreeRenderer<ISCMResourceGro
325319
const countContainer = append(element, $('.count'));
326320
const count = new CountBadge(countContainer);
327321
const styler = attachBadgeStyler(count, this.themeService);
328-
const elementDisposables = Disposable.None;
329322
const disposables = combinedDisposable(actionBar, styler);
330323

331-
return { name, count, actionBar, elementDisposables, disposables };
324+
return { name, count, actionBar, elementDisposables: new DisposableStore(), disposables };
332325
}
333326

334327
renderElement(node: ITreeNode<ISCMResourceGroup, FuzzyScore>, index: number, template: ResourceGroupTemplate): void {
335-
template.elementDisposables.dispose();
336-
337328
const group = node.element;
338329
template.name.textContent = group.label;
339330
template.actionBar.clear();
340331
template.actionBar.context = group;
341332
template.count.setCount(group.elements.length);
342333

343-
const disposables = new DisposableStore();
344334
const menus = this.scmViewService.menus.getRepositoryMenus(group.provider);
345-
disposables.add(connectPrimaryMenuToInlineActionBar(menus.getResourceGroupMenu(group), template.actionBar));
346-
347-
template.elementDisposables = disposables;
335+
template.elementDisposables.add(connectPrimaryMenuToInlineActionBar(menus.getResourceGroupMenu(group), template.actionBar));
348336
}
349337

350338
renderCompressedElements(node: ITreeNode<ICompressedTreeNode<ISCMResourceGroup>, FuzzyScore>, index: number, templateData: ResourceGroupTemplate, height: number | undefined): void {
351339
throw new Error('Should never happen since node is incompressible');
352340
}
353341

354342
disposeElement(group: ITreeNode<ISCMResourceGroup, FuzzyScore>, index: number, template: ResourceGroupTemplate): void {
355-
template.elementDisposables.dispose();
343+
template.elementDisposables.clear();
356344
}
357345

358346
disposeTemplate(template: ResourceGroupTemplate): void {
@@ -367,8 +355,8 @@ interface ResourceTemplate {
367355
fileLabel: IResourceLabel;
368356
decorationIcon: HTMLElement;
369357
actionBar: ActionBar;
370-
elementDisposables: IDisposable;
371-
disposables: IDisposable;
358+
readonly elementDisposables: DisposableStore;
359+
readonly disposables: IDisposable;
372360
}
373361

374362
interface RenderedResourceData {
@@ -430,13 +418,10 @@ class ResourceRenderer implements ICompressibleTreeRenderer<ISCMResource | IReso
430418
const decorationIcon = append(element, $('.decoration-icon'));
431419
const disposables = combinedDisposable(actionBar, fileLabel);
432420

433-
return { element, name, fileLabel, decorationIcon, actionBar, elementDisposables: Disposable.None, disposables };
421+
return { element, name, fileLabel, decorationIcon, actionBar, elementDisposables: new DisposableStore(), disposables };
434422
}
435423

436424
renderElement(node: ITreeNode<ISCMResource, FuzzyScore | LabelFuzzyScore> | ITreeNode<ISCMResource | IResourceNode<ISCMResource, ISCMResourceGroup>, FuzzyScore | LabelFuzzyScore>, index: number, template: ResourceTemplate): void {
437-
template.elementDisposables.dispose();
438-
439-
const elementDisposables = new DisposableStore();
440425
const resourceOrFolder = node.element;
441426
const iconResource = ResourceTree.isResourceNode(resourceOrFolder) ? resourceOrFolder.element : resourceOrFolder;
442427
const uri = ResourceTree.isResourceNode(resourceOrFolder) ? resourceOrFolder.uri : resourceOrFolder.sourceUri;
@@ -454,19 +439,19 @@ class ResourceRenderer implements ICompressibleTreeRenderer<ISCMResource | IReso
454439
if (ResourceTree.isResourceNode(resourceOrFolder)) {
455440
if (resourceOrFolder.element) {
456441
const menus = this.scmViewService.menus.getRepositoryMenus(resourceOrFolder.element.resourceGroup.provider);
457-
elementDisposables.add(connectPrimaryMenuToInlineActionBar(menus.getResourceMenu(resourceOrFolder.element), template.actionBar));
442+
template.elementDisposables.add(connectPrimaryMenuToInlineActionBar(menus.getResourceMenu(resourceOrFolder.element), template.actionBar));
458443
template.element.classList.toggle('faded', resourceOrFolder.element.decorations.faded);
459444
strikethrough = resourceOrFolder.element.decorations.strikeThrough;
460445
} else {
461446
matches = createMatches(node.filterData as FuzzyScore | undefined);
462447
const menus = this.scmViewService.menus.getRepositoryMenus(resourceOrFolder.context.provider);
463-
elementDisposables.add(connectPrimaryMenuToInlineActionBar(menus.getResourceFolderMenu(resourceOrFolder.context), template.actionBar));
448+
template.elementDisposables.add(connectPrimaryMenuToInlineActionBar(menus.getResourceFolderMenu(resourceOrFolder.context), template.actionBar));
464449
template.element.classList.remove('faded');
465450
}
466451
} else {
467452
[matches, descriptionMatches] = this._processFilterData(uri, node.filterData);
468453
const menus = this.scmViewService.menus.getRepositoryMenus(resourceOrFolder.resourceGroup.provider);
469-
elementDisposables.add(connectPrimaryMenuToInlineActionBar(menus.getResourceMenu(resourceOrFolder), template.actionBar));
454+
template.elementDisposables.add(connectPrimaryMenuToInlineActionBar(menus.getResourceMenu(resourceOrFolder), template.actionBar));
470455
template.element.classList.toggle('faded', resourceOrFolder.decorations.faded);
471456
strikethrough = resourceOrFolder.decorations.strikeThrough;
472457
}
@@ -487,20 +472,16 @@ class ResourceRenderer implements ICompressibleTreeRenderer<ISCMResource | IReso
487472
this.renderIcon(template, renderedData);
488473

489474
this.renderedResources.set(template, renderedData);
490-
elementDisposables.add(toDisposable(() => this.renderedResources.delete(template)));
475+
template.elementDisposables.add(toDisposable(() => this.renderedResources.delete(template)));
491476

492477
template.element.setAttribute('data-tooltip', tooltip);
493-
template.elementDisposables = elementDisposables;
494478
}
495479

496480
disposeElement(resource: ITreeNode<ISCMResource, FuzzyScore | LabelFuzzyScore> | ITreeNode<IResourceNode<ISCMResource, ISCMResourceGroup>, FuzzyScore | LabelFuzzyScore>, index: number, template: ResourceTemplate): void {
497-
template.elementDisposables.dispose();
481+
template.elementDisposables.clear();
498482
}
499483

500484
renderCompressedElements(node: ITreeNode<ICompressedTreeNode<ISCMResource> | ICompressedTreeNode<IResourceNode<ISCMResource, ISCMResourceGroup>>, FuzzyScore | LabelFuzzyScore>, index: number, template: ResourceTemplate, height: number | undefined): void {
501-
template.elementDisposables.dispose();
502-
503-
const elementDisposables = new DisposableStore();
504485
const compressed = node.element as ICompressedTreeNode<IResourceNode<ISCMResource, ISCMResourceGroup>>;
505486
const folder = compressed.elements[compressed.elements.length - 1];
506487

@@ -519,19 +500,18 @@ class ResourceRenderer implements ICompressibleTreeRenderer<ISCMResource | IReso
519500
template.actionBar.context = folder;
520501

521502
const menus = this.scmViewService.menus.getRepositoryMenus(folder.context.provider);
522-
elementDisposables.add(connectPrimaryMenuToInlineActionBar(menus.getResourceFolderMenu(folder.context), template.actionBar));
503+
template.elementDisposables.add(connectPrimaryMenuToInlineActionBar(menus.getResourceFolderMenu(folder.context), template.actionBar));
523504

524505
template.name.classList.remove('strike-through');
525506
template.element.classList.remove('faded');
526507
template.decorationIcon.style.display = 'none';
527508
template.decorationIcon.style.backgroundImage = '';
528509

529510
template.element.setAttribute('data-tooltip', '');
530-
template.elementDisposables = elementDisposables;
531511
}
532512

533513
disposeCompressedElements(node: ITreeNode<ICompressedTreeNode<ISCMResource> | ICompressedTreeNode<IResourceNode<ISCMResource, ISCMResourceGroup>>, FuzzyScore | LabelFuzzyScore>, index: number, template: ResourceTemplate, height: number | undefined): void {
534-
template.elementDisposables.dispose();
514+
template.elementDisposables.clear();
535515
}
536516

537517
disposeTemplate(template: ResourceTemplate): void {

0 commit comments

Comments
 (0)