Skip to content

Commit

Permalink
Merge pull request #11414 from Uzlopak/perf-invalid-path
Browse files Browse the repository at this point in the history
perf: improve validation sync and async by replacing forEach with classic for loops
  • Loading branch information
vkarpov15 authored Feb 17, 2022
2 parents d8dc68d + 576dbec commit a499c91
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
14 changes: 8 additions & 6 deletions lib/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -2853,19 +2853,21 @@ Document.prototype.validateSync = function(pathsToValidate, options) {
}
const validating = {};

paths.forEach(function(path) {
for (let i = 0, len = paths.length; i < len; ++i) {

This comment has been minimized.

Copy link
@AbdelrahmanHafez

AbdelrahmanHafez Feb 18, 2022

Collaborator

@Uzlopak
I've always found that for (const path of paths) { ... } more readable.
Do you happen to know if there is a performance advantage of using for loop with incrementing a variable over for-of?

This comment has been minimized.

Copy link
@vkarpov15

vkarpov15 Feb 23, 2022

Author Collaborator

I agree, my recommendation is always use for/of unless you have a good reason not to. See https://masteringjs.io/tutorials/fundamentals/array-iterate . But if vanilla for loop has better perf, I'm open to using it.

This comment has been minimized.

Copy link
@Uzlopak

Uzlopak Feb 23, 2022

Collaborator

I will try to make in the future more perf tests, when it is about using classical for vs. for of. promise ;)

const path = paths[i];

if (validating[path]) {
return;
continue;
}

validating[path] = true;

const p = _this.$__schema.path(path);
if (!p) {
return;
continue;
}
if (!_this.$isValid(path)) {
return;
continue;
}

const val = _this.$__getValue(path);
Expand All @@ -2879,11 +2881,11 @@ Document.prototype.validateSync = function(pathsToValidate, options) {
p.$isArraySubdocument ||
p.$isMongooseDocumentArray;
if (isSubdoc && err instanceof ValidationError) {
return;
continue;
}
_this.invalidate(path, err, undefined, true);
}
});
}

const err = _this.$__.validationError;
_this.$__.validationError = undefined;
Expand Down
17 changes: 8 additions & 9 deletions lib/schematype.js
Original file line number Diff line number Diff line change
Expand Up @@ -1250,10 +1250,10 @@ SchemaType.prototype.doValidate = function(value, fn, scope, options) {
return fn(null);
}

const _this = this;
validators.forEach(function(v) {
for (let i = 0, len = validators.length; i < len; ++i) {
const v = validators[i];
if (err) {
return;
break;
}

const validator = v.validator;
Expand All @@ -1265,16 +1265,16 @@ SchemaType.prototype.doValidate = function(value, fn, scope, options) {

if (validator instanceof RegExp) {
validate(validator.test(value), validatorProperties);
return;
continue;
}

if (typeof validator !== 'function') {
return;
continue;
}

if (value === undefined && validator !== _this.requiredValidator) {
if (value === undefined && validator !== this.requiredValidator) {
validate(true, validatorProperties);
return;
continue;
}

try {
Expand Down Expand Up @@ -1303,8 +1303,7 @@ SchemaType.prototype.doValidate = function(value, fn, scope, options) {
} else {
validate(ok, validatorProperties);
}

});
}

function validate(ok, validatorProperties) {
if (err) {
Expand Down

0 comments on commit a499c91

Please sign in to comment.