Skip to content

Commit

Permalink
Merge pull request #1479 from ghiscoding/perf/improve-date-sorting-speed
Browse files Browse the repository at this point in the history
feat: option to improve Date Sorting by pre-parsing date items only once
  • Loading branch information
ghiscoding authored Sep 29, 2024
2 parents 88e4372 + 8c05360 commit bffc3ee
Show file tree
Hide file tree
Showing 9 changed files with 236 additions and 152 deletions.
59 changes: 58 additions & 1 deletion docs/column-functionalities/Sorting.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- [Custom Sort Comparer](#custom-sort-comparer)
- [Update Sorting Dynamically](#update-sorting-dynamically)
- [Dynamic Query Field](#dynamic-query-field)
- [Pre-Parse Date Columns for better perf](#pre-parse-date-columns-for-better-perf)

### Demo
[Demo Page](https://ghiscoding.github.io/Angular-Slickgrid/#/clientside) / [Demo Component](https://github.com/ghiscoding/angular-slickgrid/blob/master/src/app/examples/grid-clientside.component.ts)
Expand Down Expand Up @@ -131,4 +132,60 @@ queryFieldNameGetterFn: (dataContext) => {
// for example let say that we query "profitRatio" when we have a profit else we query "lossRatio"
return dataContext.profit > 0 ? 'profitRatio' : 'lossRatio';
},
```
```

### Pre-Parse Date Columns for better perf
##### requires v5.8.0 and higher

Sorting very large dataset with dates can be extremely slow when dates formated date strings, the reason is because these strings need to first be parsed and converted to real JS Dates before the Sorting process can actually happen (i.e. US Date Format). However parsing a large dataset can be slow **and** to make it worst, a Sort will revisit the same items over and over which mean that the same date strings will have to be reparsed over and over (for example while trying to Sort a dataset of 100 items, I saw some items being revisit 10 times and I can only imagine that it is exponentially worst with a large dataset).

So what can we do to make this faster with a more reasonable time? Well, we can simply pre-parse all date strings once and only once and convert them to JS Date objects. Then once we get Date objects, we'll simply read the UNIX timestamp which is what we need to Sort. The first pre-parse takes a bit of time and will be executed only on the first date column Sort (any sort afterward will read the pre-parsed Date objects).

What perf do we get with pre-parsing versus regular non-parsing? The benchmark was pulled using 50K items with 2 date columns (with US date format)
- without non-parsing: ~15sec
- with pre-parsing: ~1.4sec (1st pre-parse) and any subsequent Date sort is about ~0.2sec => so about ~1.5sec total

The summary, is that we get a 10x boost **but** not only that, we also get an extremely fast subsequent sort afterward (sorting Date objects is as fast as sorting Numbers).

#### Usage

You can use the `preParseDateColumns` grid option, it can be either set as either `boolean` or a `string` but there's big distinction between the 2 approaches (both approaches will mutate the dataset).
1. `string` (i.e. set to `"__"`, it will parse a `"start"` date string and assign it as a `Date` object to a new `"__start"` prop)
2. `boolean` (i.e. parse `"start"` date string and reassign it as a `Date` object on the same `"start"` prop)

> **Note** this option **does not work** with Backend Services because it simply has no effect.
For example if our dataset has 2 columns named "start" and "finish", then pre-parse the dataset,

with the 1nd approach (`string`), let's use `"__"` (which is in reality a prefix) it will mutate the dataset by adding new props (where `Date` is a `Date` object)

```diff
data = [
- { id: 0, start: '02/28/24', finish: '03/02/24' },
- { id: 1, start: '01/14/24', finish: '02/13/24' },
+ { id: 0, start: '02/28/24', finish: '03/02/24', __start: Date, __finish: Date },
+ { id: 1, start: '01/14/24', finish: '02/13/24', __start: Date, __finish: Date },
]
```

with the 2nd approach (`boolean`), it will instead mutate the dataset by overwriting the same properties

```diff
data = [
- { id: 0, start: '02/28/24', finish: '03/02/24' },
- { id: 1, start: '01/14/24', finish: '02/13/24' },
+ { id: 0, start: Date, finish: Date },
+ { id: 1, start: Date, finish: Date },
]
```

Which approach to choose? Both have pros and cons, overwriting the same props might cause problems with the column `type` that you use, you will have to give it a try yoursel. On the other hand, with the other approach, it will duplicate all date properties and take a bit more memory usage and when changing cells we'll need to make sure to keep these props in sync, however you will likely have less `type` issues.

What happens when we do any cell changes (for our use case, it would be Create/Update), for any Editors we simply subscribe to the `onCellChange` change event and we re-parse the date strings when detected. We also subscribe to certain CRUD functions as long as they come from the `GridService` then all is fine... However, if you use the DataView functions directly then we have no way of knowing when to parse because these functions from the DataView don't have any events. Lastly, if we overwrite the entire dataset, we will also detect this (via an internal flag) and the next time you sort a date then the pre-parse kicks in again.

#### Can I call the pre-parse myself?

Yes, if for example you want to pre-parse right after the grid is loaded, you could call the pre-parse yourself for either all items or a single item
- all item pre-parsing: `this.sgb.sortService.preParseAllDateItems();`
- the items will be read directly from the DataView
- a single item parsing: `this.sgb.sortService.preParseSingleDateItem(item);`
28 changes: 14 additions & 14 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@
},
"dependencies": {
"@ngx-translate/core": "^15.0.0",
"@slickgrid-universal/common": "~5.7.0",
"@slickgrid-universal/custom-footer-component": "~5.7.0",
"@slickgrid-universal/empty-warning-component": "~5.7.0",
"@slickgrid-universal/event-pub-sub": "~5.7.0",
"@slickgrid-universal/pagination-component": "~5.7.0",
"@slickgrid-universal/row-detail-view-plugin": "~5.7.0",
"@slickgrid-universal/rxjs-observable": "~5.7.0",
"@slickgrid-universal/common": "~5.8.0",
"@slickgrid-universal/custom-footer-component": "~5.8.0",
"@slickgrid-universal/empty-warning-component": "~5.8.0",
"@slickgrid-universal/event-pub-sub": "~5.8.0",
"@slickgrid-universal/pagination-component": "~5.8.0",
"@slickgrid-universal/row-detail-view-plugin": "~5.8.0",
"@slickgrid-universal/rxjs-observable": "~5.8.0",
"dequal": "^2.0.3",
"rxjs": "^7.8.1"
},
Expand All @@ -79,19 +79,19 @@
"@angular/platform-browser": "^18.2.6",
"@angular/platform-browser-dynamic": "^18.2.6",
"@angular/router": "^18.2.6",
"@faker-js/faker": "^8.4.1",
"@faker-js/faker": "^9.0.3",
"@fnando/sparkline": "^0.3.10",
"@formkit/tempo": "^0.1.2",
"@ng-select/ng-select": "^13.8.2",
"@ngx-translate/http-loader": "^8.0.0",
"@popperjs/core": "^2.11.8",
"@release-it/conventional-changelog": "^8.0.2",
"@slickgrid-universal/composite-editor-component": "~5.7.0",
"@slickgrid-universal/custom-tooltip-plugin": "~5.7.0",
"@slickgrid-universal/excel-export": "~5.7.0",
"@slickgrid-universal/graphql": "~5.7.0",
"@slickgrid-universal/odata": "~5.7.0",
"@slickgrid-universal/text-export": "~5.7.0",
"@slickgrid-universal/composite-editor-component": "~5.8.0",
"@slickgrid-universal/custom-tooltip-plugin": "~5.8.0",
"@slickgrid-universal/excel-export": "~5.8.0",
"@slickgrid-universal/graphql": "~5.8.0",
"@slickgrid-universal/odata": "~5.8.0",
"@slickgrid-universal/text-export": "~5.8.0",
"@types/dompurify": "^3.0.5",
"@types/fnando__sparkline": "^0.3.7",
"@types/jest": "^29.5.13",
Expand Down
3 changes: 3 additions & 0 deletions src/app/examples/grid-clientside.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ <h2>
<button class="btn btn-outline-secondary btn-sm btn-icon" data-test="set-dynamic-sorting" (click)="setSortingDynamically()">
Set Sorting Dynamically
</button>
<button class="btn btn-outline-secondary btn-sm btn-icon" (click)="logItems()">
<span title="console.log all dataset items">Log Items</span>
</button>

<angular-slickgrid gridId="grid4"
[columnDefinitions]="columnDefinitions"
Expand Down
7 changes: 6 additions & 1 deletion src/app/examples/grid-clientside.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { CustomInputFilter } from './custom-inputFilter';
function randomBetween(min: number, max: number) {
return Math.floor(Math.random() * (max - min + 1) + min);
}
const NB_ITEMS = 1500;
const NB_ITEMS = 5500;
const URL_SAMPLE_COLLECTION_DATA = 'assets/data/collection_500_numbers.json';

@Component({
Expand Down Expand Up @@ -196,6 +196,7 @@ export class GridClientSideComponent implements OnInit {
],
},
externalResources: [new ExcelExportService()],
preParseDateColumns: '__' // or true
};

// mock a dataset
Expand All @@ -206,6 +207,10 @@ export class GridClientSideComponent implements OnInit {
this.angularGrid = angularGrid;
}

logItems() {
console.log(this.angularGrid.dataView?.getItems());
}

mockData(itemCount: number, startingIndex = 0): any[] {
// mock a dataset
const tempDataset = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
ExtensionList,
ExtensionService,
ExtensionUtility,
FieldType,
Filters,
FilterService,
Formatter,
Expand Down Expand Up @@ -458,19 +459,17 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
});

it('should keep frozen column index reference (via frozenVisibleColumnId) when grid is a frozen grid', () => {
const sharedFrozenIndexSpy = jest.spyOn(SharedService.prototype, 'frozenVisibleColumnId', 'set');
component.columnDefinitions = columnDefinitions;
component.gridOptions = gridOptions;
component.gridOptions.frozenColumn = 0;

component.initialization(slickEventHandler);

expect(component.eventHandler).toBe(slickEventHandler);
expect(sharedFrozenIndexSpy).toHaveBeenCalledWith('name');
expect(sharedService.frozenVisibleColumnId).toBe('name');
});

it('should update "visibleColumns" in the Shared Service when "onColumnsReordered" event is triggered', () => {
const sharedHasColumnsReorderedSpy = jest.spyOn(SharedService.prototype, 'hasColumnsReordered', 'set');
const sharedVisibleColumnsSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
const newVisibleColumns = [{ id: 'lastName', field: 'lastName' }, { id: 'fristName', field: 'fristName' }];

Expand All @@ -479,7 +478,7 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
mockGrid.onColumnsReordered.notify({ impactedColumns: newVisibleColumns, grid: mockGrid });

expect(component.eventHandler).toEqual(slickEventHandler);
expect(sharedHasColumnsReorderedSpy).toHaveBeenCalledWith(true);
expect(sharedService.hasColumnsReordered).toBe(true);
expect(sharedVisibleColumnsSpy).toHaveBeenCalledWith(newVisibleColumns);
});

Expand Down Expand Up @@ -575,6 +574,40 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
expect(resizerSpy).toHaveBeenCalledWith();
});

it('should expect a console warning when grid is initialized with a dataset larger than 5K items without pre-parsing enabled', () => {
const consoleWarnSpy = jest.spyOn(console, 'warn').mockReturnValue();
jest.spyOn(mockDataView, 'getItemCount').mockReturnValueOnce(5001);
const mockColumns: Column[] = [
{ id: 'firstName', field: 'firstName' },
{ id: 'updatedDate', field: 'updatedDate', type: FieldType.dateIso },
];
jest.spyOn(mockGrid, 'getColumns').mockReturnValueOnce(mockColumns);

component.gridOptions = { enableAutoResize: true };
component.ngAfterViewInit();

expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('[Slickgrid-Universal] For getting better perf, we suggest you enable the `preParseDateColumns` grid option'));
});

