From c6256cc22dc893101e5f7e5e71aa19235dd0ce26 Mon Sep 17 00:00:00 2001 From: zermelo-wisen Date: Tue, 26 Mar 2024 20:10:49 +0300 Subject: [PATCH] perf: Prevent unnecessary event object creation Defer event object cretion in all relevant Recording methods until shouldRecord() check inside Recording.emit() --- src/Recording.ts | 124 +++++++++++++++++++------------------------- src/hooks/http.ts | 4 ++ src/hooks/mongo.ts | 29 ++++++++--- src/hooks/mysql.ts | 3 ++ src/hooks/pg.ts | 10 ++-- src/hooks/prisma.ts | 11 ++-- src/hooks/sqlite.ts | 10 +++- src/recorder.ts | 9 ++-- 8 files changed, 109 insertions(+), 91 deletions(-) diff --git a/src/Recording.ts b/src/Recording.ts index 762226f8..de014f05 100644 --- a/src/Recording.ts +++ b/src/Recording.ts @@ -34,31 +34,21 @@ export default class Recording { public readonly path; public metadata: AppMap.Metadata; - functionCall(funInfo: FunctionInfo, thisArg: unknown, args: unknown[]): AppMap.FunctionCallEvent { + functionCall(funInfo: FunctionInfo, thisArg: unknown, args: unknown[]) { this.functionsSeen.add(funInfo); - const event = makeCallEvent(this.nextId++, funInfo, thisArg, args); - this.emit(event); - return event; + return this.emit(() => makeCallEvent(this.nextId++, funInfo, thisArg, args)); } - functionException( - callId: number, - exception: unknown, - elapsed?: number, - ): AppMap.FunctionReturnEvent { - const event = makeExceptionEvent(this.nextId++, callId, exception, elapsed); - this.emit(event); - return event; + functionException(callId: number, exception: unknown, elapsed?: number) { + return this.emit(() => makeExceptionEvent(this.nextId++, callId, exception, elapsed)); } - functionReturn(callId: number, result: unknown, elapsed?: number): AppMap.FunctionReturnEvent { - const event = makeReturnEvent(this.nextId++, callId, result, elapsed); - this.emit(event); - return event; + functionReturn(callId: number, result: unknown, elapsed?: number) { + return this.emit(() => makeReturnEvent(this.nextId++, callId, result, elapsed)); } - sqlQuery(databaseType: string, sql: string): AppMap.SqlQueryEvent { - const event: AppMap.SqlQueryEvent = { + sqlQuery(databaseType: string, sql: string) { + const createEvent = (): AppMap.SqlQueryEvent => ({ event: "call", sql_query: compactObject({ database_type: databaseType, @@ -66,17 +56,12 @@ export default class Recording { }), id: this.nextId++, thread_id: 0, - }; - this.emit(event); - return event; + }); + return this.emit(createEvent); } - httpClientRequest( - method: string, - url: string, - headers?: Record, - ): AppMap.HttpClientRequestEvent { - const event: AppMap.HttpClientRequestEvent = { + httpClientRequest(method: string, url: string, headers?: Record) { + const createEvent = (): AppMap.HttpClientRequestEvent => ({ event: "call", http_client_request: compactObject({ request_method: method, @@ -85,10 +70,8 @@ export default class Recording { }), id: this.nextId++, thread_id: 0, - }; - this.emit(event); - - return event; + }); + return this.emit(createEvent); } httpClientResponse( @@ -97,8 +80,8 @@ export default class Recording { status: number, headers?: Record, returnValue?: AppMap.Parameter, - ): AppMap.HttpClientResponseEvent { - const event: AppMap.HttpClientResponseEvent = { + ) { + const createEvent = (): AppMap.HttpClientResponseEvent => ({ event: "return", http_client_response: compactObject({ status_code: status, @@ -109,10 +92,8 @@ export default class Recording { thread_id: 0, parent_id: callId, elapsed, - }; - this.emit(event); - - return event; + }); + return this.emit(createEvent); } httpRequest( @@ -121,31 +102,34 @@ export default class Recording { protocol?: string, headers?: Record, params?: URLSearchParams, - ): AppMap.HttpServerRequestEvent { - const event: AppMap.HttpServerRequestEvent = { - event: "call", - http_server_request: compactObject({ - path_info: path, - request_method: method, - headers, - protocol, - }), - id: this.nextId++, - thread_id: 0, - }; - const query = params && Array.from(params); - if (query && query.length > 0) { - event.message = []; - for (const [name, value] of params) { - event.message.push({ - name, - value: inspect(value), - class: "String", - }); + ) { + const createEvent = () => { + const event: AppMap.HttpServerRequestEvent = { + event: "call", + http_server_request: compactObject({ + path_info: path, + request_method: method, + headers, + protocol, + }), + id: this.nextId++, + thread_id: 0, + }; + const query = params && Array.from(params); + if (query && query.length > 0) { + event.message = []; + for (const [name, value] of params) { + event.message.push({ + name, + value: inspect(value), + class: "String", + }); + } } - } - this.emit(event); - return event; + return event; + }; + + return this.emit(createEvent); } httpResponse( @@ -154,8 +138,8 @@ export default class Recording { status: number, headers?: Record, returnValue?: AppMap.Parameter, - ): AppMap.HttpServerResponseEvent { - const event: AppMap.HttpServerResponseEvent = { + ) { + const createEvent = (): AppMap.HttpServerResponseEvent => ({ event: "return", http_server_response: compactObject({ status_code: status, @@ -166,20 +150,16 @@ export default class Recording { thread_id: 0, parent_id: callId, elapsed, - }; - this.emit(event); - - return event; + }); + return this.emit(createEvent); } - private emit(event: unknown) { - // Check here if we should record instead of requiring each - // possible hook to check it. - // Checking this inside hooks too, will save CPU cycles only - // by preventing unnecessary event creation. + private emit(createEvent: () => E) { if (!shouldRecord()) return; assert(this.stream); + const event = createEvent(); this.stream.emit(event); + return event; } private eventUpdates: Record = {}; diff --git a/src/hooks/http.ts b/src/hooks/http.ts index a38e189e..5f4d2abe 100644 --- a/src/hooks/http.ts +++ b/src/hooks/http.ts @@ -106,6 +106,9 @@ function handleClientRequest(request: http.ClientRequest) { `${url.protocol}//${url.host}${url.pathname}`, normalizeHeaders(request.getHeaders()), ); + // clientRequestEvent event can be undefined if the recording is paused + // temporarily for reasons like code block recording. + if (!clientRequestEvent) return; request.on("response", (response) => { const capture = new BodyCapture(); @@ -264,6 +267,7 @@ function handleRequest(request: http.IncomingMessage, response: http.ServerRespo normalizeHeaders(request.headers), url.searchParams, ); + if (!requestEvent) return; const capture = new BodyCapture(); captureResponseBody(response, capture); diff --git a/src/hooks/mongo.ts b/src/hooks/mongo.ts index 6db1737f..ca256c81 100644 --- a/src/hooks/mongo.ts +++ b/src/hooks/mongo.ts @@ -108,8 +108,12 @@ function patchMethod>( const startTime = getTime(); args.push((err: unknown, res: unknown) => { setCustomInspect(res, customInspect); - if (err) recording.functionException(callEvent.id, err, getTime() - startTime); - else recording.functionReturn(callEvent.id, res, getTime() - startTime); + // callEvent can be undefined if the recording is paused + // temporarily for reasons like code block recording. + if (callEvent) { + if (err) recording.functionException(callEvent.id, err, getTime() - startTime); + else recording.functionReturn(callEvent.id, res, getTime() - startTime); + } return callback(err, res) as unknown; }); return Reflect.apply(original, this, args) as ReturnType; @@ -120,13 +124,22 @@ function patchMethod>( try { const result = Reflect.apply(original, this, args) as unknown; - setCustomInspect(result, customInspect); - const returnEvent = recording.functionReturn(callEvent.id, result, getTime() - startTime); - return fixReturnEventIfPromiseResult(result, returnEvent, callEvent, startTime) as ReturnType< - typeof original - >; + // callEvent can be undefined if the recording is paused + // temporarily for reasons like code block recording. + if (callEvent) { + setCustomInspect(result, customInspect); + const returnEvent = recording.functionReturn(callEvent.id, result, getTime() - startTime); + if (returnEvent) + return fixReturnEventIfPromiseResult( + result, + returnEvent, + callEvent, + startTime, + ) as ReturnType; + } + return result as ReturnType; } catch (exn: unknown) { - recording.functionException(callEvent.id, exn, getTime() - startTime); + if (callEvent) recording.functionException(callEvent.id, exn, getTime() - startTime); throw exn; } }; diff --git a/src/hooks/mysql.ts b/src/hooks/mysql.ts index 0025a90e..01eec699 100644 --- a/src/hooks/mysql.ts +++ b/src/hooks/mysql.ts @@ -44,6 +44,9 @@ function createQueryProxy(query: mysql.QueryFunction) { const sql: string = hasStringSqlProperty(argArray[0]) ? argArray[0].sql : argArray[0]; const call = recording.sqlQuery("mysql", sql); + // If recording is paused or stopped, call will be undefined + if (!call) return Reflect.apply(target, thisArg, argArray); + const start = getTime(); const originalCallback = diff --git a/src/hooks/pg.ts b/src/hooks/pg.ts index 30096c56..9affd320 100644 --- a/src/hooks/pg.ts +++ b/src/hooks/pg.ts @@ -31,9 +31,13 @@ function createQueryProxy( const call = recording.sqlQuery("postgres", sql); const start = getTime(); const result = target.apply(thisArg, argArray); - const ret = recording.functionReturn(call.id, result, getTime() - start); - - return fixReturnEventIfPromiseResult(result, ret, call, start); + // call event can be undefined if the recording is paused + // temporarily for reasons like code block recording. + if (call) { + const ret = recording.functionReturn(call.id, result, getTime() - start); + if (ret) return fixReturnEventIfPromiseResult(result, ret, call, start); + } + return result; }, }); } diff --git a/src/hooks/prisma.ts b/src/hooks/prisma.ts index aab59449..0c5eaf4b 100644 --- a/src/hooks/prisma.ts +++ b/src/hooks/prisma.ts @@ -106,8 +106,12 @@ function createProxy unknown>( assert("$on" in thisArg && typeof thisArg.$on === "function"); thisArg.$on("query", (queryEvent: QueryEvent) => { const call = recording.sqlQuery(dbType, queryEvent.query); - const elapsedSec = queryEvent.duration / 1000.0; - recording.functionReturn(call.id, undefined, elapsedSec); + // call event can be undefined if the recording is paused + // temporarily for reasons like code block recording. + if (call) { + const elapsedSec = queryEvent.duration / 1000.0; + recording.functionReturn(call.id, undefined, elapsedSec); + } }); } @@ -130,7 +134,8 @@ function createProxy unknown>( try { const result = target.apply(thisArg, argArray); const ret = recording.functionReturn(prismaCall.id, result, getTime() - start); - return fixReturnEventIfPromiseResult(result, ret, prismaCall, start); + if (ret) return fixReturnEventIfPromiseResult(result, ret, prismaCall, start); + return result; } catch (exn: unknown) { recording.functionException(prismaCall.id, exn, getTime() - start); throw exn; diff --git a/src/hooks/sqlite.ts b/src/hooks/sqlite.ts index 63cdecbc..8d170a0f 100644 --- a/src/hooks/sqlite.ts +++ b/src/hooks/sqlite.ts @@ -38,6 +38,8 @@ function createRecordingProxy( ) { return new Proxy(proxyTarget, { apply(target, thisArg, argArray: Parameters) { + const shortCircuit = () => Reflect.apply(target, thisArg, argArray) as T; + // Extract sql. If thisArg is a Statement then it has a sql property. // Otherwise thisArg is a Database and the sql must be the first element of the argArray. let sql: string; @@ -45,13 +47,17 @@ function createRecordingProxy( else { // If there is no sql provided, short circuit to the original function // to make it throw an error. - if (argArray.length === 0 || typeof argArray[0] !== "string") - return Reflect.apply(target, thisArg, argArray) as T; + if (argArray.length === 0 || typeof argArray[0] !== "string") return shortCircuit(); sql = argArray[0]; } const call = recording.sqlQuery("sqlite", sql); + // call event can be undefined if the recording is paused + // temporarily for reasons like code block recording. + + if (call == undefined) return shortCircuit(); + const start = getTime(); // Extract callback argument(s) to functionArgs diff --git a/src/recorder.ts b/src/recorder.ts index 6098ef2e..ce88ab38 100644 --- a/src/recorder.ts +++ b/src/recorder.ts @@ -31,10 +31,13 @@ export function record( try { const result = fun.apply(this, args); - const ret = recording.functionReturn(call.id, result, getTime() - start); - return fixReturnEventIfPromiseResult(result, ret, call, start) as Return; + if (call) { + const ret = recording.functionReturn(call.id, result, getTime() - start); + if (ret) return fixReturnEventIfPromiseResult(result, ret, call, start) as Return; + } + return result; } catch (exn: unknown) { - recording.functionException(call.id, exn, getTime() - start); + if (call) recording.functionException(call.id, exn, getTime() - start); throw exn; } }