Skip to content

fix(document): avoid massive perf degradation when saving new doc with 10 level deep subdocs #14910

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Sep 25, 2024
Merged

fix(document): avoid massive perf degradation when saving new doc with 10 level deep subdocs #14910

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
328ddaa
fix(document): avoid massive perf degradation when saving new doc wit…
vkarpov15 Sep 24, 2024
0426329
perf: add createDeepNestedDocArray benchmark re: #14897
vkarpov15 Sep 24, 2024
5851261
perf(document): avoid unnecessarily pulling all subdocs when validati…
vkarpov15 Sep 24, 2024
95500b6
perf(document): remove unnecessary reset logic
vkarpov15 Sep 25, 2024
d273278
Merge branch 'master' into vkarpov15/gh-14897
vkarpov15 Sep 25, 2024
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
Prev Previous commit
Next Next commit
perf(document): avoid unnecessarily pulling all subdocs when validati… 
…ng a subdoc
  • Loading branch information
@vkarpov15
vkarpov15 committed Sep 24, 2024
commit 5851261c1501fc58d3722513f708a1e87da63f4e
66 changes: 35 additions & 31 deletions lib/document.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2689,7 +2689,7 @@ function _evaluateRequiredFunctions(doc) {
* ignore
*/

function _getPathsToValidate(doc, pathsToValidate, pathsToSkip) {
function _getPathsToValidate(doc, pathsToValidate, pathsToSkip, isNestedValidate) {
const doValidateOptions = {};

_evaluateRequiredFunctions(doc);
Expand All @@ -2709,37 +2709,40 @@ function _getPathsToValidate(doc, pathsToValidate, pathsToSkip) {
Object.keys(doc.$__.activePaths.getStatePaths('default')).forEach(addToPaths);
function addToPaths(p) { paths.add(p); }

const subdocs = doc.$getAllSubdocs();
const modifiedPaths = doc.modifiedPaths();
for (const subdoc of subdocs) {
if (subdoc.$basePath) {
const fullPathToSubdoc = subdoc.$isSingleNested ? subdoc.$__pathRelativeToParent() : subdoc.$__fullPathWithIndexes();

// 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 (!isNestedValidate) {
// If we're validating a subdocument, all this logic will run anyway on the top-level document, so skip for subdocuments
const subdocs = doc.$getAllSubdocs();
const modifiedPaths = doc.modifiedPaths();
for (const subdoc of subdocs) {
if (subdoc.$basePath) {
const fullPathToSubdoc = subdoc.$isSingleNested ? subdoc.$__pathRelativeToParent() : subdoc.$__fullPathWithIndexes();

// 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, null, modifiedPaths) &&
// Avoid using isDirectModified() here because that does additional checks on whether the parent path
// is direct modified, which can cause performance issues re: gh-14897
!doc.$__.activePaths.getStatePaths('modify').hasOwnProperty(fullPathToSubdoc) &&
!doc.$isDefault(fullPathToSubdoc)) {
paths.add(fullPathToSubdoc);
if (doc.$isModified(fullPathToSubdoc, null, modifiedPaths) &&
// Avoid using isDirectModified() here because that does additional checks on whether the parent path
// is direct modified, which can cause performance issues re: gh-14897
!doc.$__.activePaths.getStatePaths('modify').hasOwnProperty(fullPathToSubdoc) &&
!doc.$isDefault(fullPathToSubdoc)) {
paths.add(fullPathToSubdoc);

if (doc.$__.pathsToScopes == null) {
doc.$__.pathsToScopes = {};
}
doc.$__.pathsToScopes[fullPathToSubdoc] = subdoc.$isDocumentArrayElement ?
subdoc.__parentArray :
subdoc.$parent();
if (doc.$__.pathsToScopes == null) {
doc.$__.pathsToScopes = {};
}
doc.$__.pathsToScopes[fullPathToSubdoc] = subdoc.$isDocumentArrayElement ?
subdoc.__parentArray :
subdoc.$parent();

doValidateOptions[fullPathToSubdoc] = { skipSchemaValidators: true };
if (subdoc.$isDocumentArrayElement && subdoc.__index != null) {
doValidateOptions[fullPathToSubdoc].index = subdoc.__index;
doValidateOptions[fullPathToSubdoc] = { skipSchemaValidators: true };
if (subdoc.$isDocumentArrayElement && subdoc.__index != null) {
doValidateOptions[fullPathToSubdoc].index = subdoc.__index;
}
}
}
}
Expand Down Expand Up @@ -2974,7 +2977,7 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) {
paths = [...paths];
doValidateOptionsByPath = {};
} else {
const pathDetails = _getPathsToValidate(this, pathsToValidate, pathsToSkip);
const pathDetails = _getPathsToValidate(this, pathsToValidate, pathsToSkip, options && options._nestedValidate);
paths = shouldValidateModifiedOnly ?
pathDetails[0].filter((path) => this.$isModified(path)) :
pathDetails[0];
Expand Down Expand Up @@ -3061,7 +3064,8 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) {
const doValidateOptions = {
...doValidateOptionsByPath[path],
path: path,
validateAllPaths
validateAllPaths,
_nestedValidate: true
};

schemaType.doValidate(val, function(err) {
Expand Down