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(document): avoid cloning options using spread operator for perf reasons #14565

Merged
merged 5 commits into from
May 6, 2024

Conversation

vkarpov15
Copy link
Collaborator

Re: #14394

Summary

3x perf improvement on benchmark with this one weird trick. Without this PR:

$ node gh-14394.js 
Num docs 200 200
toObject ms: 1141
{
  _id: new ObjectId('66365214acdcbc00eacbd721'),
  name: 'test child 0_0',
  parentId: new ObjectId('66365214acdcbc00eacbd71f'),
  __v: 0
}
Done

With this PR:

$ node gh-14394.js 
Num docs 200 200
toObject ms: 492
{
  _id: new ObjectId('66365214acdcbc00eacbd721'),
  name: 'test child 0_0',
  parentId: new ObjectId('66365214acdcbc00eacbd71f'),
  __v: 0
}
Done

Not sure why this is so much faster, but still works and makes $toObject() that much faster 🤷‍♂️

Examples

@vkarpov15
Copy link
Collaborator Author

f087e8f includes a similar, but smaller speedup. After this change:

$ node gh-14394.js 
toObject ms: 469
Done

@vkarpov15
Copy link
Collaborator Author

I'm going to merge this PR and ship with 8.3.4. But going to keep working on this because the performance improvements are quite substantial, and we could really stand to simplify the options parsing logic in $toObject().

@vkarpov15 vkarpov15 added this to the 8.3.4 milestone May 6, 2024
@vkarpov15 vkarpov15 merged commit 13db408 into master May 6, 2024
34 checks passed
@hasezoey hasezoey deleted the vkarpov15/gh-14394 branch May 6, 2024 19:47
vkarpov15 added a commit that referenced this pull request May 16, 2024
@vkarpov15 vkarpov15 restored the vkarpov15/gh-14394 branch June 17, 2024 18:09
@hasezoey hasezoey deleted the vkarpov15/gh-14394 branch November 1, 2024 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants