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

Co-authored-by: Bailey Pearson <[email protected]>
mongodb · Nov 20, 2024 · 7c37580 · 7c37580
1 parent 6e84925
commit 7c37580
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 26 deletions.
diff --git a/src/objectid.ts b/src/objectid.ts
@@ -7,6 +7,9 @@ import { NumberUtils } from './utils/number_utils';
 // Unique sequence for the current process (initialized on first use)
 let PROCESS_UNIQUE: Uint8Array | null = null;
 
+/** ObjectId hexString cache @internal */
+const __idCache = new WeakMap(); // TODO(NODE-6549): convert this to #__id private field when target updated to ES2022
+
 /** @public */
 export interface ObjectIdLike {
   id: string | Uint8Array;
@@ -36,8 +39,6 @@ export class ObjectId extends BSONValue {
 
   /** ObjectId Bytes @internal */
   private buffer!: Uint8Array;
-  /** ObjectId hexString cache @internal */
-  private __id?: string;
 
   /**
    * Create ObjectId from a number.
@@ -111,6 +112,10 @@ export class ObjectId extends BSONValue {
     } else if (typeof workingId === 'string') {
       if (ObjectId.validateHexString(workingId)) {
         this.buffer = ByteUtils.fromHex(workingId);
+        // If we are caching the hex string
+        if (ObjectId.cacheHexString) {
+          __idCache.set(this, workingId);
+        }
       } else {
         throw new BSONError(
           'input must be a 24 character hex string, 12 byte Uint8Array, or an integer'
@@ -119,10 +124,6 @@ export class ObjectId extends BSONValue {
     } else {
       throw new BSONError('Argument passed in does not match the accepted types');
     }
-    // If we are caching the hex string
-    if (ObjectId.cacheHexString) {
-      this.__id = ByteUtils.toHex(this.id);
-    }
   }
 
   /**
@@ -136,7 +137,7 @@ export class ObjectId extends BSONValue {
   set id(value: Uint8Array) {
     this.buffer = value;
     if (ObjectId.cacheHexString) {
-      this.__id = ByteUtils.toHex(value);
+      __idCache.set(this, ByteUtils.toHex(value));
     }
   }
 
@@ -165,14 +166,15 @@ export class ObjectId extends BSONValue {
 
   /** Returns the ObjectId id as a 24 lowercase character hex string representation */
   toHexString(): string {
-    if (ObjectId.cacheHexString && this.__id) {
-      return this.__id;
+    if (ObjectId.cacheHexString) {
+      const __id = __idCache.get(this);
+      if (__id) return __id;
     }
 
     const hexString = ByteUtils.toHex(this.id);
 
-    if (ObjectId.cacheHexString && !this.__id) {
-      this.__id = hexString;
+    if (ObjectId.cacheHexString) {
+      __idCache.set(this, hexString);
     }
 
     return hexString;
@@ -370,6 +372,11 @@ export class ObjectId extends BSONValue {
     return new ObjectId(doc.$oid);
   }
 
+  /** @internal */
+  private isCached(): boolean {
+    return ObjectId.cacheHexString && __idCache.has(this);
+  }
+
   /**
    * Converts to a string representation of this Id.
    *

diff --git a/test/node/bson_test.js b/test/node/bson_test.js
@@ -1573,36 +1573,55 @@ describe('BSON', function () {
    */
   it('ObjectId should have a correct cached representation of the hexString', function (done) {
     ObjectId.cacheHexString = true;
+    // generated ObjectID uses lazy caching
     var a = new ObjectId();
-    var __id = a.__id;
-    expect(__id).to.equal(a.toHexString());
-
-    // hexString
-    a = new ObjectId(__id);
-    expect(__id).to.equal(a.toHexString());
+    expect(a.isCached()).to.be.false;
+    a.toHexString();
+    expect(a.isCached()).to.be.true;
+    expect(a.toHexString()).to.equal(a.toHexString());
+
+    // hexString caches immediately
+    a = new ObjectId(a.toHexString());
+    expect(a.isCached()).to.be.true;
+    a.toHexString();
+    expect(a.isCached()).to.be.true;
+    expect(a.toHexString()).to.equal(a.toHexString());
 
     // fromHexString
-    a = ObjectId.createFromHexString(__id);
-    expect(a.__id).to.equal(a.toHexString());
-    expect(__id).to.equal(a.toHexString());
+    a = ObjectId.createFromHexString(a.toHexString());
+    expect(a.isCached()).to.be.false;
+    a.toHexString();
+    expect(a.isCached()).to.be.true;
+    expect(a.toHexString()).to.equal(a.toHexString());
 
     // number
     var genTime = a.generationTime;
     a = new ObjectId(genTime);
-    __id = a.__id;
-    expect(__id).to.equal(a.toHexString());
+    expect(a.isCached()).to.be.false;
+    a.toHexString();
+    expect(a.isCached()).to.be.true;
+    expect(a.toHexString()).to.equal(a.toHexString());
 
     // generationTime
-    delete a.__id;
     a.generationTime = genTime;
-    expect(__id).to.equal(a.toHexString());
+    expect(a.isCached()).to.be.true;
+    expect(a.toHexString()).to.equal(a.toHexString());
 
     // createFromTime
     a = ObjectId.createFromTime(genTime);
-    __id = a.__id;
-    expect(__id).to.equal(a.toHexString());
+    expect(a.isCached()).to.be.false;
+    a.toHexString();
+    expect(a.isCached()).to.be.true;
+    expect(a.toHexString()).to.equal(a.toHexString());
+
     ObjectId.cacheHexString = false;
 
+    // No longer caches after cache is disabled
+    a = new ObjectId();
+    expect(a.isCached()).to.be.false;
+    a.toHexString();
+    expect(a.isCached()).to.be.false;
+
     done();
   });