Skip to content

Commit

Permalink
Merge pull request #1681 from ghiscoding/perf/tree-expand-collapse-all
Browse files Browse the repository at this point in the history 
perf(treeData): decrease Tree Grid expandAll/collapseAll time by 40x
  • Loading branch information
@ghiscoding
ghiscoding authored Sep 21, 2024
2 parents b7c2061 + 62d342a commit 4eea035
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 35 deletions.
98 changes: 71 additions & 27 deletions packages/common/src/services/__tests__/treeData.service.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from 'vitest';
import { type BasePubSubService } from '@slickgrid-universal/event-pub-sub';

import type { Column, GridOption, BackendService } from '../../interfaces/index';
Expand All @@ -15,16 +15,19 @@ vi.mock('../utilities', async (importOriginal) => ({
unflattenParentChildArrayToTree: vi.fn(),
}));

const unflattenActual = (await vi.importActual<any>('../utilities')).unflattenParentChildArrayToTree;

vi.useFakeTimers();

const gridOptionsMock = {
const gridOptionsMock: GridOption = {
multiColumnSort: false,
enableFiltering: true,
enableTreeData: true,
treeDataOptions: {
columnId: 'file'
columnId: 'file',
childrenPropName: 'files'
}
} as unknown as GridOption;
};

const backendServiceStub = {
buildQuery: vi.fn(),
Expand Down Expand Up @@ -97,6 +100,7 @@ describe('TreeData Service', () => {
service = new TreeDataService(mockPubSub, sharedService, sortServiceStub);
slickgridEventHandler = service.eventHandler;
vi.spyOn(gridStub, 'getData').mockReturnValue(dataViewStub);
(unflattenParentChildArrayToTree as Mock).mockImplementationOnce(unflattenActual);
});

afterEach(() => {
Expand Down Expand Up @@ -181,7 +185,7 @@ describe('TreeData Service', () => {

it('should return hierarchical dataset when defined', () => {
const mockHierarchical = [{ file: 'documents', files: [{ file: 'vacation.txt' }] }];
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValue(mockHierarchical);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValueOnce(mockHierarchical);
expect(service.datasetHierarchical).toEqual(mockHierarchical);
});

Expand Down Expand Up @@ -265,7 +269,10 @@ describe('TreeData Service', () => {
mockRowData.__collapsed = true;
vi.spyOn(gridStub, 'getData').mockReturnValue(dataViewStub);
const spyGetItem = vi.spyOn(dataViewStub, 'getItem').mockReturnValue(mockRowData);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValue([mockRowData]);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get')
.mockReturnValueOnce([mockRowData])
.mockReturnValueOnce([mockRowData])
.mockReturnValueOnce([mockRowData]);
const spyUptItem = vi.spyOn(dataViewStub, 'updateItem');
const spyInvalidate = vi.spyOn(gridStub, 'invalidate');

Expand Down Expand Up @@ -316,22 +323,22 @@ describe('TreeData Service', () => {
vi.clearAllMocks();
mockFlatDataset = [
{ id: 0, file: 'TXT', size: 5.8, __hasChildren: true, __treeLevel: 0 },
{ id: 1, file: 'myFile.txt', size: 0.5, __treeLevel: 1 },
{ id: 2, file: 'myMusic.txt', size: 5.3, __treeLevel: 1 },
{ id: 1, file: 'myFile.txt', size: 0.5, __treeLevel: 1, __parentId: 0 },
{ id: 2, file: 'myMusic.txt', size: 5.3, __treeLevel: 1, __parentId: 0 },
{ id: 4, file: 'MP3', size: 3.4, __hasChildren: true, __treeLevel: 0 },
{ id: 5, file: 'relaxation.mp3', size: 3.4, __treeLevel: 1 }
{ id: 5, file: 'relaxation.mp3', size: 3.4, __treeLevel: 1, __parentId: 4 }
];
mockHierarchical = [
{ id: 0, file: 'TXT', files: [{ id: 1, file: 'myFile.txt', size: 0.5, }, { id: 2, file: 'myMusic.txt', size: 5.3, }] },
{ id: 4, file: 'MP3', files: [{ id: 5, file: 'relaxation.mp3', size: 3.4, }] }
];
gridOptionsMock.treeDataOptions = { columnId: 'file' };
gridOptionsMock.treeDataOptions = { columnId: 'file', childrenPropName: 'files' };
(unflattenParentChildArrayToTree as Mock).mockImplementationOnce(unflattenActual);
});

it('should collapse all items when calling the method with collapsing True', async () => {
const dataGetItemsSpy = vi.spyOn(dataViewStub, 'getItems').mockReturnValue(mockFlatDataset);
vi.spyOn(dataViewStub, 'getItemCount').mockReturnValue(mockFlatDataset.length);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValue(mockHierarchical);
const beginUpdateSpy = vi.spyOn(dataViewStub, 'beginUpdate');
const endUpdateSpy = vi.spyOn(dataViewStub, 'endUpdate');
const updateItemSpy = vi.spyOn(dataViewStub, 'updateItem');
Expand All @@ -344,12 +351,30 @@ describe('TreeData Service', () => {
expect(pubSubSpy).toHaveBeenCalledWith(`onTreeFullToggleEnd`, { type: 'full-collapse', previousFullToggleType: 'full-collapse', toggledItems: null, });
expect(dataGetItemsSpy).toHaveBeenCalled();
expect(beginUpdateSpy).toHaveBeenCalled();
expect(updateItemSpy).toHaveBeenNthCalledWith(1, 0, { __collapsed: true, __hasChildren: true, id: 0, file: 'TXT', size: 5.8, __treeLevel: 0 });
expect(updateItemSpy).toHaveBeenNthCalledWith(2, 4, { __collapsed: true, __hasChildren: true, id: 4, file: 'MP3', size: 3.4, __treeLevel: 0 });
expect(SharedService.prototype.hierarchicalDataset![0].file).toBe('TXT');
expect(SharedService.prototype.hierarchicalDataset![0].__collapsed).toBe(true);
expect(SharedService.prototype.hierarchicalDataset![1].file).toBe('MP3');
expect(SharedService.prototype.hierarchicalDataset![1].__collapsed).toBe(true);
expect(updateItemSpy).toHaveBeenNthCalledWith(1, 0, {
id: 0, file: 'TXT', size: 5.8, __collapsed: true, __hasChildren: true, __treeLevel: 0,
files: [
{ file: 'myFile.txt', id: 1, size: 0.5, __parentId: 0, __treeLevel: 1, },
{ file: 'myMusic.txt', id: 2, size: 5.3, __parentId: 0, __treeLevel: 1, },
],
});
expect(updateItemSpy).toHaveBeenNthCalledWith(2, 4, {
id: 4, file: 'MP3', size: 3.4, __collapsed: true, __hasChildren: true, __treeLevel: 0,
files: [{ id: 5, file: 'relaxation.mp3', size: 3.4, __parentId: 4, __treeLevel: 1, },],
});
expect(sharedService.hierarchicalDataset).toEqual([
{
id: 0, file: 'TXT', size: 5.8, __collapsed: true, __hasChildren: true, __treeLevel: 0,
files: [
{ id: 1, file: 'myFile.txt', size: 0.5, __parentId: 0, __treeLevel: 1, },
{ id: 2, file: 'myMusic.txt', size: 5.3, __parentId: 0, __treeLevel: 1, }
],
},
{
id: 4, file: 'MP3', size: 3.4, __collapsed: true, __hasChildren: true, __treeLevel: 0,
files: [{ id: 5, file: 'relaxation.mp3', size: 3.4, __parentId: 4, __treeLevel: 1 }],
}
]);
expect(service.getItemCount(0)).toBe(2); // get count by tree level 0
expect(service.getItemCount(1)).toBe(3);
expect(service.getItemCount()).toBe(5); // get full count of all tree
Expand All @@ -372,8 +397,17 @@ describe('TreeData Service', () => {
expect(dataGetItemsSpy).toHaveBeenCalled();
expect(dataGetItemsSpy).toHaveBeenCalled();
expect(beginUpdateSpy).toHaveBeenCalled();
expect(updateItemSpy).toHaveBeenNthCalledWith(1, 0, { customCollapsed: true, __hasChildren: true, id: 0, file: 'TXT', size: 5.8, __treeLevel: 0 });
expect(updateItemSpy).toHaveBeenNthCalledWith(2, 4, { customCollapsed: true, __hasChildren: true, id: 4, file: 'MP3', size: 3.4, __treeLevel: 0 });
expect(updateItemSpy).toHaveBeenNthCalledWith(1, 0, {
customCollapsed: true, __hasChildren: true, id: 0, file: 'TXT', size: 5.8, __treeLevel: 0,
files: [
{ file: 'myFile.txt', id: 1, size: 0.5, __parentId: 0, __treeLevel: 1, },
{ file: 'myMusic.txt', id: 2, size: 5.3, __parentId: 0, __treeLevel: 1, },
],
});
expect(updateItemSpy).toHaveBeenNthCalledWith(2, 4, {
customCollapsed: true, __hasChildren: true, id: 4, file: 'MP3', size: 3.4, __treeLevel: 0,
files: [{ file: 'relaxation.mp3', id: 5, size: 3.4, __parentId: 4, __treeLevel: 1, },],
});
expect(endUpdateSpy).toHaveBeenCalled();
});

Expand All @@ -391,16 +425,25 @@ describe('TreeData Service', () => {
expect(pubSubSpy).toHaveBeenCalledWith(`onTreeFullToggleEnd`, { type: 'full-expand', previousFullToggleType: 'full-expand', toggledItems: null, });
expect(dataGetItemsSpy).toHaveBeenCalled();
expect(beginUpdateSpy).toHaveBeenCalled();
expect(updateItemSpy).toHaveBeenNthCalledWith(1, 0, { __collapsed: false, __hasChildren: true, id: 0, file: 'TXT', size: 5.8, __treeLevel: 0 });
expect(updateItemSpy).toHaveBeenNthCalledWith(2, 4, { __collapsed: false, __hasChildren: true, id: 4, file: 'MP3', size: 3.4, __treeLevel: 0 });
expect(updateItemSpy).toHaveBeenNthCalledWith(1, 0, {
__collapsed: false, __hasChildren: true, id: 0, file: 'TXT', size: 5.8, __treeLevel: 0,
files: [
{ file: 'myFile.txt', id: 1, size: 0.5, __parentId: 0, __treeLevel: 1, },
{ file: 'myMusic.txt', id: 2, size: 5.3, __parentId: 0, __treeLevel: 1, },
],
});
expect(updateItemSpy).toHaveBeenNthCalledWith(2, 4, {
__collapsed: false, __hasChildren: true, id: 4, file: 'MP3', size: 3.4, __treeLevel: 0,
files: [{ file: 'relaxation.mp3', id: 5, size: 3.4, __parentId: 4, __treeLevel: 1, },],
});
expect(endUpdateSpy).toHaveBeenCalled();
});

describe('applyToggledItemStateChanges method', () => {
it('should execute the method and expect a full toggle or items', () => {
const dataGetItemsSpy = vi.spyOn(dataViewStub, 'getItems').mockReturnValue(mockFlatDataset);
vi.spyOn(dataViewStub, 'getItemById').mockReturnValue(mockFlatDataset[3]);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValue(mockHierarchical);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValueOnce(mockHierarchical);
const beginUpdateSpy = vi.spyOn(dataViewStub, 'beginUpdate');
const endUpdateSpy = vi.spyOn(dataViewStub, 'endUpdate');
const updateItemSpy = vi.spyOn(dataViewStub, 'updateItem');
Expand All @@ -419,7 +462,7 @@ describe('TreeData Service', () => {
it('should execute the method but without calling "getItems" to skip doing a full toggle of items', () => {
const dataGetItemsSpy = vi.spyOn(dataViewStub, 'getItems').mockReturnValue(mockFlatDataset);
vi.spyOn(dataViewStub, 'getItemById').mockReturnValue(mockFlatDataset[3]);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValue(mockHierarchical);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValueOnce(mockHierarchical);
const beginUpdateSpy = vi.spyOn(dataViewStub, 'beginUpdate');
const endUpdateSpy = vi.spyOn(dataViewStub, 'endUpdate');
const updateItemSpy = vi.spyOn(dataViewStub, 'updateItem');
Expand All @@ -438,7 +481,7 @@ describe('TreeData Service', () => {
it('should execute the method and also trigger an event when specified', () => {
const dataGetItemsSpy = vi.spyOn(dataViewStub, 'getItems').mockReturnValue(mockFlatDataset);
vi.spyOn(dataViewStub, 'getItemById').mockReturnValue(mockFlatDataset[3]);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValue(mockHierarchical);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValueOnce(mockHierarchical);
const beginUpdateSpy = vi.spyOn(dataViewStub, 'beginUpdate');
const endUpdateSpy = vi.spyOn(dataViewStub, 'endUpdate');
const updateItemSpy = vi.spyOn(dataViewStub, 'updateItem');
Expand All @@ -459,7 +502,7 @@ describe('TreeData Service', () => {
describe('dynamicallyToggleItemState method', () => {
it('should execute the method and also trigger an event by default', () => {
vi.spyOn(dataViewStub, 'getItemById').mockReturnValue(mockFlatDataset[3]);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValue(mockHierarchical);
vi.spyOn(SharedService.prototype, 'hierarchicalDataset', 'get').mockReturnValueOnce(mockHierarchical);
const beginUpdateSpy = vi.spyOn(dataViewStub, 'beginUpdate');
const endUpdateSpy = vi.spyOn(dataViewStub, 'endUpdate');
const updateItemSpy = vi.spyOn(dataViewStub, 'updateItem');
Expand Down Expand Up @@ -580,8 +623,9 @@ describe('TreeData Service', () => {

// 2nd test, if we toggled all items to be collapsed, we should expect the unflatten to be called with updated `initiallyCollapsed` flag
await service.toggleTreeDataCollapse(true);
service.convertFlatParentChildToTreeDatasetAndSort(mockFlatDataset, mockColumns, gridOptionsMock);
expect(unflattenParentChildArrayToTree).toHaveBeenNthCalledWith(2, mockFlatDataset, {
const result2 = service.convertFlatParentChildToTreeDatasetAndSort(mockFlatDataset, mockColumns, gridOptionsMock);
expect(result2).toEqual({ flat: mockFlatDataset as any[], hierarchical: mockHierarchical as any[] });
expect(unflattenParentChildArrayToTree).toHaveBeenNthCalledWith(3, mockFlatDataset, { // 3rd call because toggleTreeDataCollapse() made the 2nd call
columnId: 'file',
identifierPropName: 'id',
initiallyCollapsed: true, // changed to True
Expand Down
28 changes: 20 additions & 8 deletions packages/common/src/services/treeData.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ export class TreeDataService {
// then we reapply only the ones that changed (provided as argument to the function)
treeToggledItems.forEach(collapsedItem => {
const item = this.dataView.getItemById(collapsedItem.itemId);
this.updateToggledItem(item, collapsedItem.isCollapsed);
this.updateToggledItem(item, collapsedItem.isCollapsed, true);

if (shouldTriggerEvent) {
const parentFoundIdx = this._currentToggledItems.findIndex(treeChange => treeChange.itemId === collapsedItem.itemId);
Expand Down Expand Up @@ -360,12 +360,17 @@ export class TreeDataService {
this.dataView.beginUpdate(true);

// toggle the collapsed flag but only when it's a parent item with children
(this.dataView.getItems() || []).forEach((item: any) => {
const flatDataItems = (this.dataView.getItems() || []);
flatDataItems.forEach((item: any) => {
if (item[hasChildrenPropName]) {
this.updateToggledItem(item, collapsing);
this.updateToggledItem(item, collapsing, false);
}
});

// instead of finding each tree item and update their collapse flag one-by-one, we can simply recreate the tree dataset
// since the flat dataset already has collapse flag updated from previous step
this.sharedService.hierarchicalDataset = this.convertFlatParentChildToTreeDataset(flatDataItems, this.gridOptions);

this.dataView.endUpdate();
this.dataView.refresh();
this._isLastFullToggleCollapsed = collapsing;
Expand Down Expand Up @@ -437,7 +442,12 @@ export class TreeDataService {
}
}

protected updateToggledItem(item: any, isCollapsed: boolean): void {
/**
* Toggle an item in the flat dataset and update it in the grid.
* NOTE: We should update the Tree Data collapse status (shouldUpdateTree=true) ONLY when toggling 1 item at a time,
* however for toggle a batch (i.e. collapse all), we'll want to convert skip updating the tree but rather convert from flat to tree which is much quicker to execute.
*/
protected updateToggledItem(item: any, isCollapsed: boolean, shouldUpdateTree: boolean): void {
const dataViewIdIdentifier = this.gridOptions?.datasetIdPropertyName ?? 'id';
const childrenPropName = getTreeDataOptionPropName(this.treeDataOptions, 'childrenPropName');
const collapsedPropName = getTreeDataOptionPropName(this.treeDataOptions, 'collapsedPropName');
Expand All @@ -448,10 +458,12 @@ export class TreeDataService {
this.dataView.updateItem(item[dataViewIdIdentifier], item);

// also update the hierarchical tree item
const searchTreePredicate = (treeItemToSearch: any) => treeItemToSearch[dataViewIdIdentifier] === item[dataViewIdIdentifier];
const treeItemFound = findItemInTreeStructure(this.sharedService.hierarchicalDataset || [], searchTreePredicate, childrenPropName);
if (treeItemFound) {
treeItemFound[collapsedPropName] = isCollapsed;
if (shouldUpdateTree) {
const searchTreePredicate = (treeItemToSearch: any) => treeItemToSearch[dataViewIdIdentifier] === item[dataViewIdIdentifier];
const treeItemFound = findItemInTreeStructure(this.sharedService.hierarchicalDataset || [], searchTreePredicate, childrenPropName);
if (treeItemFound) {
treeItemFound[collapsedPropName] = isCollapsed;
}
}
}
}
Expand Down

0 comments on commit 4eea035

Please sign in to comment.