chore: reduce caches

Author: nbbeeken

package-lock.json

@@ -26,7 +26,7 @@
   },
   "dependencies": {
     "@mongodb-js/saslprep": "^1.1.5",
-    "bson": "github:mongodb/js-bson#NODE-6059-cleanup",
+    "bson": "github:mongodb/js-bson#main",
     "mongodb-connection-string-url": "^3.0.0"
   },
   "peerDependencies": {

package.json

@@ -33,12 +33,8 @@ export function parseToElementsToArray(bytes: Uint8Array, offset?: number): BSON
   const res = BSON.onDemand.parseToElements(bytes, offset);
   return Array.isArray(res) ? res : [...res];
 }
-
-// eslint-disable-next-line @typescript-eslint/unbound-method
 export const getInt32LE = BSON.onDemand.NumberUtils.getInt32LE;
-// eslint-disable-next-line @typescript-eslint/unbound-method
 export const getFloat64LE = BSON.onDemand.NumberUtils.getFloat64LE;
-// eslint-disable-next-line @typescript-eslint/unbound-method
 export const getBigInt64LE = BSON.onDemand.NumberUtils.getBigInt64LE;
 export const toUTF8 = BSON.onDemand.ByteUtils.toUTF8;
 

src/bson.ts

@@ -39,14 +39,20 @@ export type JSTypeOf = {
   [BSONType.array]: OnDemandDocument;
 };
 
