perf(NODE-3570): short circuit check for keys that start with $

[znewsham]

Description

If no key starts with a $ - then there can be no $ref/$id/$db property. Looking for these is pretty inefficient currently.

Doing this results in about a 25% performance improvement (making it faster than JS where otherwise it would be slower).

All tests are passing

Turn off whitespace changes when reviewing - most of the change is just indenting the existing functionality into an if

[nbbeeken]

Hi @znewsham thanks for taking the time to offer some performance improvements for bson-ext, we have not maintained the library for some time so performance degradation is expected. It was written when Node.js and v8 were in a very different place (perf-wise) so there was motivation to write native addons to increase performance. Today we believe we can get more performance out of improvements to our js-bson library than we can in bson-ext due to the inherent limitation of crossing the JS-CPP boundary. So we planned to soon deprecate this library in favor of focusing on making js-bson the go to library for performance. Notably, a recent release to js-bson contained an improvement similar to the one you suggest here about how we go about parsing DBRef.

That being said, we appreciate the effort to improve this library and are open to accepting changes. If you still want to see these improvements land could you share with me the numbers you've measured and how you went about measuring them? Also what version of js-bson are you comparing against?

And, just confirming, is this your community post? If so I'll just link to this thread and we can continue the conversation here :)

Tracking: NODE-3570

[znewsham]

Hi @nbbeeken - I had no idea there was a plan to deprecate this library :( I can't find any reference to this anywhere in the documentation and it's still referenced by the mongo node driver. We recently switched to it as it offered a small (~2% in our testing) performance improvement in our general cases, though is slower in some specific cases.

Yes - that's my post! Figured this out about 5 mins after I posted that :D

I'm a little surprised that native modules don't have a clear performance benefit over JS for cases such as this - I guess perhaps there is a tradeoff where smaller objects are faster with JS and larger are vaster with native? Perhaps a hybrid model could work here, something heuristic based, e.g., > X bytes use native?

I'm comparing bson-ext@4.0.0 against bson@4.5.1 and my local version of bson-ext. The test below exposes the performance improvement quite nicely - though the exact amount of benefit changes based on how many objects (nested or siblings) are included - you can tweak MAX_DEPTH to see this in action - while the usecase is contrived, we actually found this when parsing a real document from our DB (one that is in no way an edge case either).

I'm getting results like this - where each line is the average time taken over about 333 passes (randomized order)

JS 6.12063550769233 321
native 5.273185519108295 345
nativeDev 4.92181353462604 334

const now = require('performance-now');

function testBSON() {
const jsBSON = require('bson');
const nativeBSON = require('bson-ext');
const nativeDevBSON = require('bson-ext-dev'); // sym links to my bson-ext

const MAX_DEPTH = 1000;
function largeObject(depth = 0) {
	for (let i = 0; i < 10; i++) {
		this[`v${i}`] = 'test';
	}
	if (depth < MAX_DEPTH) {
		this.d = new largeObject(depth + 1);
	}
}

ISODate = Date;


const obj = new largeObject();
const bsonObject = jsBSON.serialize(obj);

let times = { js: 0, native: 0, nativeDev: 0 };
let counts = { js: 0, native: 0, nativeDev: 0 };
for (let i = 0; i < 1000; i++) {
	let random = Math.random();
	let name = random >= 0.333 ? random >= 0.667 ? "js" : "native" : "nativeDev";
	let BSON = random >= 0.333 ? random >= 0.667 ? jsBSON : nativeBSON : nativeDevBSON;
	let start = now();
	let o = BSON.deserialize(bsonObject);
	let time = now() - start;
	times[name] += time;
	counts[name]++;
}

console.log("JS", times.js / counts.js, counts.js);
console.log("native", times.native / counts.native, counts.native);
console.log("nativeDev", times.nativeDev / counts.nativeDev, counts.nativeDev);
}
testBSON();

[luckyyyyy]

Don't give up, we use large object processing and get a significant performance improvement

[nbbeeken]

TL;DR we'll look into merging this soon if logic checks out.

Okay! So I've taken the benchmark you provided here and expanded it a bit in this repository, take a look at the code here and the results can be seen here (from a run on a Github Action machine)

I added a large object that is from this JSON file to give us something big with a complex shape, it is marked as DATAS in the results. Also tested serialization just to see it compared.

Bare with me while I describe the steps I've taken below and how they confirm our performance expectations. (Also feel free to correct the direction I took for testing this if you see something is off)

Focusing on only deserialization and bson-ext the results currently posted are:

LARGE deserialize bson-ext-40                      x 1,186 ops/sec ±0.85% (89 runs sampled)
LARGE deserialize bson-ext-shortcircuit-dollar     x 1,344 ops/sec ±1.17% (87 runs sampled)

SMALL deserialize bson-ext-40                      x 51,062 ops/sec ±1.05% (85 runs sampled)
SMALL deserialize bson-ext-shortcircuit-dollar     x 59,080 ops/sec ±1.28% (86 runs sampled)

DATAS deserialize bson-ext-40                      x 8.56 ops/sec ±1.49% (26 runs sampled)
DATAS deserialize bson-ext-shortcircuit-dollar     x 9.39 ops/sec ±2.39% (28 runs sampled)

Calculating the low end of the error margin for shortcircuit-dollar:

• LARGE: 1344 - (1344 * 0.0117) is 1,328 ops/sec
• SMALL: 59080 - (59080 * 0.0128) is 58,324 ops/sec
• DATAS: 9.39 - (9.39 * 0.0239) is 9.16 ops/sec

Calculating the high end of the error margin for 40:

• LARGE: 1186 + (1186 * 0.0085) is 1,196 ops/sec
• SMALL: 51062 + (51062 * 0.0105) is 51,598 ops/sec
• DATAS: 8.56 + (8.56 * 0.0149) is 8.69 ops/sec

So the calculations above show that even when taking the margins of error to their extreme in either direction we still have a clear improvement in performance.

Thanks again for taking the time to contribute this! I will bring the team in on taking a look at the logic to confirm we're set to merge this in.

---

On the subject of bson-ext itself:

It is correct we don't have the deprecation I mention properly documented anywhere, apologies, so I should instead say TBD about the planned support for this lib. We will clear up the messaging on that soon.
In the same performance measurement that I linked above you can see js-bson's results next to the BSON-EXT runs. While BSON-EXT does pull some better numbers in certain categories, merely stating an opinion here, the margin appears to be narrow. And js-bson does take the win for small objects as well as the large dataset that has a complex shape.

I agree that maybe there is a future for some hybrid model or possibly there are more improvements that could be made to the javascript implementation.
Another idea is that for very large inputs, deserialization, could be done on demand. A Map like API that looks up keys when requested would make for a good interface without needing the entire object to be translated to a JS format. I look forward to having continuous benchmarking soon and taking a crack at a number of suggested improvements we want to tackle.

[nbbeeken]

Started a patch here: https://spruce.mongodb.com/version/6137869a850e61403b8b54d4
(Not publicly viewable sorry!)

[nbbeeken]

LGTM Thanks again for this!