From d64461cd81e78e24fff1c08c0dd8e0e6c9e8c603 Mon Sep 17 00:00:00 2001
From: Paul Fitzpatrick <paulfitz@alum.mit.edu>
Date: Thu, 15 Apr 2021 18:07:40 -0400
Subject: [PATCH] (core) optimization: remove lodash/pullAt

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
---
 app/common/gutil.ts              | 29 +++++++++++++++++++++++++++++
 app/server/lib/GranularAccess.ts |  9 +++++----
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/app/common/gutil.ts b/app/common/gutil.ts
index 3814d7d5..92fcd12c 100644
--- a/app/common/gutil.ts
+++ b/app/common/gutil.ts
@@ -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
diff --git a/app/server/lib/GranularAccess.ts b/app/server/lib/GranularAccess.ts
index 88ec4fdb..2bfcd9f7 100644
--- a/app/server/lib/GranularAccess.ts
+++ b/app/server/lib/GranularAccess.ts
@@ -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);
         }
       }
     }