Merge pull request #11988 from Automattic/vkarpov15/gh-11541

perf(document): avoid creating unnecessary empty objects when creating a state machine
Automattic · Jul 18, 2022 · 871a121 · 871a121
2 parents 7187253 + fa29f3a
commit 871a121
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 53 deletions.
diff --git a/lib/connection.js b/lib/connection.js
@@ -491,6 +491,9 @@ Connection.prototype.transaction = function transaction(fn, options) {
             doc.set(doc.schema.options.versionKey, state.versionKey);
           }
 
+          if (state.modifiedPaths.length > 0 && doc.$__.activePaths.states.modify == null) {
+            doc.$__.activePaths.states.modify = {};
+          }
           for (const path of state.modifiedPaths) {
             doc.$__.activePaths.paths[path] = 'modify';
             doc.$__.activePaths.states.modify[path] = true;

diff --git a/lib/document.js b/lib/document.js
@@ -93,7 +93,11 @@ function Document(obj, fields, skipId, options) {
   }
 
   this.$__ = new InternalCache();
-  this.$isNew = 'isNew' in options ? options.isNew : true;
+
+  // Avoid setting `isNew` to `true`, because it is `true` by default
+  if (options.isNew != null && options.isNew !== true) {
+    this.$isNew = options.isNew;
+  }
 
   if (options.priorDoc != null) {
     this.$__.priorDoc = options.priorDoc;
@@ -116,13 +120,12 @@ function Document(obj, fields, skipId, options) {
   const schema = this.$__schema;
 
   if (typeof fields === 'boolean' || fields === 'throw') {
-    this.$__.strictMode = fields;
+    if (fields !== true) {
+      this.$__.strictMode = fields;
+    }
     fields = undefined;
-  } else {
+  } else if (schema.options.strict !== true) {
     this.$__.strictMode = schema.options.strict;
-    if (fields != null) {
-      this.$__.selected = fields;
-    }
   }
 
   const requiredPaths = schema.requiredPaths(true);
@@ -134,9 +137,9 @@ function Document(obj, fields, skipId, options) {
 
   // determine if this doc is a result of a query with
   // excluded fields
-  if (utils.isPOJO(fields)) {
+  if (utils.isPOJO(fields) && Object.keys(fields).length > 0) {
     exclude = isExclusive(fields);
-    this.$__.fields = fields;
+    this.$__.selected = fields;
     this.$__.exclude = exclude;
   }
 
@@ -210,6 +213,13 @@ Object.defineProperty(Document.prototype, 'errors', {
     this.$errors = value;
   }
 });
+
+/*!
+ * ignore
+ */
+
+Document.prototype.$isNew = true;
+
 /*!
  * Document exposes the NodeJS event emitter API, so you can use
  * `on`, `once`, etc.
@@ -773,10 +783,10 @@ Document.prototype.$__init = function(doc, opts) {
   this.$emit('init', this);
   this.constructor.emit('init', this);
 
-  const hasIncludedChildren = this.$__.exclude === false && this.$__.fields ?
-    $__hasIncludedChildren(this.$__.fields) :
+  const hasIncludedChildren = this.$__.exclude === false && this.$__.selected ?
+    $__hasIncludedChildren(this.$__.selected) :
     null;
-  $__applyDefaults(this, this.$__.fields, this.$__.exclude, hasIncludedChildren, false, this.$__.skipDefaults);
+  $__applyDefaults(this, this.$__.selected, this.$__.exclude, hasIncludedChildren, false, this.$__.skipDefaults);
 
   return this;
 };
@@ -1620,7 +1630,7 @@ Document.prototype.$__shouldModify = function(pathToMark, path, options, constru
     return true;
   }
 
-  if (val === void 0 && path in this.$__.activePaths.states.default) {
+  if (val === void 0 && path in this.$__.activePaths.getStatePaths('default')) {
     // we're just unsetting the default value which was never saved
     return false;
   }
@@ -1640,7 +1650,7 @@ Document.prototype.$__shouldModify = function(pathToMark, path, options, constru
   if (!constructing &&
       val !== null &&
       val !== undefined &&
-      path in this.$__.activePaths.states.default &&
+      path in this.$__.activePaths.getStatePaths('default') &&
       deepEqual(val, schema.getDefault(this, constructing))) {
     // a path with a default was $unset on the server
     // and the user is setting it to the same value again
@@ -1938,7 +1948,7 @@ Document.prototype.$ignore = function(path) {
  */
 
 Document.prototype.directModifiedPaths = function() {
-  return Object.keys(this.$__.activePaths.states.modify);
+  return Object.keys(this.$__.activePaths.getStatePaths('modify'));
 };
 
 /**
@@ -2016,7 +2026,7 @@ function _isEmpty(v) {
 Document.prototype.modifiedPaths = function(options) {
   options = options || {};
 
-  const directModifiedPaths = Object.keys(this.$__.activePaths.states.modify);
+  const directModifiedPaths = Object.keys(this.$__.activePaths.getStatePaths('modify'));
   const result = new Set();
 
   let i = 0;
@@ -2095,7 +2105,7 @@ Document.prototype[documentModifiedPaths] = Document.prototype.modifiedPaths;
 
 Document.prototype.isModified = function(paths, modifiedPaths) {
   if (paths) {
-    const directModifiedPaths = Object.keys(this.$__.activePaths.states.modify);
+    const directModifiedPaths = Object.keys(this.$__.activePaths.getStatePaths('modify'));
     if (directModifiedPaths.length === 0) {
       return false;
     }
@@ -2145,15 +2155,15 @@ Document.prototype.$isDefault = function(path) {
   }
 
   if (typeof path === 'string' && path.indexOf(' ') === -1) {
-    return this.$__.activePaths.states.default.hasOwnProperty(path);
+    return this.$__.activePaths.getStatePaths('default').hasOwnProperty(path);
   }
 
   let paths = path;
   if (!Array.isArray(paths)) {
     paths = paths.split(' ');
   }
 
-  return paths.some(path => this.$__.activePaths.states.default.hasOwnProperty(path));
+  return paths.some(path => this.$__.activePaths.getStatePaths('default').hasOwnProperty(path));
 };
 
 /**
@@ -2206,15 +2216,15 @@ Document.prototype.isDirectModified = function(path) {
   }
 
   if (typeof path === 'string' && path.indexOf(' ') === -1) {
-    return this.$__.activePaths.states.modify.hasOwnProperty(path);
+    return this.$__.activePaths.getStatePaths('modify').hasOwnProperty(path);
   }
 
   let paths = path;
   if (!Array.isArray(paths)) {
     paths = paths.split(' ');
   }
 
-  return paths.some(path => this.$__.activePaths.states.modify.hasOwnProperty(path));
+  return paths.some(path => this.$__.activePaths.getStatePaths('modify').hasOwnProperty(path));
 };
 
 /**
@@ -2231,15 +2241,15 @@ Document.prototype.isInit = function(path) {
   }
 
   if (typeof path === 'string' && path.indexOf(' ') === -1) {
-    return this.$__.activePaths.states.init.hasOwnProperty(path);
+    return this.$__.activePaths.getStatePaths('init').hasOwnProperty(path);
   }
 
   let paths = path;
   if (!Array.isArray(paths)) {
     paths = paths.split(' ');
   }
 
-  return paths.some(path => this.$__.activePaths.states.init.hasOwnProperty(path));
+  return paths.some(path => this.$__.activePaths.getStatePaths('init').hasOwnProperty(path));
 };
 
 /**
@@ -2467,7 +2477,7 @@ Document.prototype.$validate = Document.prototype.validate;
  */
 
 function _evaluateRequiredFunctions(doc) {
-  const requiredFields = Object.keys(doc.$__.activePaths.states.require);
+  const requiredFields = Object.keys(doc.$__.activePaths.getStatePaths('require'));
   let i = 0;
   const len = requiredFields.length;
   for (i = 0; i < len; ++i) {
@@ -2495,7 +2505,7 @@ function _getPathsToValidate(doc) {
 
   _evaluateRequiredFunctions(doc);
   // only validate required fields when necessary
-  let paths = new Set(Object.keys(doc.$__.activePaths.states.require).filter(function(path) {
+  let paths = new Set(Object.keys(doc.$__.activePaths.getStatePaths('require')).filter(function(path) {
     if (!doc.$__isSelected(path) && !doc.$isModified(path)) {
       return false;
     }
@@ -2505,9 +2515,9 @@ function _getPathsToValidate(doc) {
     return true;
   }));
 
-  Object.keys(doc.$__.activePaths.states.init).forEach(addToPaths);
-  Object.keys(doc.$__.activePaths.states.modify).forEach(addToPaths);
-  Object.keys(doc.$__.activePaths.states.default).forEach(addToPaths);
+  Object.keys(doc.$__.activePaths.getStatePaths('init')).forEach(addToPaths);
+  Object.keys(doc.$__.activePaths.getStatePaths('modify')).forEach(addToPaths);
+  Object.keys(doc.$__.activePaths.getStatePaths('default')).forEach(addToPaths);
   function addToPaths(p) { paths.add(p); }
 
   const subdocs = doc.$getAllSubdocs();
@@ -3229,8 +3239,8 @@ Document.prototype.$__reset = function reset() {
 
   this.$__.backup = {};
   this.$__.backup.activePaths = {
-    modify: Object.assign({}, this.$__.activePaths.states.modify),
-    default: Object.assign({}, this.$__.activePaths.states.default)
+    modify: Object.assign({}, this.$__.activePaths.getStatePaths('modify')),
+    default: Object.assign({}, this.$__.activePaths.getStatePaths('default'))
   };
   this.$__.backup.validationError = this.$__.validationError;
   this.$__.backup.errors = this.$errors;

diff --git a/lib/helpers/document/cleanModifiedSubpaths.js b/lib/helpers/document/cleanModifiedSubpaths.js
@@ -13,21 +13,21 @@ module.exports = function cleanModifiedSubpaths(doc, path, options) {
     return deleted;
   }
 
-  for (const modifiedPath of Object.keys(doc.$__.activePaths.states.modify)) {
+  for (const modifiedPath of Object.keys(doc.$__.activePaths.getStatePaths('modify'))) {
     if (skipDocArrays) {
       const schemaType = doc.$__schema.path(modifiedPath);
       if (schemaType && schemaType.$isMongooseDocumentArray) {
         continue;
       }
     }
     if (modifiedPath.startsWith(path + '.')) {
-      delete doc.$__.activePaths.states.modify[modifiedPath];
+      doc.$__.activePaths.clearPath(modifiedPath);
       ++deleted;
 
       if (doc.$isSubdocument) {
         const owner = doc.ownerDocument();
         const fullPath = doc.$__fullPath(modifiedPath);
-        delete owner.$__.activePaths.states.modify[fullPath];
+        owner.$__.activePaths.clearPath(fullPath);
       }
     }
   }

diff --git a/lib/helpers/document/compile.js b/lib/helpers/document/compile.js
@@ -17,6 +17,13 @@ const isPOJO = utils.isPOJO;
 exports.compile = compile;
 exports.defineKey = defineKey;
 
+const _isEmptyOptions = Object.freeze({
+  minimize: true,
+  virtuals: false,
+  getters: false,
+  transform: false
+});
+
 /*!
  * Compiles schemas.
  */
@@ -130,12 +137,6 @@ function defineKey({ prop, subprops, prototype, prefix, options }) {
             value: true
           });
 
-          const _isEmptyOptions = Object.freeze({
-            minimize: true,
-            virtuals: false,
-            getters: false,
-            transform: false
-          });
           Object.defineProperty(nested, '$isEmpty', {
             enumerable: false,
             configurable: true,

diff --git a/lib/internal.js b/lib/internal.js
@@ -13,8 +13,9 @@ function InternalCache() {
   this.activePaths = new ActiveRoster();
 }
 
+InternalCache.prototype.strictMode = true;
+
 InternalCache.prototype.fullPath = undefined;
-InternalCache.prototype.strictMode = undefined;
 InternalCache.prototype.selected = undefined;
 InternalCache.prototype.shardval = undefined;
 InternalCache.prototype.saveError = undefined;

diff --git a/lib/plugins/trackTransaction.js b/lib/plugins/trackTransaction.js
@@ -23,14 +23,14 @@ module.exports = function trackTransaction(schema) {
         initialState.versionKey = this.get(this.$__schema.options.versionKey);
       }
 
-      initialState.modifiedPaths = new Set(Object.keys(this.$__.activePaths.states.modify));
+      initialState.modifiedPaths = new Set(Object.keys(this.$__.activePaths.getStatePaths('modify')));
       initialState.atomics = _getAtomics(this);
 
       session[sessionNewDocuments].set(this, initialState);
     } else {
       const state = session[sessionNewDocuments].get(this);
 
-      for (const path of Object.keys(this.$__.activePaths.states.modify)) {
+      for (const path of Object.keys(this.$__.activePaths.getStatePaths('modify'))) {
         state.modifiedPaths.add(path);
       }
       state.atomics = _getAtomics(this, state.atomics);

diff --git a/lib/schema/documentarray.js b/lib/schema/documentarray.js
@@ -36,6 +36,13 @@ let Subdocument;
  */
 
 function DocumentArrayPath(key, schema, options, schemaOptions) {
+  const schemaTypeIdOption = DocumentArrayPath.defaultOptions &&
+    DocumentArrayPath.defaultOptions._id;
+  if (schemaTypeIdOption != null) {
+    schemaOptions = schemaOptions || {};
+    schemaOptions._id = schemaTypeIdOption;
+  }
+
   if (schemaOptions != null && schemaOptions._id != null) {
     schema = handleIdOption(schema, schemaOptions);
   } else if (options != null && options._id != null) {

diff --git a/lib/statemachine.js b/lib/statemachine.js
@@ -38,19 +38,12 @@ StateMachine.ctor = function() {
     StateMachine.apply(this, arguments);
     this.paths = {};
     this.states = {};
-    this.stateNames = states;
-
-    let i = states.length,
-        state;
-
-    while (i--) {
-      state = states[i];
-      this.states[state] = {};
-    }
   };
 
   ctor.prototype = new StateMachine();
 
+  ctor.prototype.stateNames = states;
+
   states.forEach(function(state) {
     // Changes the `path`'s state to `state`.
     ctor.prototype[state] = function(path) {
@@ -76,6 +69,7 @@ StateMachine.prototype._changeState = function _changeState(path, nextState) {
   if (prevBucket) delete prevBucket[path];
 
   this.paths[path] = nextState;
+  this.states[nextState] = this.states[nextState] || {};
   this.states[nextState][path] = true;
 };
 
@@ -84,6 +78,9 @@ StateMachine.prototype._changeState = function _changeState(path, nextState) {
  */
 
 StateMachine.prototype.clear = function clear(state) {
+  if (this.states[state] == null) {
+    return;
+  }
   const keys = Object.keys(this.states[state]);
   let i = keys.length;
   let path;
@@ -108,6 +105,17 @@ StateMachine.prototype.clearPath = function clearPath(path) {
   delete this.states[state][path];
 };
 
+/*!
+ * Gets the paths for the given state, or empty object `{}` if none.
+ */
+
+StateMachine.prototype.getStatePaths = function getStatePaths(state) {
+  if (this.states[state] != null) {
+    return this.states[state];
+  }
+  return {};
+};
+
 /*!
  * Checks to see if at least one path is in the states passed in via `arguments`
  * e.g., this.some('required', 'inited')
@@ -120,6 +128,9 @@ StateMachine.prototype.some = function some() {
   const _this = this;
   const what = arguments.length ? arguments : this.stateNames;
   return Array.prototype.some.call(what, function(state) {
+    if (_this.states[state] == null) {
+      return false;
+    }
     return Object.keys(_this.states[state]).length;
   });
 };
@@ -143,6 +154,9 @@ StateMachine.prototype._iter = function _iter(iterMethod) {
     const _this = this;
 
     const paths = states.reduce(function(paths, state) {
+      if (_this.states[state] == null) {
+        return paths;
+      }
       return paths.concat(Object.keys(_this.states[state]));
     }, []);