perf(NODE-6450): Lazy objectId hex string cache

[SeanReece]

Description

When ObjectId.cacheHexString is enabled there is a performance impact on ObjectId creation and the extra memory used by the hex string is consumed even if .toHexString() is never called on the ObjectId.

By lazily caching the hex string it provides the following benefits:

• No performance impact on ObjectId creation
• Memory use does not increase unless hex string is requested
• Duplicate .toHexString calls are still efficient as cache is used

Possible Negatives:

• hex string performance impact is moved to first .toHexString() invocation. Overall performance is still consistent, but it is no longer front loaded on ObjectId creation.

Performance

Custom benchmarks with cacheHexString=true

Test Case	bson Ops/sec	bson lazy Ops/sec	Improvement (%)
new ObjectId(string)	3,386,079	4,805,488	41.92%
new ObjectId()	5,269,078	11,309,317	114.64%
new ObjectId(string) + toHexString()	3,334,197	4,780,762	43.39%
new ObjectId() + toHexString()	5,082,142	5,080,183	0%

new ObjectId(string) + toHexString() test sees a performance improvement as we were performing an unnecessary .toHex when string is passed in.

Running granular benchmarks with cacheHexString=true

Test Case	bson throughput	bson lazy throughput	Improvement (%)
objectid_array_1000._deserialize_bson	95.1MB	178.2MB	87.4%
objectid_array_100._deserialize_bson	87.5MB	158.6MB	81.1%
objectid_array_10._deserialize_bson	77.5MB	132.9MB	71.5%

What is changing?

When cacheHexString=true we are no longer calling ByteUtils.toHex(buffer) in the constructor, this was already being checked/performed in the toHexString() call. If a string is passed into the constructor we can cache this immediately as there is no performance impact and no extra memory that needs to be allocated.

Is there new documentation needed for these changes?

No.

What is the motivation for this change?

We are often pulling many ObjectIds into memory (possibly millions) and need to deduplicate arrays of objectIds. In order to efficiently deduplicate ObjectIds (using Set) we need the string representation of an ObjectId, then when serializing documents to JSON we need the string representation of the ObjectId again. Converting an ObjectId to a hex string is a somewhat expensive operation that we'd like to avoid if necessary.

Enabling cacheHexString would improve the efficiency of these specific scenarios, but would lead to a performance impact and increased memory usage across the board for other scenarios that may never need ObjectId string.

Implementing lazy hex string caching will allow us to enable cacheHexString with zero performance impact, but see performance improvements in some scenarios.

Release Highlight

Fill in title or leave empty for no highlight

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

[aditi-khare-mongoDB]

Hi @SeanReece, thank you for submitting this pull request!

As of now, we won't explore fully supporting lazy loading hex string cache until the next major release (see NODE-6480) for the following reasons:

1. This changes the behavior of the non-hidden ObjectId.__id property upon construction
2. Generic deep equality checkers, such as LODASH, will no longer safely work on ObjectId

For the time being, when ObjectId.cacheHexString === true, let's still cache the hex string in constructor. In the case that a hex string is input to the constructor, there's no need to call toHex, but otherwise toHex is still called. This still allows for a perf optimization without being a breaking change. What do you think?

[SeanReece]

Thanks for the review @aditi-khare-mongoDB

Good point about deep equality. That's the same thing holding my other PR (#707) as well 😅

Being able to use # to hide __id would be great, but I noticed a significant performance and memory hit since we target es2021 and # was added in es2022 so tslib has to shim every call to __id

By using a weakMap instead of # to create actual private __id I don't see any performance hit so that's one option if we don't want to bump our target anytime soon.

Another option is to provide a cacheHexStringLazy option with documentation around it being incompatible with deep equality.

I can split the PR to implement the toHex perf improvement, but the main goal of this PR was to completely avoid any performance / memory impact when caching is not necessary. We have an application that is very memory sensitive and we handle millions of ObjectIds at a time.

Edit: This is what I'm thinking in terms of using weakMap for private members (if moving to ES2022 is not an option). Performance seems to be good with this implementation. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakMap#emulating_private_members

[aditi-khare-mongoDB]

Hi again @SeanReece, thanks for sharing your thoughts and replying so promptly! Next week, the team will consider your points, discuss the best path forward, and get back to you.

[baileympearson]

Hey @SeanReece, thanks for bearing with us on this PR. Private properties implemented with weakmaps work for us, if you're willing to make those changes. We'll need to figure out a way to test the caching behavior though, the existing tests rely on accessing the TS-private __id variable.

[SeanReece]

@baileympearson great! I don’t mind implementing this. Good point about the tests. I’ll investigate how to best make these changes and make them testable 🤔

[SeanReece]

@baileympearson I've updated the code to hide __id using weakMap implementation. There is a slight performance hit vs direct property access but it's still faster than using # with ES2021 target, and there's still meaningful performance gains vs main. I've added a // TODO to convert this to # private field when target is updated to ES2022.

As for the tests, I've added a isCached() private function that checks if the hex string is cached. This also allows us to test the lazy functionality by ensuring that isCached is only true after toHexString() is called once.

Let me know if you have any questions or concerns about anything here.

Perf tests

main vs lazy-weakMap
🎉WINNERS
objectid_singleFieldDocument._deserialize_bson 46.3MB 154.7MB 234.3%
objectid_array_1000._deserialize_bson 113.2MB 294.3MB 160.1%
objectid_array_100._deserialize_bson 106.1MB 261.8MB 146.8%
objectid_array_10._deserialize_bson 94.4MB 180.6MB 91.2%
objectid_singleElementArray._deserialize_bson 76.5MB 105.5MB 37.8%

[baileympearson]

One small change - otherwise LGTM!

[SeanReece]

@baileympearson Good catch. I've added it back.

Any idea when v7 is planned for?

[baileympearson]

@SeanReece We don't have a target date for BSON v7 or driver v7.