Skip to content

Commit

Permalink
Merge pull request #13614 from Automattic/vkarpov15/gh-13191-2
Browse files Browse the repository at this point in the history 
perf: speed up mapOfSubdocs benchmark by 4x by avoiding unnecessary O(n^2) loop in getPathsToValidate()
  • Loading branch information
@vkarpov15
vkarpov15 committed Jul 17, 2023
2 parents e9eb8ab + b8ebe80 commit 895bc32
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 38 deletions.
61 changes: 30 additions & 31 deletions lib/document.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1112,9 +1112,11 @@ Document.prototype.$set = function $set(path, val, type, options) {
return this;
}

options = Object.assign({}, options, { _skipMinimizeTopLevel: false });

for (let i = 0; i < len; ++i) {
key = keys[i];
const pathName = prefix + key;
const pathName = prefix ? prefix + key : key;
pathtype = this.$__schema.pathType(pathName);
const valForKey = path[key];

Expand All @@ -1126,20 +1128,15 @@ Document.prototype.$set = function $set(path, val, type, options) {
pathtype === 'nested' &&
this._doc[key] != null) {
delete this._doc[key];
// Make sure we set `{}` back even if we minimize re: gh-8565
options = Object.assign({}, options, { _skipMinimizeTopLevel: true });
} else {
// Make sure we set `{_skipMinimizeTopLevel: false}` if don't have overwrite: gh-10441
options = Object.assign({}, options, { _skipMinimizeTopLevel: false });
}

if (utils.isNonBuiltinObject(valForKey) && pathtype === 'nested') {
this.$set(prefix + key, path[key], constructing, Object.assign({}, options, { _skipMarkModified: true }));
$applyDefaultsToNested(this.$get(prefix + key), prefix + key, this);
this.$set(pathName, valForKey, constructing, Object.assign({}, options, { _skipMarkModified: true }));
$applyDefaultsToNested(this.$get(pathName), pathName, this);
continue;
} else if (strict) {
// Don't overwrite defaults with undefined keys (gh-3981) (gh-9039)
if (constructing && path[key] === void 0 &&
if (constructing && valForKey === void 0 &&
this.$get(pathName) !== void 0) {
continue;
}
Expand All @@ -1149,20 +1146,19 @@ Document.prototype.$set = function $set(path, val, type, options) {
}

if (pathtype === 'real' || pathtype === 'virtual') {
const p = path[key];
this.$set(prefix + key, p, constructing, options);
} else if (pathtype === 'nested' && path[key] instanceof Document) {
this.$set(prefix + key,
path[key].toObject({ transform: false }), constructing, options);
this.$set(pathName, valForKey, constructing, options);
} else if (pathtype === 'nested' && valForKey instanceof Document) {
this.$set(pathName,
valForKey.toObject({ transform: false }), constructing, options);
} else if (strict === 'throw') {
if (pathtype === 'nested') {
throw new ObjectExpectedError(key, path[key]);
throw new ObjectExpectedError(key, valForKey);
} else {
throw new StrictModeError(key);
}
}
} else if (path[key] !== void 0) {
this.$set(prefix + key, path[key], constructing, options);
} else if (valForKey !== void 0) {
this.$set(pathName, valForKey, constructing, options);
}
}

Expand Down Expand Up @@ -2225,16 +2221,17 @@ Document.prototype[documentModifiedPaths] = Document.prototype.modifiedPaths;

Document.prototype.isModified = function(paths, modifiedPaths) {
if (paths) {
if (!Array.isArray(paths)) {
paths = paths.indexOf(' ') === -1 ? [paths] : paths.split(' ');
}

const directModifiedPathsObj = this.$__.activePaths.states.modify;
if (directModifiedPathsObj == null) {
return false;
}

if (typeof paths === 'string') {
paths = [paths];
}

for (const path of paths) {
if (Object.prototype.hasOwnProperty.call(directModifiedPathsObj, path)) {
if (directModifiedPathsObj[path] != null) {
return true;
}
}
Expand Down Expand Up @@ -2668,14 +2665,14 @@ function _getPathsToValidate(doc) {
const modifiedPaths = doc.modifiedPaths();
for (const subdoc of subdocs) {
if (subdoc.$basePath) {
// Remove child paths for now, because we'll be validating the whole
// subdoc
const fullPathToSubdoc = subdoc.$__fullPathWithIndexes();

for (const p of paths) {
if (p == null || p.startsWith(fullPathToSubdoc + '.')) {
paths.delete(p);
}
// Remove child paths for now, because we'll be validating the whole
// subdoc.
// The following is a faster take on looping through every path in `paths`
// and checking if the path starts with `fullPathToSubdoc` re: gh-13191
for (const modifiedPath of subdoc.modifiedPaths()) {
paths.delete(fullPathToSubdoc + '.' + modifiedPath);
}

if (doc.$isModified(fullPathToSubdoc, modifiedPaths) &&
Expand Down Expand Up @@ -3348,12 +3345,13 @@ Document.prototype.$__reset = function reset() {
resetArrays.add(array);
}
} else {
if (subdoc.$parent() === this) {
const parent = subdoc.$parent();
if (parent === this) {
this.$__.activePaths.clearPath(subdoc.$basePath);
} else if (subdoc.$parent() != null && subdoc.$parent().$isSubdocument) {
} else if (parent != null && parent.$isSubdocument) {
// If map path underneath subdocument, may end up with a case where
// map path is modified but parent still needs to be reset. See gh-10295
subdoc.$parent().$__reset();
parent.$__reset();
}
}
}
Expand Down Expand Up @@ -4070,6 +4068,7 @@ function applyGetters(self, json, options) {
path = paths[i];

const parts = path.split('.');

const plen = parts.length;
const last = plen - 1;
let branch = json;
Expand Down
1 change: 0 additions & 1 deletion lib/helpers/schema/getPath.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ module.exports = function getPath(schema, path) {
if (schematype != null) {
return schematype;
}

const pieces = path.split('.');
let cur = '';
let isArray = false;
Expand Down
7 changes: 4 additions & 3 deletions lib/schema.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1530,9 +1530,6 @@ Schema.prototype.indexedPaths = function indexedPaths() {
*/

Schema.prototype.pathType = function(path) {
// Convert to '.$' to check subpaths re: gh-6405
const cleanPath = _pathToPositionalSyntax(path);

if (this.paths.hasOwnProperty(path)) {
return 'real';
}
Expand All @@ -1542,6 +1539,10 @@ Schema.prototype.pathType = function(path) {
if (this.nested.hasOwnProperty(path)) {
return 'nested';
}

// Convert to '.$' to check subpaths re: gh-6405
const cleanPath = _pathToPositionalSyntax(path);

if (this.subpaths.hasOwnProperty(cleanPath) || this.subpaths.hasOwnProperty(path)) {
return 'real';
}
Expand Down
6 changes: 3 additions & 3 deletions lib/schema/SubdocumentPath.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,22 +160,22 @@ SubdocumentPath.prototype.cast = function(val, doc, init, priorVal, options) {
let subdoc;

// Only pull relevant selected paths and pull out the base path
const parentSelected = doc && doc.$__ && doc.$__.selected || {};
const parentSelected = doc && doc.$__ && doc.$__.selected;
const path = this.path;
const selected = Object.keys(parentSelected).reduce((obj, key) => {
const selected = parentSelected == null ? null : Object.keys(parentSelected).reduce((obj, key) => {
if (key.startsWith(path + '.')) {
obj = obj || {};
obj[key.substring(path.length + 1)] = parentSelected[key];
}
return obj;
}, null);
options = Object.assign({}, options, { priorDoc: priorVal });
if (init) {
subdoc = new Constructor(void 0, selected, doc, false, { defaults: false });
delete subdoc.$__.defaults;
subdoc.$init(val);
applyDefaults(subdoc, selected);
} else {
options = Object.assign({}, options, { priorDoc: priorVal });
if (Object.keys(val).length === 0) {
return new Constructor({}, selected, doc, undefined, options);
}
Expand Down

0 comments on commit 895bc32

Please sign in to comment.