Skip to content

perf: cache results from getAllSubdocs() on saveOptions, only loop through known subdoc properties#15055

Merged
vkarpov15 merged 2 commits intomasterfrom
vkarpov15/gh-15029
Nov 26, 2024
Merged

perf: cache results from getAllSubdocs() on saveOptions, only loop through known subdoc properties#15055
vkarpov15 merged 2 commits intomasterfrom
vkarpov15/gh-15029

Conversation

@vkarpov15
Copy link
Copy Markdown
Collaborator

Fix #15029

Summary

#15029 points out that we spend a lot of unnecessary CPU cycles in save() in $getAllSubdocs(), even for schemas that have no subdocs. That's because currently save() calls $getAllSubdocs() 5 times, and $getAllSubdocs() loops through every path in the document searching for subdocs.

With this PR, we will only call $getAllSubdocs() 1x if calling save() with no subdocs, and also $getAllSubdocs() will only loop through paths we know can have subdocs based on the schema. So if the schema doesn't have any subdocuments, document arrays, or maps of subdocs, then $getAllSubdocs() will have an empty for loop.

In my quick experiment with the saveSimple benchmark, I'm seeing a 2.7% improvement in performance, which isn't huge, but that's 2.7% on every save call.

I'll also work on adding a benchmark with subdocuments to see how much this helps with subdocs.

Examples

@vkarpov15
Copy link
Copy Markdown
Collaborator Author

Quick check with the createDeepNestedDocArray benchmark, which is a good example of save() with a lot of very deeply nested subdocs, shows a 5% speedup.

@billouboq
Copy link
Copy Markdown
Contributor

Does it improves performance of create or insertMany ?

Copy link
Copy Markdown
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

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

LGTM, aside from the minor jsdoc change necessary. I would recommend to put this into the next minor version for caution.

Comment thread benchmarks/saveSimple.js
Comment thread lib/document.js
@hasezoey
Copy link
Copy Markdown
Collaborator

@billouboq

Does it improves performance of create or insertMany ?

It will affect any function making use of $getAllSubdocs, which by extension is $validate (which is then made use of in insertMany, save, etc), thought to different extends depending on previous amount of calls to $getAllSubdocs.
(note that create is using .save, so any changes to save also affects create directly).

@vkarpov15 vkarpov15 added this to the 8.8.3 milestone Nov 25, 2024
@vkarpov15
Copy link
Copy Markdown
Collaborator Author

insertMany() uses validate() under the hood, so this should help a bit on insertMany() too. Definitely on create(), because create() calls save().

@vkarpov15 vkarpov15 merged commit 862d1a5 into master Nov 26, 2024
@vkarpov15 vkarpov15 deleted the vkarpov15/gh-15029 branch November 26, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mongose docReducer function inside __getAllSubdocs taking server hgh cpu even if my use-case doesn't involve embedded docs at all?

3 participants