Skip to content

Commit 184ef1e

Browse files
committed
Implement CodecContext caching in the Options object.
This can give a noticeable perf boost in a regular use case scenario.
1 parent 9df6ac9 commit 184ef1e

File tree

2 files changed

+86
-8
lines changed

2 files changed

+86
-8
lines changed

packages/driver/src/baseConn.ts

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,8 @@ export class BaseRawConnection {
680680
capabilitiesFlags: number = RESTRICTED_CAPABILITIES,
681681
options?: QueryOptions,
682682
): Promise<errors.EdgeDBError[]> {
683-
let ctx: CodecContext | null = null;
683+
let ctx = state.makeCodecContext();
684+
684685
const wb = new WriteMessageBuffer();
685686
wb.beginMessage(chars.$O);
686687

@@ -699,9 +700,6 @@ export class BaseRawConnection {
699700
wb.writeBuffer(outCodec.tidBuffer);
700701

701702
if (inCodec) {
702-
if (ctx == null) {
703-
ctx = new CodecContext(state.codecs);
704-
}
705703
wb.writeBuffer(this._encodeArgs(args, inCodec, ctx));
706704
} else {
707705
wb.writeInt32(0);
@@ -716,10 +714,6 @@ export class BaseRawConnection {
716714
let parsing = true;
717715
let warnings: errors.EdgeDBError[] = [];
718716

719-
if (ctx == null) {
720-
ctx = new CodecContext(state.codecs);
721-
}
722-
723717
while (parsing) {
724718
if (!this.buffer.takeMessage()) {
725719
await this._waitForMessage();
@@ -755,6 +749,8 @@ export class BaseRawConnection {
755749

756750
case chars.$T: {
757751
try {
752+
ctx = state.makeCodecContext()
753+
758754
const [
759755
newCard,
760756
newInCodec,
@@ -764,6 +760,37 @@ export class BaseRawConnection {
764760
__,
765761
_warnings,
766762
] = this._parseDescribeTypeMessage(query);
763+
764+
/* Quoting the docs:
765+
766+
- If the declared input type descriptor does not match
767+
the expected value, a CommandDataDescription message is
768+
returned followed by a ParameterTypeMismatchError in
769+
an ErrorResponse message.
770+
771+
- If the declared output type descriptor does not match,
772+
the server will send a CommandDataDescription prior
773+
to sending any Data messages.
774+
775+
Therefore, basically receiving CommandDataDescription
776+
means that our codecs have outdate knowledge of the schema.
777+
The only exception to that is if our codecs were NULL codecs
778+
in the first place, in which case we're here because we want
779+
to learn
780+
*/
781+
if (
782+
(outCodec !== NULL_CODEC && outCodec.tid != newOutCodec.tid) ||
783+
(inCodec !== NULL_CODEC && inCodec.tid != newInCodec.tid)
784+
) {
785+
Options.signalSchemaChange();
786+
787+
// If this was the result of outCodec mismatch, we'll get
788+
// the information to build a new one, build it, and
789+
// then continue this loop to receiving and parsing data
790+
// messages. In this case we want a fresh new CodecContext.
791+
ctx = state.makeCodecContext();
792+
}
793+
767794
const key = this._getQueryCacheKey(
768795
query,
769796
outputFormat,
@@ -778,12 +805,30 @@ export class BaseRawConnection {
778805
outCodec = newOutCodec;
779806
warnings = _warnings;
780807
} catch (e: any) {
808+
// An error happened, so we don't know if we bumped the internal
809+
// schema tracker or not, so let's do it again to be on the safe
810+
// side.
811+
Options.signalSchemaChange();
812+
813+
// Keep parsing the buffer, we'll raise it later.
781814
error = e;
782815
}
783816
break;
784817
}
785818

786819
case chars.$s: {
820+
// Quoting docs:
821+
//
822+
// If the declared state type descriptor does not match
823+
// the expected value, a StateDataDescription message is
824+
// returned followed by a StateMismatchError in
825+
// an ErrorResponse message
826+
//
827+
// If we're here it means the state data descriptor has changed,
828+
// which can be the result of a new global added, which is a schema
829+
// changes. So let's signal it just to be safe.
830+
Options.signalSchemaChange();
831+
787832
this._parseDescribeStateMessage();
788833
break;
789834
}

packages/driver/src/options.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { utf8Encoder } from "./primitives/buffer";
33
import type { Mutable } from "./typeutil";
44
import type { Codecs } from "./codecs/codecs";
55
import type { ReadonlyCodecMap } from "./codecs/context";
6+
import { CodecContext, NOOP_CODEC_CONTEXT } from "./codecs/context";
67

78
export type BackoffFunction = (n: number) => number;
89

@@ -158,6 +159,8 @@ export interface OptionsList {
158159
export class Options {
159160
// This type is immutable.
160161

162+
private static schemaVersion: number = 0;
163+
161164
readonly module: string;
162165
readonly moduleAliases: ReadonlyMap<string, string>;
163166
readonly config: ReadonlyMap<string, any>;
@@ -170,10 +173,17 @@ export class Options {
170173
/** @internal */
171174
readonly annotations: ReadonlyMap<string, string> = new Map<string, string>();
172175

176+
private cachedCodecContext: CodecContext | null = null;
177+
private cachedCodecContextVer = -1;
178+
173179
get tag(): string | null {
174180
return this.annotations.get(TAG_ANNOTATION_KEY) ?? null;
175181
}
176182

183+
static signalSchemaChange() {
184+
this.schemaVersion += 1;
185+
}
186+
177187
constructor({
178188
retryOptions = RetryOptions.defaults(),
179189
transactionOptions = TransactionOptions.defaults(),
@@ -194,6 +204,29 @@ export class Options {
194204
this.codecs = new Map(Object.entries(codecs)) as ReadonlyCodecMap;
195205
}
196206

207+
public makeCodecContext(): CodecContext {
208+
if (this.codecs.size === 0) {
209+
return NOOP_CODEC_CONTEXT;
210+
}
211+
212+
if (this.cachedCodecContextVer === Options.schemaVersion) {
213+
/* We really want to cache CodecContext because it works fast
214+
when it's "warm", i.e. enough kinds of scalar types went through
215+
it to populate its internal mapping of 'type name => user codec'.
216+
This is a complication of the fact that users can define client
217+
codecs for user-defined scalar types, and correct codec selection
218+
requires correct "order of resolution", the cost of building which
219+
is non-negligible.
220+
*/
221+
return this.cachedCodecContext!;
222+
}
223+
224+
const ctx = new CodecContext(this.codecs);
225+
this.cachedCodecContext = ctx;
226+
this.cachedCodecContextVer = Options.schemaVersion;
227+
return ctx;
228+
}
229+
197230
private _cloneWith(mergeOptions: OptionsList) {
198231
const clone: Mutable<Options> = Object.create(Options.prototype);
199232

0 commit comments

Comments
 (0)