Merge pull request #13614 from Automattic/vkarpov15/gh-13191-2

perf: speed up mapOfSubdocs benchmark by 4x by avoiding unnecessary O(n^2) loop in getPathsToValidate()

Author: vkarpov15

lib/document.js

@@ -1112,9 +1112,11 @@ Document.prototype.$set = function $set(path, val, type, options) {
       return this;
     }
 
+    options = Object.assign({}, options, { _skipMinimizeTopLevel: false });
+
     for (let i = 0; i < len; ++i) {
       key = keys[i];
-      const pathName = prefix + key;
+      const pathName = prefix ? prefix + key : key;
       pathtype = this.$__schema.pathType(pathName);
       const valForKey = path[key];
 
@@ -1126,20 +1128,15 @@ Document.prototype.$set = function $set(path, val, type, options) {
           pathtype === 'nested' &&
           this._doc[key] != null) {
         delete this._doc[key];
-        // Make sure we set `{}` back even if we minimize re: gh-8565
-        options = Object.assign({}, options, { _skipMinimizeTopLevel: true });
-      } else {
-        // Make sure we set `{_skipMinimizeTopLevel: false}` if don't have overwrite: gh-10441
-        options = Object.assign({}, options, { _skipMinimizeTopLevel: false });
       }
 
       if (utils.isNonBuiltinObject(valForKey) && pathtype === 'nested') {
-        this.$set(prefix + key, path[key], constructing, Object.assign({}, options, { _skipMarkModified: true }));
-        $applyDefaultsToNested(this.$get(prefix + key), prefix + key, this);
+        this.$set(pathName, valForKey, constructing, Object.assign({}, options, { _skipMarkModified: true }));
+        $applyDefaultsToNested(this.$get(pathName), pathName, this);
         continue;
       } else if (strict) {
         // Don't overwrite defaults with undefined keys (gh-3981) (gh-9039)
-        if (constructing && path[key] === void 0 &&
+        if (constructing && valForKey === void 0 &&
             this.$get(pathName) !== void 0) {
           continue;
         }
@@ -1149,20 +1146,19 @@ Document.prototype.$set = function $set(path, val, type, options) {
         }
 
         if (pathtype === 'real' || pathtype === 'virtual') {
-          const p = path[key];
-          this.$set(prefix + key, p, constructing, options);
-        } else if (pathtype === 'nested' && path[key] instanceof Document) {
-          this.$set(prefix + key,
-            path[key].toObject({ transform: false }), constructing, options);
+          this.$set(pathName, valForKey, constructing, options);
+        } else if (pathtype === 'nested' && valForKey instanceof Document) {
+          this.$set(pathName,
+            valForKey.toObject({ transform: false }), constructing, options);
         } else if (strict === 'throw') {
           if (pathtype === 'nested') {
-            throw new ObjectExpectedError(key, path[key]);
+            throw new ObjectExpectedError(key, valForKey);
           } else {
             throw new StrictModeError(key);
           }
         }
-      } else if (path[key] !== void 0) {
-        this.$set(prefix + key, path[key], constructing, options);
+      } else if (valForKey !== void 0) {
+        this.$set(pathName, valForKey, constructing, options);
       }
     }
 
@@ -2225,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;
       }
     }
@@ -2668,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) &&
@@ -3348,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();
         }
       }
     }
@@ -4070,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;

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;

lib/schema.js

@@ -1530,9 +1530,6 @@ Schema.prototype.indexedPaths = function indexedPaths() {
  */
 
 Schema.prototype.pathType = function(path) {
-  // Convert to '.$' to check subpaths re: gh-6405
-  const cleanPath = _pathToPositionalSyntax(path);
-
   if (this.paths.hasOwnProperty(path)) {
     return 'real';
   }
@@ -1542,6 +1539,10 @@ Schema.prototype.pathType = function(path) {
   if (this.nested.hasOwnProperty(path)) {
     return 'nested';
   }
+
+  // Convert to '.$' to check subpaths re: gh-6405
+  const cleanPath = _pathToPositionalSyntax(path);
+
   if (this.subpaths.hasOwnProperty(cleanPath) || this.subpaths.hasOwnProperty(path)) {
     return 'real';
   }

lib/schema/SubdocumentPath.js

@@ -160,22 +160,22 @@ SubdocumentPath.prototype.cast = function(val, doc, init, priorVal, options) {
   let subdoc;
 
   // Only pull relevant selected paths and pull out the base path
-  const parentSelected = doc && doc.$__ && doc.$__.selected || {};
+  const parentSelected = doc && doc.$__ && doc.$__.selected;
   const path = this.path;
-  const selected = Object.keys(parentSelected).reduce((obj, key) => {
+  const selected = parentSelected == null ? null : Object.keys(parentSelected).reduce((obj, key) => {
     if (key.startsWith(path + '.')) {
       obj = obj || {};
       obj[key.substring(path.length + 1)] = parentSelected[key];
     }
     return obj;
   }, null);
-  options = Object.assign({}, options, { priorDoc: priorVal });
   if (init) {
     subdoc = new Constructor(void 0, selected, doc, false, { defaults: false });
     delete subdoc.$__.defaults;
     subdoc.$init(val);
     applyDefaults(subdoc, selected);
   } else {
+    options = Object.assign({}, options, { priorDoc: priorVal });
     if (Object.keys(val).length === 0) {
       return new Constructor({}, selected, doc, undefined, options);
     }