it('should expect a console warning when assigned dataset is larger than 5K items without pre-parsing enabled', () => {
const consoleWarnSpy = jest.spyOn(console, 'warn').mockReturnValue();
jest.spyOn(mockDataView, 'getItemCount').mockReturnValueOnce(0);
const mockColumns: Column[] = [
{ id: 'firstName', field: 'firstName' },
{ id: 'updatedDate', field: 'updatedDate', type: FieldType.dateIso },
];
jest.spyOn(mockGrid, 'getColumns').mockReturnValueOnce(mockColumns);

component.gridOptions = { enableAutoResize: true };
component.ngAfterViewInit();

// we'll do a fake dataset assignment of 5001 items
jest.spyOn(mockDataView, 'getItemCount').mockReturnValueOnce(5001);
component.dataset = [{ firstName: 'John', updatedDate: '2020-02-01' }];

expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('[Slickgrid-Universal] For getting better perf, we suggest you enable the `preParseDateColumns` grid option'));
});

describe('autoAddCustomEditorFormatter grid option', () => {
it('should initialize the grid and automatically add custom Editor Formatter when provided in the grid options', () => {
component.gridOptions = { ...gridOptions, autoAddCustomEditorFormatter: customEditableInputFormatter };
Expand Down Expand Up @@ -818,22 +851,19 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
describe('use grouping', () => {
it('should load groupItemMetaProvider to the DataView when using "draggableGrouping" feature', () => {
const dataviewSpy = jest.spyOn(SlickDataView.prototype, 'constructor' as any);
const sharedMetaSpy = jest.spyOn(SharedService.prototype, 'groupItemMetadataProvider', 'set');
jest.spyOn(extensionServiceStub, 'extensionList', 'get').mockReturnValue({ draggableGrouping: { pluginName: 'DraggableGrouping' } } as unknown as ExtensionList<any>);

component.gridOptions = { draggableGrouping: {} };
component.initialization(slickEventHandler);

expect(dataviewSpy).toHaveBeenCalledWith(expect.objectContaining({ inlineFilters: false, groupItemMetadataProvider: expect.anything() }), eventPubSubService);
expect(sharedService.groupItemMetadataProvider instanceof SlickGroupItemMetadataProvider).toBeTruthy();
expect(sharedMetaSpy).toHaveBeenCalledWith(expect.toBeObject());

component.destroy();
});

it('should load groupItemMetaProvider to the DataView when using "enableGrouping" feature', () => {
const dataviewSpy = jest.spyOn(SlickDataView.prototype, 'constructor' as any);
const sharedMetaSpy = jest.spyOn(SharedService.prototype, 'groupItemMetadataProvider', 'set');
jest.spyOn(extensionServiceStub, 'extensionList', 'get').mockReturnValue({ draggableGrouping: { pluginName: 'DraggableGrouping' } } as unknown as ExtensionList<any>);

component.gridOptions = { enableGrouping: true, draggableGrouping: {} };
Expand All @@ -843,7 +873,6 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
// expect(Object.keys(extensions).length).toBe(1);
expect(dataviewSpy).toHaveBeenCalledWith(expect.objectContaining({ inlineFilters: false, groupItemMetadataProvider: expect.anything() }), eventPubSubService);
expect(sharedService.groupItemMetadataProvider instanceof SlickGroupItemMetadataProvider).toBeTruthy();
expect(sharedMetaSpy).toHaveBeenCalledWith(expect.toBeObject());
expect(mockGrid.registerPlugin).toHaveBeenCalled();

// component.destroy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
DataViewOption,
EventSubscription,
ExternalResource,
isColumnDateType,
ItemMetadata,
Locale,
Metrics,
Expand Down Expand Up @@ -84,6 +85,8 @@ import { AngularUtilService } from '../services/angularUtil.service';
import { SlickRowDetailView } from '../extensions/slickRowDetailView';
import { ContainerService } from '../services/container.service';

const WARN_NO_PREPARSE_DATE_SIZE = 5000; // data size to warn user when pre-parse isn't enabled

@Component({
selector: 'angular-slickgrid',
templateUrl: './angular-slickgrid.component.html',
Expand Down Expand Up @@ -211,6 +214,7 @@ export class AngularSlickgridComponent<TData = any> implements AfterViewInit, On
this.slickGrid.autosizeColumns();
this._isAutosizeColsCalled = true;
}
this.suggestDateParsingWhenHelpful();
}

@Input()
Expand Down Expand Up @@ -302,7 +306,7 @@ export class AngularSlickgridComponent<TData = any> implements AfterViewInit, On
this.filterFactory = new FilterFactory(slickgridConfig, this.translaterService, this.collectionService);
this.filterService = externalServices?.filterService ?? new FilterService(this.filterFactory as any, this._eventPubSubService, this.sharedService, this.backendUtilityService);
this.resizerService = externalServices?.resizerService ?? new ResizerService(this._eventPubSubService);
this.sortService = externalServices?.sortService ?? new SortService(this.sharedService, this._eventPubSubService, this.backendUtilityService);
this.sortService = externalServices?.sortService ?? new SortService(this.collectionService, this.sharedService, this._eventPubSubService, this.backendUtilityService);
this.treeDataService = externalServices?.treeDataService ?? new TreeDataService(this._eventPubSubService, this.sharedService, this.sortService);
this.paginationService = externalServices?.paginationService ?? new PaginationService(this._eventPubSubService, this.sharedService, this.backendUtilityService);

Expand Down Expand Up @@ -372,6 +376,8 @@ export class AngularSlickgridComponent<TData = any> implements AfterViewInit, On
if (this.gridOptions.darkMode) {
this.setDarkMode(true);
}

this.suggestDateParsingWhenHelpful();
}

ngOnDestroy(): void {
Expand Down Expand Up @@ -937,6 +943,7 @@ export class AngularSlickgridComponent<TData = any> implements AfterViewInit, On
this.handleOnItemCountChanged(dataView.getFilteredItemCount() || 0, dataView.getItemCount() || 0);
});
this._eventHandler.subscribe(dataView.onSetItemsCalled, (_e, args) => {
this.sharedService.isItemsDateParsed = false;
this.handleOnItemCountChanged(dataView.getFilteredItemCount() || 0, args.itemCount);

// when user has resize by content enabled, we'll force a full width calculation since we change our entire dataset
Expand Down Expand Up @@ -1487,6 +1494,15 @@ export class AngularSlickgridComponent<TData = any> implements AfterViewInit, On
});
}

protected suggestDateParsingWhenHelpful() {
if (this.dataView?.getItemCount() > WARN_NO_PREPARSE_DATE_SIZE && !this.gridOptions.preParseDateColumns && this.slickGrid.getColumns().some(c => isColumnDateType(c.type))) {
console.warn(
'[Slickgrid-Universal] For getting better perf, we suggest you enable the `preParseDateColumns` grid option, ' +
'for more info visit:: https://ghiscoding.gitbook.io/slickgrid-universal/column-functionalities/sorting#pre-parse-date-columns-for-better-perf'
);
}
}

/**
* When the Editor(s) has a "editor.collection" property, we'll load the async collection.
* Since this is called after the async call resolves, the pointer will not be the same as the "column" argument passed.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ApplicationRef, Component } from '@angular/core';
import { Column, OnSelectedRowsChangedEventArgs, SharedService, SlickEvent, SlickEventData, SlickEventHandler, SlickGrid, SlickRowSelectionModel, } from '@slickgrid-universal/common';
import { Column, OnSelectedRowsChangedEventArgs, SlickEvent, SlickEventData, SlickEventHandler, SlickGrid, SlickRowSelectionModel, } from '@slickgrid-universal/common';
import { EventPubSubService } from '@slickgrid-universal/event-pub-sub';
import { of } from 'rxjs';

Expand Down Expand Up @@ -182,7 +182,6 @@ describe('SlickRowDetailView', () => {
gridOptionsMock.rowDetailView!.preloadComponent = TestPreloadComponent;
gridOptionsMock.rowDetailView!.viewComponent = TestComponent;
columnsMock = [{ id: 'field1', field: 'field1', width: 100, cssClass: 'red' }];
jest.spyOn(SharedService.prototype, 'slickGrid', 'get').mockReturnValue(gridStub);
jest.spyOn(gridStub, 'getOptions').mockReturnValue(gridOptionsMock);
jest.clearAllMocks();
gridStub.onColumnsReordered = new SlickEvent();
Expand Down
Loading

0 comments on commit bffc3ee

Please sign in to comment.