+/** @internal */
+type CachedBSONElement = { element: BSONElement; value: any | undefined };
+
 /** @internal */
 export class OnDemandDocument {
-  /** Caches the existence of a property */
-  private readonly existenceOf: Record<string, boolean> = Object.create(null);
-  /** Caches a look up of name to element */
-  private readonly elementOf: Record<string, BSONElement> = Object.create(null);
-  /** Caches the revived javascript value */
-  private readonly valueOf: Record<string, any> = Object.create(null);
+  /**
+   * Maps JS strings to elements and jsValues for speeding up subsequent lookups.
+   * - If `false` then name does not exist in the BSON document
+   * - If `CachedBSONElement` instance name exists
+   * - If `cache[name].value == null` jsValue has not yet been parsed
+   *   - Null/Undefined values do not get cached because they are zero-length values.
+   */
+  private readonly cache: Record<string, CachedBSONElement | false | undefined> =
+    Object.create(null);
   /** Caches the index of elements that have been named */
   private readonly indexFound: Record<number, boolean> = Object.create(null);
 
@@ -90,29 +96,27 @@ export class OnDemandDocument {
    * @param name - a basic latin string name of a BSON element
    * @returns
    */
-  private getElement(name: string): BSONElement | null {
-    if (this.existenceOf[name] === false) return null;
+  private getElement(name: string): CachedBSONElement | null {
+    const cachedElement = this.cache[name];
+    if (cachedElement === false) return null;
 
-    if (this.elementOf[name] != null) {
-      return this.elementOf[name];
+    if (cachedElement != null) {
+      return cachedElement;
     }
 
     for (let index = 0; index < this.elements.length; index++) {
       const element = this.elements[index];
 
-      if (
-        // skip this element if it has already been associated with a name
-        !this.indexFound[index] &&
-        this.isElementName(name, element)
-      ) {
-        this.elementOf[name] = element;
+      // skip this element if it has already been associated with a name
+      if (!this.indexFound[index] && this.isElementName(name, element)) {
+        const cachedElement = { element, value: undefined };
+        this.cache[name] = cachedElement;
         this.indexFound[index] = true;
-        this.existenceOf[name] = true;
-        return this.elementOf[name];
+        return cachedElement;
       }
     }
 
-    this.existenceOf[name] = false;
+    this.cache[name] = false;
     return null;
   }
 
@@ -202,7 +206,10 @@ export class OnDemandDocument {
    * @param name - element name
    */
   public has(name: string): boolean {
-    return (this.existenceOf[name] ??= this.getElement(name) != null);
+    const cachedElement = this.cache[name];
+    if (cachedElement === false) return false;
+    if (cachedElement != null) return true;
+    return this.getElement(name) != null;
   }
 
   /**
@@ -236,8 +243,8 @@ export class OnDemandDocument {
       }
     }
 
-    if (!(name in this.valueOf)) {
-      const value = this.toJSValue(element, as);
+    if (element.value == null) {
+      const value = this.toJSValue(element.element, as);
       if (value == null) {
         if (required === true) {
           throw new BSONError(`BSON element "${name}" is missing`);
@@ -246,10 +253,10 @@ export class OnDemandDocument {
         }
       }
       // It is important to never store null
-      this.valueOf[name] = value;
+      element.value = value;
     }
 
-    return this.valueOf[name];
+    return element.value;
   }
 
   /**
@@ -306,9 +313,9 @@ export class OnDemandDocument {
     }
     let counter = 0;
     for (const element of this.elements) {
-      const item = this.toJSValue<T>(element, as);
-      this.valueOf[counter] = item;
-      yield item;
+      const value = this.toJSValue<T>(element, as);
+      this.cache[counter] = { element, value };
+      yield value;
       counter += 1;
     }
   }

src/cmap/wire_protocol/on_demand/document.ts

@@ -17,8 +17,8 @@ describe('class OnDemandDocument', () => {
       const doc = new OnDemandDocument(emptyDocument, 0, false);
       expect(doc.has('ok')).to.be.false;
       expect(doc.has('$clusterTime')).to.be.false;
-      expect(doc).to.have.nested.property('existenceOf.ok', false);
-      expect(doc).to.have.nested.property('existenceOf.$clusterTime', false);
+      expect(doc).to.have.nested.property('cache.ok', false);
+      expect(doc).to.have.nested.property('cache.$clusterTime', false);
     });
   });
 
@@ -27,14 +27,14 @@ describe('class OnDemandDocument', () => {
       const emptyDocument = BSON.serialize({ ok: 1 });
       const doc = new OnDemandDocument(emptyDocument, 0, false);
       expect(doc.has('ok')).to.be.true;
-      expect(doc).to.have.nested.property('existenceOf.ok', true);
+      expect(doc).to.have.nested.property('cache.ok').that.is.an('object');
     });
 
     it('sets exists cache to false for any other key', () => {
       const emptyDocument = BSON.serialize({ ok: 1 });
       const doc = new OnDemandDocument(emptyDocument, 0, false);
       expect(doc.has('$clusterTime')).to.be.false;
-      expect(doc).to.have.nested.property('existenceOf.$clusterTime', false);
+      expect(doc).to.have.nested.property('cache.$clusterTime', false);
     });
   });
 
@@ -43,7 +43,7 @@ describe('class OnDemandDocument', () => {
       const emptyDocument = BSON.serialize({ ok: 0, code: 2 });
       const doc = new OnDemandDocument(emptyDocument, 0, false);
       expect(doc.has('code')).to.be.true;
-      expect(doc).to.have.nested.property('existenceOf.code', true);
+      expect(doc).to.have.nested.property('cache.code').that.is.an('object');
       expect(doc).to.not.have.nested.property('indexFound.0');
       expect(doc).to.have.nested.property('indexFound.1', true);
     });
@@ -116,7 +116,7 @@ describe('class OnDemandDocument', () => {
 
     it('throws if required is set to true and element name does not exist', () => {
       expect(() => document.get('blah!', BSONType.bool, true)).to.throw(BSONError);
-      expect(document).to.have.nested.property('existenceOf.blah!', false);
+      expect(document).to.have.nested.property('cache.blah!', false);
     });
 
     it('throws if requested type is unsupported', () => {
@@ -127,8 +127,10 @@ describe('class OnDemandDocument', () => {
     });
 
     it('caches the value', () => {
+      document.has('int');
+      expect(document).to.have.nested.property('cache.int.value', undefined);
       document.get('int', BSONType.int);
-      expect(document).to.have.nested.property('valueOf.int', 1);
+      expect(document).to.have.nested.property('cache.int.value', 1);
     });
 
     it('supports returning null for null and undefined bson elements', () => {
@@ -284,12 +286,12 @@ describe('class OnDemandDocument', () => {
       expect(didRun).to.be.true;
     });
 
-    it('caches the results in valueOf', () => {
+    it('caches the results of array', () => {
       const generator = array.valuesAs(BSONType.int);
       generator.next();
       generator.next();
-      expect(array).to.have.nested.property('valueOf.0', 1);
-      expect(array).to.have.nested.property('valueOf.1', 1);
+      expect(array).to.have.nested.property('cache.0.value', 1);
+      expect(array).to.have.nested.property('cache.1.value', 1);
     });
   });
 });