From e910cb7c666ed5059c7ffcad91c3c3ababf34e77 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 4 May 2024 11:31:43 -0400 Subject: [PATCH 1/5] perf(document): avoid cloning options using spread operator for perf reasons Re: #14394 --- lib/document.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/document.js b/lib/document.js index 8a9149ebbe9..7000747793d 100644 --- a/lib/document.js +++ b/lib/document.js @@ -3855,7 +3855,11 @@ Document.prototype.$toObject = function(options, json) { } // merge default options with input options. - options = { ...defaultOptions, ...options }; + for (const key of Object.keys(defaultOptions)) { + if (options[key] == null) { + options[key] = defaultOptions[key]; + } + } options._isNested = true; options.json = json; options.minimize = _minimize; From f087e8f62984dae937ec7f45f670cb84c00d3f89 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 4 May 2024 14:39:35 -0400 Subject: [PATCH 2/5] perf(document): remove some more unnecessary cloning in $toObject() re: #14394 --- lib/document.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/document.js b/lib/document.js index 7000747793d..10158246145 100644 --- a/lib/document.js +++ b/lib/document.js @@ -3780,7 +3780,7 @@ Document.prototype.$__handleReject = function handleReject(err) { */ Document.prototype.$toObject = function(options, json) { - let defaultOptions = { + const defaultOptions = { transform: true, flattenDecimals: true }; @@ -3793,7 +3793,7 @@ Document.prototype.$toObject = function(options, json) { const schemaOptions = this.$__schema && this.$__schema.options || {}; // merge base default options with Schema's set default options if available. // `clone` is necessary here because `utils.options` directly modifies the second input. - defaultOptions = { ...defaultOptions, ...baseOptions, ...schemaOptions[path] }; + Object.assign(defaultOptions, baseOptions, schemaOptions[path]); // If options do not exist or is not an object, set it to empty object options = utils.isPOJO(options) ? { ...options } : {}; @@ -3865,19 +3865,17 @@ Document.prototype.$toObject = function(options, json) { options.minimize = _minimize; cloneOptions._parentOptions = options; - cloneOptions._skipSingleNestedGetters = false; - - const gettersOptions = Object.assign({}, cloneOptions); - gettersOptions._skipSingleNestedGetters = true; + cloneOptions._skipSingleNestedGetters = false; // remember the root transform function // to save it from being overwritten by sub-transform functions const originalTransform = options.transform; let ret = clone(this._doc, cloneOptions) || {}; + cloneOptions._skipSingleNestedGetters = true; if (options.getters) { - applyGetters(this, ret, gettersOptions); + applyGetters(this, ret, cloneOptions); if (options.minimize) { ret = minimize(ret) || {}; @@ -3885,7 +3883,7 @@ Document.prototype.$toObject = function(options, json) { } if (options.virtuals || (options.getters && options.virtuals !== false)) { - applyVirtuals(this, ret, gettersOptions, options); + applyVirtuals(this, ret, cloneOptions, options); } if (options.versionKey === false && this.$__schema.options.versionKey) { From abc87b2f299360457056e3eab478fc020e591f3b Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 4 May 2024 16:41:35 -0400 Subject: [PATCH 3/5] remove unnecessary logic --- lib/document.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/document.js b/lib/document.js index 10158246145..db7cb008465 100644 --- a/lib/document.js +++ b/lib/document.js @@ -3839,13 +3839,6 @@ Document.prototype.$toObject = function(options, json) { _seen: (options && options._seen) || new Map() }); - if (utils.hasUserDefinedProperty(options, 'getters')) { - cloneOptions.getters = options.getters; - } - if (utils.hasUserDefinedProperty(options, 'virtuals')) { - cloneOptions.virtuals = options.virtuals; - } - const depopulate = options.depopulate || (options._parentOptions && options._parentOptions.depopulate || false); // _isNested will only be true if this is not the top level document, we From c51fdd8e081cb788e012b988816b2d1a6fe82952 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 6 May 2024 14:24:58 -0400 Subject: [PATCH 4/5] perf(document): avoid copying all properties into cloneOptions, just the necessary ones Re: #14394 --- lib/document.js | 10 +++++++--- test/document.test.js | 7 +------ test/model.populate.test.js | 8 +------- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/lib/document.js b/lib/document.js index db7cb008465..54267d26b9e 100644 --- a/lib/document.js +++ b/lib/document.js @@ -3830,14 +3830,18 @@ Document.prototype.$toObject = function(options, json) { // `clone()` will recursively call `$toObject()` on embedded docs, so we // need the original options the user passed in, plus `_isNested` and // `_parentOptions` for checking whether we need to depopulate. - const cloneOptions = Object.assign({}, options, { + const cloneOptions = { _isNested: true, json: json, minimize: _minimize, flattenMaps: flattenMaps, flattenObjectIds: flattenObjectIds, - _seen: (options && options._seen) || new Map() - }); + _seen: (options && options._seen) || new Map(), + _calledWithOptions: options._calledWithOptions, + virtuals: options.virtuals, + getters: options.getters, + depopulate: options.depopulate + }; const depopulate = options.depopulate || (options._parentOptions && options._parentOptions.depopulate || false); diff --git a/test/document.test.js b/test/document.test.js index a770b3b8352..19f7f3c1f05 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -722,32 +722,27 @@ describe('document', function() { lastName: String, password: String }); - userSchema.virtual('fullName').get(function() { return this.firstName + ' ' + this.lastName; }); - userSchema.set('toObject', { virtuals: false }); const postSchema = new Schema({ owner: { type: Schema.Types.ObjectId, ref: 'User' }, content: String }); - postSchema.virtual('capContent').get(function() { return this.content.toUpperCase(); }); - postSchema.set('toObject', { virtuals: true }); + const User = db.model('User', userSchema); const Post = db.model('BlogPost', postSchema); const user = new User({ firstName: 'Joe', lastName: 'Smith', password: 'password' }); - const savedUser = await user.save(); const post = await Post.create({ owner: savedUser._id, content: 'lorem ipsum' }); - const newPost = await Post.findById(post._id).populate('owner').exec(); const obj = newPost.toObject(); diff --git a/test/model.populate.test.js b/test/model.populate.test.js index a987bd6d4cc..9ad6376f89a 100644 --- a/test/model.populate.test.js +++ b/test/model.populate.test.js @@ -2976,33 +2976,27 @@ describe('model: populate:', function() { return ret; } }); - const Team = db.model('Test', teamSchema); const userSchema = new Schema({ username: String }); - userSchema.set('toJSON', { transform: function(doc, ret) { return ret; } }); - const User = db.model('User', userSchema); const user = new User({ username: 'Test' }); - await user.save(); const team = new Team({ members: [{ user: user }] }); - await team.save(); - await team.populate('members.user'); + assert.equal(calls, 0); team.toJSON(); - assert.equal(calls, 1); }); From 31596d558231038755748606c9ec74857219426e Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 6 May 2024 14:27:47 -0400 Subject: [PATCH 5/5] style: fix lint --- test/document.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/document.test.js b/test/document.test.js index 19f7f3c1f05..b8030900f9b 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -735,7 +735,7 @@ describe('document', function() { return this.content.toUpperCase(); }); postSchema.set('toObject', { virtuals: true }); - + const User = db.model('User', userSchema); const Post = db.model('BlogPost', postSchema);