perf(NODE-6344): Improve ObjectId.isValid(string) performance

[SeanReece]

Description

This MR improves performance of using ObjectId.isValid('some string') to check if a string is a valid hex representation of an ObjectId. We frequently call this function to check if a user's input is valid (for example, when creating a document that links to other document(s)).

Performance tests

Test	Ops/s (Main)	Ops/s (New)	% Improvement
bson#new ObjectId(string)	3,173,643 ops/sec	3,299,153 ops/sec	3.95%
bson#isValid	5,513,041 ops/sec	22,529,639 ops/sec	308.66%
bson#isValid uppercase	5,498,098 ops/sec	21,776,208 ops/sec	296.07%
bson#isValid invalid end	131,657 ops/sec	22,592,719 ops/sec	17,060.24%
bson#isValid invalid start	132,751 ops/sec	152,311,298 ops/sec	114,634.47%

There is a drastic performance improvement when checking for invalid strings since throwing/catching errors is expensive.

What is changing?

Is there new documentation needed for these changes?

None

What is the motivation for this change?

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

[SeanReece]

Not sure if you wanted a Jira created since this is such a small change, but I could create one if necessary.

[baileympearson]

Hey @SeanReece - I don't believe we have benchmarks for this method. Could you share your benchmarking script so I can test and reproduce?

[SeanReece]

@baileympearson Sure thing. I just threw together a simple benchmark since I can't get bson-bench to run. If you pull this branch locally, change the name in package.json to bson-isvalid and run an npm run build then you can import bson main and this branch alongside each other.

var Benchmark = require("benchmark");
const bson = require("bson");
const bsonVAL = require("bson-isvalid");

var suite = new Benchmark.Suite();

suite
  // Test calling isValid with a valid objectId hex
  .add("bson#isValid", function () {
    bson.ObjectId.isValid("6683fe22cd402749418c7e2b");
  })
  .add("bsonVAL#isValid", function () {
    bsonVAL.ObjectId.isValid("6683fe22cd402749418c7e2b");
  })

   // Test calling isValid with an INVALID objectId hex
  .add("bson#isValid with invalid string", function () {
    bson.ObjectId.isValid("6683fE22cd402749418c7e2x");
  })
  .add("bsonVAL#isValid with invalid string", function () {
    bsonVAL.ObjectId.isValid("6683fE22cd402749418c7e2x");
  })
  .on("cycle", function (event) {
    console.log(String(event.target));
  })
  .on("complete", function () {
    console.log("Fastest is " + this.filter("fastest").map("name"));
  })
  .run({ async: true });

[baileympearson]

@SeanReece Thanks - I see the same behavior.

I'll bring this up with the team later this week.

[durran]

@SeanReece just as the other PR for isValid I've rebased this one as well.

[nbbeeken]

Now we do have benchmarks merged for this helper! Also we're better set up to add any helper to our CI so after it is improved we can check if any future changes regress it or make it better. (check out test/bench/custom/readme.md if you're curious)

[nbbeeken]

LGTM!