Merge pull request #15055 from Automattic/vkarpov15/gh-15029

perf: cache results from getAllSubdocs() on saveOptions, only loop through known subdoc properties
Automattic · Nov 26, 2024 · 862d1a5 · 862d1a5
2 parents 768d460 + 8f774f0
commit 862d1a5
Show file tree

Hide file tree

Showing 6 changed files with 115 additions and 49 deletions.
diff --git a/benchmarks/saveSimple.js b/benchmarks/saveSimple.js
@@ -0,0 +1,57 @@
+'use strict';
+
+const mongoose = require('../');
+
+run().catch(err => {
+  console.error(err);
+  process.exit(-1);
+});
+
+async function run() {
+  await mongoose.connect('mongodb://127.0.0.1:27017/mongoose_benchmark');
+  const FooSchema = new mongoose.Schema({
+    prop1: String,
+    prop2: String,
+    prop3: String,
+    prop4: String,
+    prop5: String,
+    prop6: String,
+    prop7: String,
+    prop8: String,
+    prop9: String,
+    prop10: String
+  });
+  const FooModel = mongoose.model('Foo', FooSchema);
+
+  if (!process.env.MONGOOSE_BENCHMARK_SKIP_SETUP) {
+    await FooModel.deleteMany({});
+  }
+
+  const numIterations = 500;
+  const saveStart = Date.now();
+  for (let i = 0; i < numIterations; ++i) {
+    for (let j = 0; j < 10; ++j) {
+      const doc = new FooModel({
+        prop1: `test ${i}`,
+        prop2: `test ${i}`,
+        prop3: `test ${i}`,
+        prop4: `test ${i}`,
+        prop5: `test ${i}`,
+        prop6: `test ${i}`,
+        prop7: `test ${i}`,
+        prop8: `test ${i}`,
+        prop9: `test ${i}`,
+        prop10: `test ${i}`
+      });
+      await doc.save();
+    }
+  }
+  const saveEnd = Date.now();
+
+  const results = {
+    'Average save time ms': +((saveEnd - saveStart) / numIterations).toFixed(2)
+  };
+
+  console.log(JSON.stringify(results, null, '  '));
+  process.exit(0);
+}
diff --git a/lib/document.js b/lib/document.js
@@ -2711,7 +2711,7 @@ function _getPathsToValidate(doc, pathsToValidate, pathsToSkip, isNestedValidate
 
   if (!isNestedValidate) {
     // If we're validating a subdocument, all this logic will run anyway on the top-level document, so skip for subdocuments
-    const subdocs = doc.$getAllSubdocs();
+    const subdocs = doc.$getAllSubdocs({ useCache: true });
     const modifiedPaths = doc.modifiedPaths();
     for (const subdoc of subdocs) {
       if (subdoc.$basePath) {
@@ -3482,7 +3482,7 @@ Document.prototype.$__reset = function reset() {
   let _this = this;
 
   // Skip for subdocuments
-  const subdocs = !this.$isSubdocument ? this.$getAllSubdocs() : null;
+  const subdocs = !this.$isSubdocument ? this.$getAllSubdocs({ useCache: true }) : null;
   if (subdocs && subdocs.length > 0) {
     for (const subdoc of subdocs) {
       subdoc.$__reset();
@@ -3672,64 +3672,58 @@ Document.prototype.$__getArrayPathsToValidate = function() {
 /**
  * Get all subdocs (by bfs)
  *
+ * @param {Object} [options] options. Currently for internal use.
  * @return {Array}
  * @api public
  * @method $getAllSubdocs
  * @memberOf Document
  * @instance
  */
 
-Document.prototype.$getAllSubdocs = function() {
+Document.prototype.$getAllSubdocs = function(options) {
+  if (options?.useCache && this.$__.saveOptions?.__subdocs) {
+    return this.$__.saveOptions.__subdocs;
+  }
+
   DocumentArray || (DocumentArray = require('./types/documentArray'));
   Embedded = Embedded || require('./types/arraySubdocument');
 
-  function docReducer(doc, seed, path) {
-    let val = doc;
-    let isNested = false;
-    if (path) {
-      if (doc instanceof Document && doc[documentSchemaSymbol].paths[path]) {
-        val = doc._doc[path];
-      } else if (doc instanceof Document && doc[documentSchemaSymbol].nested[path]) {
-        val = doc._doc[path];
-        isNested = true;
-      } else {
-        val = doc[path];
+  const subDocs = [];
+  function getSubdocs(doc) {
+    const newSubdocs = [];
+    for (const { path } of doc.$__schema.childSchemas) {
+      const val = doc.$__getValue(path);
+      if (val == null) {
+        continue;
       }
-    }
-    if (val instanceof Embedded) {
-      seed.push(val);
-    } else if (val instanceof Map) {
-      seed = Array.from(val.keys()).reduce(function(seed, path) {
-        return docReducer(val.get(path), seed, null);
-      }, seed);
-    } else if (val && !Array.isArray(val) && val.$isSingleNested) {
-      seed = Object.keys(val._doc).reduce(function(seed, path) {
-        return docReducer(val, seed, path);
-      }, seed);
-      seed.push(val);
-    } else if (val && utils.isMongooseDocumentArray(val)) {
-      val.forEach(function _docReduce(doc) {
-        if (!doc || !doc._doc) {
-          return;
+      if (val.$__) {
+        newSubdocs.push(val);
+      }
+      if (Array.isArray(val)) {
+        for (const el of val) {
+          if (el != null && el.$__) {
+            newSubdocs.push(el);
+          }
         }
-        seed = Object.keys(doc._doc).reduce(function(seed, path) {
-          return docReducer(doc._doc, seed, path);
-        }, seed);
-        if (doc instanceof Embedded) {
-          seed.push(doc);
+      }
+      if (val instanceof Map) {
+        for (const el of val.values()) {
+          if (el != null && el.$__) {
+            newSubdocs.push(el);
+          }
         }
-      });
-    } else if (isNested && val != null) {
-      for (const path of Object.keys(val)) {
-        docReducer(val, seed, path);
       }
     }
-    return seed;
+    for (const subdoc of newSubdocs) {
+      getSubdocs(subdoc);
+    }
+    subDocs.push(...newSubdocs);
   }
 
-  const subDocs = [];
-  for (const path of Object.keys(this._doc)) {
-    docReducer(this, subDocs, path);
+  getSubdocs(this);
+
+  if (this.$__.saveOptions) {
+    this.$__.saveOptions.__subdocs = subDocs;
   }
 
   return subDocs;

diff --git a/lib/model.js b/lib/model.js
@@ -3146,7 +3146,7 @@ function _setIsNew(doc, val) {
   doc.$emit('isNew', val);
   doc.constructor.emit('isNew', val);
 
-  const subdocs = doc.$getAllSubdocs();
+  const subdocs = doc.$getAllSubdocs({ useCache: true });
   for (const subdoc of subdocs) {
     subdoc.$isNew = val;
     subdoc.$emit('isNew', val);

diff --git a/lib/options/saveOptions.js b/lib/options/saveOptions.js
@@ -11,4 +11,6 @@ class SaveOptions {
   }
 }
 
+SaveOptions.prototype.__subdocs = null;
+
 module.exports = SaveOptions;
diff --git a/lib/plugins/saveSubdocs.js b/lib/plugins/saveSubdocs.js
@@ -15,7 +15,7 @@ module.exports = function saveSubdocs(schema) {
     }
 
     const _this = this;
-    const subdocs = this.$getAllSubdocs();
+    const subdocs = this.$getAllSubdocs({ useCache: true });
 
     if (!subdocs.length) {
       next();
@@ -27,6 +27,8 @@ module.exports = function saveSubdocs(schema) {
         cb(err);
       });
     }, function(error) {
+      // Bust subdocs cache because subdoc pre hooks can add new subdocuments
+      _this.$__.saveOptions.__subdocs = null;
       if (error) {
         return _this.$__schema.s.hooks.execPost('save:error', _this, [_this], { error: error }, function(error) {
           next(error);
@@ -64,7 +66,7 @@ module.exports = function saveSubdocs(schema) {
     }
 
     const _this = this;
-    const subdocs = this.$getAllSubdocs();
+    const subdocs = this.$getAllSubdocs({ useCache: true });
 
     if (!subdocs.length) {
       return;

diff --git a/lib/schema.js b/lib/schema.js
@@ -1126,6 +1126,13 @@ Schema.prototype.path = function(path, obj) {
 
     this.paths[mapPath] = schemaType.$__schemaType;
     this.mapPaths.push(this.paths[mapPath]);
+    if (schemaType.$__schemaType.$isSingleNested) {
+      this.childSchemas.push({
+        schema: schemaType.$__schemaType.schema,
+        model: schemaType.$__schemaType.caster,
+        path: path
+      });
+    }
   }
 
   if (schemaType.$isSingleNested) {
@@ -1154,7 +1161,8 @@ Schema.prototype.path = function(path, obj) {
     schemaType.caster.base = this.base;
     this.childSchemas.push({
       schema: schemaType.schema,
-      model: schemaType.caster
+      model: schemaType.caster,
+      path: path
     });
   } else if (schemaType.$isMongooseDocumentArray) {
     Object.defineProperty(schemaType.schema, 'base', {
@@ -1167,7 +1175,8 @@ Schema.prototype.path = function(path, obj) {
     schemaType.casterConstructor.base = this.base;
     this.childSchemas.push({
       schema: schemaType.schema,
-      model: schemaType.casterConstructor
+      model: schemaType.casterConstructor,
+      path: path
     });
   }
 
@@ -1235,7 +1244,9 @@ function gatherChildSchemas(schema) {
   for (const path of Object.keys(schema.paths)) {
     const schematype = schema.paths[path];
     if (schematype.$isMongooseDocumentArray || schematype.$isSingleNested) {
-      childSchemas.push({ schema: schematype.schema, model: schematype.caster });
+      childSchemas.push({ schema: schematype.schema, model: schematype.caster, path: path });
+    } else if (schematype.$isSchemaMap && schematype.$__schemaType.$isSingleNested) {
+      childSchemas.push({ schema: schematype.$__schemaType.schema, model: schematype.$__schemaType.caster, path: path });
     }
   }
-Original file line number
+Diff line change
@@ Expand Up / @@ -11,4 +11,6 @@ class SaveOptions { @@
       }
     }
+    SaveOptions.prototype.__subdocs = null;
     module.exports = SaveOptions;