perf: speed up mapOfSubdocs benchmark by 4x by avoiding unnecessary O…

…(n^2) loop in getPathsToValidate() Re: #13191 Re: #13271
Automattic · Jul 15, 2023 · b8ebe80 · b8ebe80
1 parent dcc4c9a
commit b8ebe80
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 15 deletions.
diff --git a/lib/document.js b/lib/document.js
@@ -2221,16 +2221,17 @@ Document.prototype[documentModifiedPaths] = Document.prototype.modifiedPaths;
 
 Document.prototype.isModified = function(paths, modifiedPaths) {
   if (paths) {
-    if (!Array.isArray(paths)) {
-      paths = paths.indexOf(' ') === -1 ? [paths] : paths.split(' ');
-    }
-
     const directModifiedPathsObj = this.$__.activePaths.states.modify;
     if (directModifiedPathsObj == null) {
       return false;
     }
+
+    if (typeof paths === 'string') {
+      paths = [paths];
+    }
+
     for (const path of paths) {
-      if (Object.prototype.hasOwnProperty.call(directModifiedPathsObj, path)) {
+      if (directModifiedPathsObj[path] != null) {
         return true;
       }
     }
@@ -2664,14 +2665,14 @@ function _getPathsToValidate(doc) {
   const modifiedPaths = doc.modifiedPaths();
   for (const subdoc of subdocs) {
     if (subdoc.$basePath) {
-      // Remove child paths for now, because we'll be validating the whole
-      // subdoc
       const fullPathToSubdoc = subdoc.$__fullPathWithIndexes();
 
-      for (const p of paths) {
-        if (p == null || p.startsWith(fullPathToSubdoc + '.')) {
-          paths.delete(p);
-        }
+      // Remove child paths for now, because we'll be validating the whole
+      // subdoc.
+      // The following is a faster take on looping through every path in `paths`
+      // and checking if the path starts with `fullPathToSubdoc` re: gh-13191
+      for (const modifiedPath of subdoc.modifiedPaths()) {
+        paths.delete(fullPathToSubdoc + '.' + modifiedPath);
       }
 
       if (doc.$isModified(fullPathToSubdoc, modifiedPaths) &&
@@ -3344,12 +3345,13 @@ Document.prototype.$__reset = function reset() {
           resetArrays.add(array);
         }
       } else {
-        if (subdoc.$parent() === this) {
+        const parent = subdoc.$parent();
+        if (parent === this) {
           this.$__.activePaths.clearPath(subdoc.$basePath);
-        } else if (subdoc.$parent() != null && subdoc.$parent().$isSubdocument) {
+        } else if (parent != null && parent.$isSubdocument) {
           // If map path underneath subdocument, may end up with a case where
           // map path is modified but parent still needs to be reset. See gh-10295
-          subdoc.$parent().$__reset();
+          parent.$__reset();
         }
       }
     }
@@ -4066,6 +4068,7 @@ function applyGetters(self, json, options) {
     path = paths[i];
 
     const parts = path.split('.');
+
     const plen = parts.length;
     const last = plen - 1;
     let branch = json;

diff --git a/lib/helpers/schema/getPath.js b/lib/helpers/schema/getPath.js
@@ -13,7 +13,6 @@ module.exports = function getPath(schema, path) {
   if (schematype != null) {
     return schematype;
   }
-
   const pieces = path.split('.');
   let cur = '';
   let isArray = false;