Skip to content

Commit 1a24217

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 1a24217

File tree

2 files changed

+88
-9
lines changed

2 files changed

+88
-9
lines changed

packages/driver/src/baseConn.ts

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ import type { CodecsRegistry } from "./codecs/registry";
2323
import { versionGreaterThan, versionGreaterThanOrEqual } from "./utils";
2424
import * as errors from "./errors";
2525
import { resolveErrorCode, errorFromJSON } from "./errors/resolve";
26-
import { CodecContext, NOOP_CODEC_CONTEXT } from "./codecs/context";
26+
import type { CodecContext } from "./codecs/context";
27+
import { NOOP_CODEC_CONTEXT } from "./codecs/context";
2728
import type {
2829
QueryOptions,
2930
ProtocolVersion,
@@ -680,7 +681,8 @@ export class BaseRawConnection {
680681
capabilitiesFlags: number = RESTRICTED_CAPABILITIES,
681682
options?: QueryOptions,
682683
): Promise<errors.EdgeDBError[]> {
683-
let ctx: CodecContext | null = null;
684+
let ctx = state.makeCodecContext();
685+
684686
const wb = new WriteMessageBuffer();
685687
wb.beginMessage(chars.$O);
686688

@@ -699,9 +701,6 @@ export class BaseRawConnection {
699701
wb.writeBuffer(outCodec.tidBuffer);
700702

701703
if (inCodec) {
702-
if (ctx == null) {
703-
ctx = new CodecContext(state.codecs);
704-
}
705704
wb.writeBuffer(this._encodeArgs(args, inCodec, ctx));
706705
} else {
707706
wb.writeInt32(0);
@@ -716,10 +715,6 @@ export class BaseRawConnection {
716715
let parsing = true;
717716
let warnings: errors.EdgeDBError[] = [];
718717

719-
if (ctx == null) {
720-
ctx = new CodecContext(state.codecs);
721-
}
722-
723718
while (parsing) {
724719
if (!this.buffer.takeMessage()) {
725720
await this._waitForMessage();
@@ -755,6 +750,8 @@ export class BaseRawConnection {
755750

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

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

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)