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

feat(perf): huge filtering execution speed improvements #694

Merged
merged 2 commits into from
Feb 12, 2021

Conversation

ghiscoding
Copy link
Owner

@ghiscoding ghiscoding commented Feb 12, 2021

The problem was that previous code was parsing the search values at the same time as processing the filtering check (on every single row) and that is totally unnecessary, the search value once provided will never change when comparing to each cell. This PR is rewriting the Filter Service to process the search values only once (before looping through all cells). Parsing Dates are the biggest performance killer because we take dates, that could be in any format, and we parse (convert) them to MomentJS object and that is expensive on 50,000 or more rows (see below for perf logs before/after).

Avg Gain on a set of 50k rows

  • single date filter: 6.1x faster
  • date range filter: 4.4x faster

below is a quick test with perf logs performing date filtering before & after code change with 50k rows and 8 columns grid
note: the date are all in ISO format (yyyy-mm-dd), there might be an even bigger perf boost with non-ISO format

The tests were performed on Example 14 and then clicking on "50k rows" button


each tests were made after clearing cache & refresh page

BEFORE

with a single date

searching: 1471.196044921875 ms
searching: 1438.469970703125 ms
searching: 1363.123046875 ms
avg: ~1424.26 ms

with a date range

searching: 2695.779052734375 ms
searching: 2751.52490234375 ms
searching: 2727.10888671875 ms
avg: ~2724.80 ms

AFTER

with a single date

searching: 246.010009765625 ms
searching: 236.799072265625 ms
searching: 217.127197265625 ms
avg: ~233.31 ms

with a date range

searching: 591.408935546875 ms
searching: 695.953857421875 ms
searching: 585.989990234375 ms
avg: ~624.45 ms

The problem was that previous code was parsing the search values at the same time as processing the filtering check and that is totally unnecessary, the search value once provided will never change when comparing to each cell. This PR is rewriting the Filter Service to process the search values only once (before looping through all cells). Parsing Dates are the biggest performance killer because we take dates, that could be in any format, and we parse (convert) them to MomentJS object and that is expensive on 50,000 or more rows (see below for perf logs before/after).
@codecov
Copy link

codecov bot commented Feb 12, 2021

Codecov Report

Merging #694 (012c1a5) into master (844e167) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #694   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          157       159    +2     
  Lines        10478     10576   +98     
  Branches      3684      3726   +42     
=========================================
+ Hits         10478     10576   +98     
Impacted Files Coverage Δ
...modules/angular-slickgrid/filters/filters.index.ts 100.00% <ø> (ø)
...ckgrid/filter-conditions/booleanFilterCondition.ts 100.00% <100.00%> (ø)
...lter-conditions/collectionSearchFilterCondition.ts 100.00% <100.00%> (ø)
...slickgrid/filter-conditions/dateFilterCondition.ts 100.00% <100.00%> (ø)
...grid/filter-conditions/filterConditionProcesses.ts 100.00% <100.00%> (ø)
...ckgrid/filter-conditions/filterConditions.index.ts 100.00% <100.00%> (ø)
...lar-slickgrid/filter-conditions/filterUtilities.ts 100.00% <100.00%> (ø)
...dules/angular-slickgrid/filter-conditions/index.ts 100.00% <100.00%> (ø)
...ickgrid/filter-conditions/numberFilterCondition.ts 100.00% <100.00%> (ø)
...ickgrid/filter-conditions/objectFilterCondition.ts 100.00% <100.00%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 844e167...012c1a5. Read the comment docs.

@ghiscoding ghiscoding merged commit f93a24d into master Feb 12, 2021
@ghiscoding ghiscoding deleted the feat/filter-improvements branch February 12, 2021 20:47
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.

None yet

1 participant