Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

(core) optimization: remove lodash/pullAt

Browse Source 
Summary:
For a long array with removals proportional to that length,
lodash/pullAt becomes slow due to doing one splice per removal.
This diff swaps in an alternate implementation that doesn't become
quadratic.  On a 250k-row doc with a row-level access rule, this improves
initial page load for a viewer with access to half the rows from minutes
to seconds.

Test Plan: added test; did manual benchmarking

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2777
This commit is contained in:
Paul Fitzpatrick 2021-04-15 18:07:40 -04:00
parent bc0d6605a1
commit d64461cd81
2 changed files with 34 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
app/common/gutil.ts
View File

@ -577,6 +577,35 @@ export function mapToObject<T>(keysArray: string[], callback: (key: string) => T
return map;
}
/**
* Remove the specified elements from the array, with the elements specified by
* their index. The array arr is modified in-place. The indexes must be provided
* in order, sorted lowest to highest, with no duplicates, or out-of-bound indices,
* etc (this method does no error checking; it is used in place of lodash-pullAt
* for performance reasons).
*/
export function pruneArray<T>(arr: T[], indexes: number[]) {
if (indexes.length === 0) { return; }
if (indexes.length === 1) {
arr.splice(indexes[0], 1);
return;
}
const len = arr.length;
let arrAt = 0;
let indexesAt = 0;
for (let i = 0; i < len; i++) {
if (i === indexes[indexesAt]) {
indexesAt++;
continue;
}
if (i !== arrAt) {
arr[arrAt] = arr[i];
}
arrAt++;
}
arr.length = arrAt;
}
/**
* A List of python identifiers; the result of running keywords.kwlist in Python 2.7.6,
* plus additional illegal identifiers None, False, True

9
app/server/lib/GranularAccess.ts
View File

@ -13,7 +13,7 @@ import { UserOverride } from 'app/common/DocListAPI';
import { ErrorWithCode } from 'app/common/ErrorWithCode';
import { AclMatchInput, InfoEditor, InfoView } from 'app/common/GranularAccessClause';
import { UserInfo } from 'app/common/GranularAccessClause';
import { getSetMapValue, isObject } from 'app/common/gutil';
import { getSetMapValue, isObject, pruneArray } from 'app/common/gutil';
import { canView, Role } from 'app/common/roles';
import { FullUser } from 'app/common/UserAPI';
import { HomeDBManager } from 'app/gen-server/lib/HomeDBManager';
@ -28,7 +28,6 @@ import { getRelatedRows, getRowIdsFromDocAction } from 'app/server/lib/RowAccess
import cloneDeep = require('lodash/cloneDeep');
import fromPairs = require('lodash/fromPairs');
import get = require('lodash/get');
import pullAt = require('lodash/pullAt');
// tslint:disable:no-bitwise
@ -860,13 +859,15 @@ export class GranularAccess implements GranularAccessForBundle {
/**
* Removes the toRemove rows (indexes, not row ids) from the rowIds list and from
* the colValues structure.
*
* toRemove must be sorted, lowest to highest.
*/
private _removeRowsAt(toRemove: number[], rowIds: number[], colValues: BulkColValues|undefined) {
if (toRemove.length > 0) {
pullAt(rowIds, toRemove);
pruneArray(rowIds, toRemove);
if (colValues) {
for (const values of Object.values(colValues)) {
pullAt(values, toRemove);
pruneArray(values, toRemove);
}
}
}