Skip to content

Commit

Permalink
perf: Prevent unnecessary event object creation
Browse files Browse the repository at this point in the history
Defer event object cretion in all relevant Recording methods until
shouldRecord() check inside Recording.emit()
  • Loading branch information
zermelo-wisen committed Mar 26, 2024
1 parent ace21dc commit c6256cc
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 91 deletions.
124 changes: 52 additions & 72 deletions src/Recording.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,49 +34,34 @@ 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,
sql,
}),
id: this.nextId++,
thread_id: 0,
};
this.emit(event);
return event;
});
return this.emit(createEvent);
}

httpClientRequest(
method: string,
url: string,
headers?: Record<string, string>,
): AppMap.HttpClientRequestEvent {
const event: AppMap.HttpClientRequestEvent = {
httpClientRequest(method: string, url: string, headers?: Record<string, string>) {
const createEvent = (): AppMap.HttpClientRequestEvent => ({
event: "call",
http_client_request: compactObject({
request_method: method,
Expand All @@ -85,10 +70,8 @@ export default class Recording {
}),
id: this.nextId++,
thread_id: 0,
};
this.emit(event);

return event;
});
return this.emit(createEvent);
}

httpClientResponse(
Expand All @@ -97,8 +80,8 @@ export default class Recording {
status: number,
headers?: Record<string, string>,
returnValue?: AppMap.Parameter,
): AppMap.HttpClientResponseEvent {
const event: AppMap.HttpClientResponseEvent = {
) {
const createEvent = (): AppMap.HttpClientResponseEvent => ({
event: "return",
http_client_response: compactObject({
status_code: status,
Expand All @@ -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(
Expand All @@ -121,31 +102,34 @@ export default class Recording {
protocol?: string,
headers?: Record<string, string>,
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(
Expand All @@ -154,8 +138,8 @@ export default class Recording {
status: number,
headers?: Record<string, string>,
returnValue?: AppMap.Parameter,
): AppMap.HttpServerResponseEvent {
const event: AppMap.HttpServerResponseEvent = {
) {
const createEvent = (): AppMap.HttpServerResponseEvent => ({
event: "return",
http_server_response: compactObject({
status_code: status,
Expand All @@ -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<E>(createEvent: () => E) {
if (!shouldRecord()) return;
assert(this.stream);
const event = createEvent();
this.stream.emit(event);
return event;
}

private eventUpdates: Record<number, AppMap.Event> = {};
Expand Down
4 changes: 4 additions & 0 deletions src/hooks/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
29 changes: 21 additions & 8 deletions src/hooks/mongo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,12 @@ function patchMethod<K extends MethodLikeKeys<mongodb.Collection>>(
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<typeof original>;
Expand All @@ -120,13 +124,22 @@ function patchMethod<K extends MethodLikeKeys<mongodb.Collection>>(

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<typeof original>;
}
return result as ReturnType<typeof original>;
} catch (exn: unknown) {
recording.functionException(callEvent.id, exn, getTime() - startTime);
if (callEvent) recording.functionException(callEvent.id, exn, getTime() - startTime);
throw exn;
}
};
Expand Down
3 changes: 3 additions & 0 deletions src/hooks/mysql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
10 changes: 7 additions & 3 deletions src/hooks/pg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},
});
}
11 changes: 8 additions & 3 deletions src/hooks/prisma.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,12 @@ function createProxy<T extends (...args: unknown[]) => 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);
}
});
}

Expand All @@ -130,7 +134,8 @@ function createProxy<T extends (...args: unknown[]) => 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;
Expand Down
10 changes: 8 additions & 2 deletions src/hooks/sqlite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,26 @@ function createRecordingProxy<T extends RecordingProxyTarget>(
) {
return new Proxy(proxyTarget, {
apply(target, thisArg, argArray: Parameters<typeof proxyTarget>) {
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;
if (hasSqlStringProperty(thisArg)) sql = thisArg.sql;
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
Expand Down
9 changes: 6 additions & 3 deletions src/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,13 @@ export function record<This, Return>(

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;
}
}
Expand Down

0 comments on commit c6256cc

Please sign in to comment.