perf(NODE-4194): improve objectId constructor performance

[nbbeeken]

Description

What is changing?

Gating ensureBuffer with an instanceof check should avoid the reletively expensive Buffer.from call.

What is the motivation for this change?

Improve the performance of creating an ObjectId from a buffer.

Double check the following

• Ran npm run lint script
• Self-review completed using the steps outlined here
• PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
• Changes are covered by tests
• New TODOs have a related JIRA ticket

[nbbeeken]

@addaleax This change would revert a bits of #429 and #432. The thinking is that the instanceof check can succeed for nodejs environments dodging the Buffer.from call, and in environments where the prototype chain doesn't match we still fall back to calling .from. Any thoughts on this?

[addaleax]

@nbbeeken I don’t see any downsides in terms of correctness. I remember that instanceof could be quite slow on some older-but-not-that-old V8 versions, so it might be worth checking the performance impact across Node.js versions.

In the long run, I think the best solution would be to just use Uint8Array everywhere anyway :)

[durran]

My only comment would be to up the iterations in the benchmark to get a more accurate representation of performance once it's warmed up. Here are the differences on my machine on 10.000 vs 1.000.000 iterations - it's far less of a discrepancy.

Iterations: 10000

BSON@current local
deserialize({ oid, string }, { validation: { utf8: false } }) takes 0.00283865ms on average
new Oid(buf) take 0.00018213ms on average

[email protected]
deserialize({ oid, string }, { validation: { utf8: false } }) takes 0.00303077ms on average
new Oid(buf) take 0.00059272ms on average

[email protected]
deserialize({ oid, string }, { validation: { utf8: false } }) takes 0.00203647ms on average
new Oid(buf) take 0.00031121ms on average

Iterations: 1000000

BSON@current local
deserialize({ oid, string }, { validation: { utf8: false } }) takes 0.00206223ms on average
new Oid(buf) take 0.00013110ms on average

[email protected]
deserialize({ oid, string }, { validation: { utf8: false } }) takes 0.00150312ms on average
new Oid(buf) take 0.00020934ms on average

[email protected]
deserialize({ oid, string }, { validation: { utf8: false } }) takes 0.00098606ms on average
new Oid(buf) take 0.00014574ms on average

[Uzlopak]

What did you use to bench?

[nbbeeken]

@durran I bumped the iterations and I added system info print out

- cpu: Intel(R) Core(TM) i5-8279U CPU @ 2.40GHz
- cores: 8
- os: darwin
- ram: 16GB
- iterations: 1,000,000

BSON@current local
deserialize({ oid, string }, { validation: { utf8: false } }) takes 0.00236919ms on average
new Oid(buf) take 0.00009414ms on average

[email protected]
deserialize({ oid, string }, { validation: { utf8: false } }) takes 0.00156644ms on average
new Oid(buf) take 0.00018286ms on average

[email protected]
deserialize({ oid, string }, { validation: { utf8: false } }) takes 0.00113818ms on average
new Oid(buf) take 0.00011388ms on average

[harrisonhunter]

Hello! Is there a planned date for the next release of the package?

Waiting for this performance improvement to be shipped to release a different update in another project so just curious if there is a timeline

[nbbeeken]

@harrisonhunter The improvement is now available in v4.6.4 🚀

[harrisonhunter]

Awesome! Thank you very much