Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 18 additions & 11 deletions src/objectid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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'
Expand All @@ -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);
}
}

/**
Expand All @@ -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));
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*
Expand Down
49 changes: 34 additions & 15 deletions test/node/bson_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand Down