Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: make a few micro-optimizations to help speed up findOne() #15022

Merged
merged 3 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions lib/helpers/isBsonType.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@

function isBsonType(obj, typename) {
return (
typeof obj === 'object' &&
obj !== null &&
obj != null &&
obj._bsontype === typename
);
}
Expand Down
4 changes: 1 addition & 3 deletions lib/helpers/schema/applyReadConcern.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
'use strict';

const get = require('../get');

module.exports = function applyReadConcern(schema, options) {
if (options.readConcern !== undefined) {
return;
Expand All @@ -15,7 +13,7 @@ module.exports = function applyReadConcern(schema, options) {
return;
}

const level = get(schema, 'options.readConcern.level', null);
const level = schema.options?.readConcern?.level;
if (level != null) {
options.readConcern = { level };
}
Expand Down
4 changes: 1 addition & 3 deletions lib/helpers/schema/applyWriteConcern.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
'use strict';

const get = require('../get');

module.exports = function applyWriteConcern(schema, options) {
if (options.writeConcern != null) {
return;
Expand All @@ -12,7 +10,7 @@ module.exports = function applyWriteConcern(schema, options) {
if (options && options.session && options.session.transaction) {
return;
}
const writeConcern = get(schema, 'options.writeConcern', {});
const writeConcern = schema.options.writeConcern ?? {};
if (Object.keys(writeConcern).length != 0) {
options.writeConcern = {};
if (!('w' in options) && writeConcern.w != null) {
Expand Down
25 changes: 13 additions & 12 deletions lib/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const castUpdate = require('./helpers/query/castUpdate');
const clone = require('./helpers/clone');
const getDiscriminatorByValue = require('./helpers/discriminator/getDiscriminatorByValue');
const helpers = require('./queryHelpers');
const immediate = require('./helpers/immediate');
const internalToObjectOptions = require('./options').internalToObjectOptions;
const isExclusive = require('./helpers/projection/isExclusive');
const isInclusive = require('./helpers/projection/isInclusive');
Expand Down Expand Up @@ -2248,6 +2247,9 @@ Query.prototype.error = function error(err) {
*/

Query.prototype._unsetCastError = function _unsetCastError() {
if (this._error == null) {
return;
}
if (this._error != null && !(this._error instanceof CastError)) {
Copy link

@BenaliDjamel BenaliDjamel Dec 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vkarpov15 Since you already checked (this._error == null) above, you can simplify the condition as below.
if (!(this._error instanceof CastError)) {}

return;
}
Expand Down Expand Up @@ -2291,9 +2293,9 @@ Query.prototype.mongooseOptions = function(v) {

Query.prototype._castConditions = function() {
let sanitizeFilterOpt = undefined;
if (this.model != null && utils.hasUserDefinedProperty(this.model.db.options, 'sanitizeFilter')) {
if (this.model != null && this.model.db.options?.sanitizeFilter != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can also be shortened to:

Suggested change
if (this.model != null && this.model.db.options?.sanitizeFilter != null) {
if (this.model?.db.options?.sanitizeFilter != null) {

sanitizeFilterOpt = this.model.db.options.sanitizeFilter;
} else if (this.model != null && utils.hasUserDefinedProperty(this.model.base.options, 'sanitizeFilter')) {
} else if (this.model != null && this.model.base.options?.sanitizeFilter != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Suggested change
} else if (this.model != null && this.model.base.options?.sanitizeFilter != null) {
} else if (this.model?.base.options?.sanitizeFilter != null) {

sanitizeFilterOpt = this.model.base.options.sanitizeFilter;
} else {
sanitizeFilterOpt = this._mongooseOptions.sanitizeFilter;
Expand Down Expand Up @@ -2536,13 +2538,12 @@ Query.prototype.collation = function(value) {
* @api private
*/

Query.prototype._completeOne = function(doc, res, callback) {
Query.prototype._completeOne = function(doc, res, projection, callback) {
if (!doc && !this.options.includeResultMetadata) {
return callback(null, null);
}

const model = this.model;
const projection = clone(this._fields);
const userProvidedFields = this._userProvidedFields || {};
// `populate`, `lean`
const mongooseOptions = this._mongooseOptions;
Expand Down Expand Up @@ -2643,7 +2644,7 @@ Query.prototype._findOne = async function _findOne() {
// don't pass in the conditions because we already merged them in
const doc = await this.mongooseCollection.findOne(this._conditions, options);
return new Promise((resolve, reject) => {
this._completeOne(doc, null, (err, res) => {
this._completeOne(doc, null, options.projection, (err, res) => {
if (err) {
return reject(err);
}
Expand Down Expand Up @@ -3238,7 +3239,7 @@ function completeOne(model, doc, res, options, fields, userProvidedFields, pop,

function _init(err, casted) {
if (err) {
return immediate(() => callback(err));
return callback(err);
}


Expand All @@ -3251,12 +3252,12 @@ function completeOne(model, doc, res, options, fields, userProvidedFields, pop,
} else {
res.value = null;
}
return immediate(() => callback(null, res));
return callback(null, res);
}
if (options.session != null) {
casted.$session(options.session);
}
immediate(() => callback(null, casted));
callback(null, casted);
}
}

Expand Down Expand Up @@ -3465,7 +3466,7 @@ Query.prototype._findOneAndUpdate = async function _findOneAndUpdate() {
const doc = !options.includeResultMetadata ? res : res.value;

return new Promise((resolve, reject) => {
this._completeOne(doc, res, (err, res) => {
this._completeOne(doc, res, options.projection, (err, res) => {
if (err) {
return reject(err);
}
Expand Down Expand Up @@ -3561,7 +3562,7 @@ Query.prototype._findOneAndDelete = async function _findOneAndDelete() {
const doc = !includeResultMetadata ? res : res.value;

return new Promise((resolve, reject) => {
this._completeOne(doc, res, (err, res) => {
this._completeOne(doc, res, options.projection, (err, res) => {
if (err) {
return reject(err);
}
Expand Down Expand Up @@ -3715,7 +3716,7 @@ Query.prototype._findOneAndReplace = async function _findOneAndReplace() {

const doc = !includeResultMetadata ? res : res.value;
return new Promise((resolve, reject) => {
this._completeOne(doc, res, (err, res) => {
this._completeOne(doc, res, options.projection, (err, res) => {
if (err) {
return reject(err);
}
Expand Down