Skip to content

Commit

Permalink
Optimize codecs context cacheing
Browse files Browse the repository at this point in the history
We should really copy the cached codex context for cloned
Options instances (unless .withCodecs() was called).
  • Loading branch information
1st1 committed Feb 13, 2025
1 parent abed926 commit 69178c9
Showing 1 changed file with 13 additions and 5 deletions.
18 changes: 13 additions & 5 deletions packages/driver/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,10 @@ export class Options {
/** @internal */
readonly annotations: ReadonlyMap<string, string> = new Map<string, string>();

private cachedCodecContext: CodecContext | null = null;
private cachedCodecContextVer = -1;
/** @internal */
cachedCodecContext: CodecContext | null = null;
/** @internal */
cachedCodecContextVer = -1;

get tag(): string | null {
return this.annotations.get(TAG_ANNOTATION_KEY) ?? null;
Expand Down Expand Up @@ -269,30 +271,36 @@ export class Options {
clone.moduleAliases = this.moduleAliases;
}

let originalCodecsMap = false;
if (mergeOptions.codecs != null) {
clone.codecs = new Map([
...this.codecs,
...Object.entries(mergeOptions.codecs),
]) as ReadonlyCodecMap;
} else {
clone.codecs = this.codecs;
originalCodecsMap = true;
// It makes sense to just inherit the cached codec context
// as it's expensive to build. The only case when it can't be
// cached is when .withCodecs() was called, but caching will
// work great for all other .withSomething() methods we have.
clone.cachedCodecContext = this.cachedCodecContext;
clone.cachedCodecContextVer = this.cachedCodecContextVer;
}

if (mergeOptions._dropSQLRowCodec && clone.codecs.has("_private_sql_row")) {
// This is an optimization -- if "sql_row" is the only codec defined
// and it's set to "object mode", the we want the codec mapping to be
// empty instead. Why? Empty codec mapping short circuits a lot of
// custom codec code, and object is the default behavior anyway.
if (originalCodecsMap) {
if (clone.codecs === this.codecs) {
// "codecs" map isn't a full-blown read-only mapping, we're just
// treating it as such and protecting from the user to mutate
// directly. This allows for optimizing away pointless copies.
// However, in this case, if we just "inherited" our codecs
// mapping from another options object, we have to clone it
// before deleting any keys or mutating it in any way.
clone.codecs = new Map(clone.codecs);
clone.cachedCodecContext = null;
clone.cachedCodecContextVer = -1;
}
(clone.codecs as MutableCodecMap).delete("_private_sql_row");
}
Expand Down

0 comments on commit 69178c9

Please sign in to comment.