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(NODE-5955): use pooled memory when possible #653

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

nbbeeken
Copy link
Contributor

Description

What is changing?

Use allocUnsafe which uses Node.js' pool of pre-allocated memory and does not santize it before returning it to the caller. We can use it in cases where we overwrite the entire returned Buffer to improve the performance of creating small memory allocations for ObjectIds and Decimal128 instances.

Is there new documentation needed for these changes?

No.

What is the motivation for this change?

Using the memory pool is faster than creating a new allocation.

Release Highlight

Use allocUnsafe for ObjectIds and Decimal128

For small allocations Node.js performance can be improved by using pre-allocated pooled memory. ObjectIds and Decimal128 instance will now use allocUnsafe on Node.js.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@nbbeeken
Copy link
Contributor Author

Node.js v20.11.0
OS: linux
CPUs: 8
Arch: x64
RAM: 33.105743872 GB

test single ObjectId
bson_deserialize_current_main x 2,735,078 ops/sec ±0.11% (395 runs sampled)
bson_deserialize_pr x 3,568,042 ops/sec ±0.46% (394 runs sampled)
bson_serialize_current_main x 2,260,330 ops/sec ±0.16% (393 runs sampled)
bson_serialize_pr x 2,227,234 ops/sec ±0.14% (394 runs sampled)
Fastest to slowest is:
- serialize on main is faster by 1.47%
- deserialize on pr is faster by 26.43%

test 1000 ObjectId
bson_deserialize_current_main x 3,987 ops/sec ±0.09% (395 runs sampled)
bson_deserialize_pr x 5,369 ops/sec ±0.17% (393 runs sampled)
bson_serialize_current_main x 8,484 ops/sec ±0.29% (391 runs sampled)
bson_serialize_pr x 8,655 ops/sec ±0.28% (395 runs sampled)
Fastest to slowest is:
- serialize on pr is faster by 1.99%
- deserialize on pr is faster by 29.54%

test single Decimal128
bson_deserialize_current_main x 1,834,433 ops/sec ±0.11% (393 runs sampled)
bson_deserialize_pr x 2,303,206 ops/sec ±0.09% (394 runs sampled)
bson_serialize_current_main x 1,995,501 ops/sec ±0.13% (394 runs sampled)
bson_serialize_pr x 1,987,482 ops/sec ±0.43% (394 runs sampled)
Fastest to slowest is:
- serialize on main is faster by 0.40%
- deserialize on pr is faster by 22.66%

test 1000 Decimal128
bson_deserialize_current_main x 2,396 ops/sec ±0.08% (394 runs sampled)
bson_deserialize_pr x 3,190 ops/sec ±0.09% (395 runs sampled)
bson_serialize_current_main x 6,776 ops/sec ±0.40% (393 runs sampled)
bson_serialize_pr x 7,085 ops/sec ±0.31% (393 runs sampled)
Fastest to slowest is:
- serialize on pr is faster by 4.45%
- deserialize on pr is faster by 28.41%

Granular benchmarks

@nbbeeken nbbeeken marked this pull request as ready for review February 28, 2024 19:38
@billouboq
Copy link
Contributor

billouboq commented Feb 29, 2024

Nice improvement, didn't think it would be that much 👍

@durran durran self-assigned this Feb 29, 2024
@durran durran added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Feb 29, 2024
@durran durran added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Feb 29, 2024
@durran durran merged commit 78c4264 into main Feb 29, 2024
4 checks passed
@durran durran deleted the NODE-5955-allocate-faster branch February 29, 2024 16:26
@nathan-knight
Copy link

Hey @nbbeeken, I just wanted to say that this appears to have been a massive boost in performance for my use case so thank you for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants