Add CodSpeed performance benchmarks#153
Conversation
Changed Files
|
| timeout-minutes: 30 | ||
| permissions: | ||
| contents: "read" | ||
| id-token: "write" |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Congrats! CodSpeed is installed 🎉
You will start to see performance impacts in the reports once the benchmarks are run from your default branch.
|
Greptile SummaryThis PR wires up CodSpeed continuous performance benchmarking by extracting the three normalize utilities (
Confidence Score: 4/5Safe to merge; the production code change is a pure refactor with no behavior change, and the benchmark inaccuracies only affect measurement quality. The normalize-body benchmarks measure allocation and JSON serialization overhead on every iteration rather than isolating the function under test, which will produce inflated and misleading baseline numbers in the CodSpeed dashboard from the first run onward. The other issues (missing --run flag, bare semver specifier) are minor. Production code is unaffected. packages/node/tests/bench/normalize-body.bench.ts needs fixtures hoisted out of the bench callbacks before the first CI run establishes baselines. Important Files Changed
Prompt To Fix All With AIFix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
packages/node/tests/bench/normalize-body.bench.ts:6-26
Fixture data created inside bench callbacks inflates benchmark numbers. `Buffer.from()`, `new Uint8Array()`, `JSON.stringify()`, and especially `"x".repeat(1024)` all run on every iteration, so measured timings include allocation and serialization cost rather than just `normalizeBody` performance. `normalize-pem.bench.ts` correctly hoists fixtures to module level — the same pattern should be applied here.
```suggestion
const sampleBuffer = Buffer.from("request body content");
const sampleUint8Array = new Uint8Array([72, 101, 108, 108, 111]);
const largeStringBody = JSON.stringify({ data: "x".repeat(1024) });
describe("normalizeBody", () => {
bench("null input", () => {
normalizeBody(null);
});
bench("string body", () => {
normalizeBody('{"key":"value"}');
});
bench("Buffer body", () => {
normalizeBody(sampleBuffer);
});
bench("Uint8Array body", () => {
normalizeBody(sampleUint8Array);
});
bench("large string body", () => {
normalizeBody(largeStringBody);
});
});
```
### Issue 2 of 4
packages/node/tests/bench/normalize-headers.bench.ts:53-60
The `Map` is constructed inside the bench callback, so every iteration measures both `Map` construction and `normalizeHeaders`. Hoisting it to module level (like the fixtures in `normalize-pem.bench.ts`) isolates the function under test.
```suggestion
const sampleMap = new Map<string, string>([
["Content-Type", "application/json"],
["Accept", "text/html"],
["Authorization", "Bearer token123"],
]);
bench("Map iterable", () => {
normalizeHeaders(sampleMap);
});
```
### Issue 3 of 4
packages/node/package.json:70
The local `bench` script omits `--run`, so `pnpm bench` starts vitest in watch mode and never exits. The CI workflow explicitly adds `--run`; the local script should match to avoid accidental hangs in automation and to produce a deterministic one-shot run when invoked manually.
```suggestion
"bench": "vitest bench --config vitest.bench.config.mts --run"
```
### Issue 4 of 4
packages/node/package.json:77
Every other dev dependency in this file uses the `catalog:` specifier; `@codspeed/vitest-plugin` uses a bare semver range instead. Using `catalog:` keeps the version centralised in the workspace catalog and consistent with the project's conventions.
```suggestion
"@codspeed/vitest-plugin": "catalog:",
```
Reviews (1): Last reviewed commit: "Add CodSpeed benchmarks for normalize ut..." | Re-trigger Greptile |
| describe("normalizeBody", () => { | ||
| bench("null input", () => { | ||
| normalizeBody(null); | ||
| }); | ||
|
|
||
| bench("string body", () => { | ||
| normalizeBody('{"key":"value"}'); | ||
| }); | ||
|
|
||
| bench("Buffer body", () => { | ||
| normalizeBody(Buffer.from("request body content")); | ||
| }); | ||
|
|
||
| bench("Uint8Array body", () => { | ||
| normalizeBody(new Uint8Array([72, 101, 108, 108, 111])); | ||
| }); | ||
|
|
||
| bench("large string body", () => { | ||
| normalizeBody(JSON.stringify({ data: "x".repeat(1024) })); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Fixture data created inside bench callbacks inflates benchmark numbers.
Buffer.from(), new Uint8Array(), JSON.stringify(), and especially "x".repeat(1024) all run on every iteration, so measured timings include allocation and serialization cost rather than just normalizeBody performance. normalize-pem.bench.ts correctly hoists fixtures to module level — the same pattern should be applied here.
| describe("normalizeBody", () => { | |
| bench("null input", () => { | |
| normalizeBody(null); | |
| }); | |
| bench("string body", () => { | |
| normalizeBody('{"key":"value"}'); | |
| }); | |
| bench("Buffer body", () => { | |
| normalizeBody(Buffer.from("request body content")); | |
| }); | |
| bench("Uint8Array body", () => { | |
| normalizeBody(new Uint8Array([72, 101, 108, 108, 111])); | |
| }); | |
| bench("large string body", () => { | |
| normalizeBody(JSON.stringify({ data: "x".repeat(1024) })); | |
| }); | |
| }); | |
| const sampleBuffer = Buffer.from("request body content"); | |
| const sampleUint8Array = new Uint8Array([72, 101, 108, 108, 111]); | |
| const largeStringBody = JSON.stringify({ data: "x".repeat(1024) }); | |
| describe("normalizeBody", () => { | |
| bench("null input", () => { | |
| normalizeBody(null); | |
| }); | |
| bench("string body", () => { | |
| normalizeBody('{"key":"value"}'); | |
| }); | |
| bench("Buffer body", () => { | |
| normalizeBody(sampleBuffer); | |
| }); | |
| bench("Uint8Array body", () => { | |
| normalizeBody(sampleUint8Array); | |
| }); | |
| bench("large string body", () => { | |
| normalizeBody(largeStringBody); | |
| }); | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/node/tests/bench/normalize-body.bench.ts
Line: 6-26
Comment:
Fixture data created inside bench callbacks inflates benchmark numbers. `Buffer.from()`, `new Uint8Array()`, `JSON.stringify()`, and especially `"x".repeat(1024)` all run on every iteration, so measured timings include allocation and serialization cost rather than just `normalizeBody` performance. `normalize-pem.bench.ts` correctly hoists fixtures to module level — the same pattern should be applied here.
```suggestion
const sampleBuffer = Buffer.from("request body content");
const sampleUint8Array = new Uint8Array([72, 101, 108, 108, 111]);
const largeStringBody = JSON.stringify({ data: "x".repeat(1024) });
describe("normalizeBody", () => {
bench("null input", () => {
normalizeBody(null);
});
bench("string body", () => {
normalizeBody('{"key":"value"}');
});
bench("Buffer body", () => {
normalizeBody(sampleBuffer);
});
bench("Uint8Array body", () => {
normalizeBody(sampleUint8Array);
});
bench("large string body", () => {
normalizeBody(largeStringBody);
});
});
```
How can I resolve this? If you propose a fix, please make it concise.| bench("Map iterable", () => { | ||
| const headers = new Map<string, string>([ | ||
| ["Content-Type", "application/json"], | ||
| ["Accept", "text/html"], | ||
| ["Authorization", "Bearer token123"], | ||
| ]); | ||
| normalizeHeaders(headers); | ||
| }); |
There was a problem hiding this comment.
The
Map is constructed inside the bench callback, so every iteration measures both Map construction and normalizeHeaders. Hoisting it to module level (like the fixtures in normalize-pem.bench.ts) isolates the function under test.
| bench("Map iterable", () => { | |
| const headers = new Map<string, string>([ | |
| ["Content-Type", "application/json"], | |
| ["Accept", "text/html"], | |
| ["Authorization", "Bearer token123"], | |
| ]); | |
| normalizeHeaders(headers); | |
| }); | |
| const sampleMap = new Map<string, string>([ | |
| ["Content-Type", "application/json"], | |
| ["Accept", "text/html"], | |
| ["Authorization", "Bearer token123"], | |
| ]); | |
| bench("Map iterable", () => { | |
| normalizeHeaders(sampleMap); | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/node/tests/bench/normalize-headers.bench.ts
Line: 53-60
Comment:
The `Map` is constructed inside the bench callback, so every iteration measures both `Map` construction and `normalizeHeaders`. Hoisting it to module level (like the fixtures in `normalize-pem.bench.ts`) isolates the function under test.
```suggestion
const sampleMap = new Map<string, string>([
["Content-Type", "application/json"],
["Accept", "text/html"],
["Authorization", "Bearer token123"],
]);
bench("Map iterable", () => {
normalizeHeaders(sampleMap);
});
```
How can I resolve this? If you propose a fix, please make it concise.| "pack-addon": "slsa pack dist/node_reqwest-v{version}-{platform}-{arch}.node.gz", | ||
| "postinstall": "slsa wget" | ||
| "postinstall": "slsa wget", | ||
| "bench": "vitest bench --config vitest.bench.config.mts" |
There was a problem hiding this comment.
The local
bench script omits --run, so pnpm bench starts vitest in watch mode and never exits. The CI workflow explicitly adds --run; the local script should match to avoid accidental hangs in automation and to produce a deterministic one-shot run when invoked manually.
| "bench": "vitest bench --config vitest.bench.config.mts" | |
| "bench": "vitest bench --config vitest.bench.config.mts --run" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/node/package.json
Line: 70
Comment:
The local `bench` script omits `--run`, so `pnpm bench` starts vitest in watch mode and never exits. The CI workflow explicitly adds `--run`; the local script should match to avoid accidental hangs in automation and to produce a deterministic one-shot run when invoked manually.
```suggestion
"bench": "vitest bench --config vitest.bench.config.mts --run"
```
How can I resolve this? If you propose a fix, please make it concise.| }, | ||
| "devDependencies": { | ||
| "@codecov/vite-plugin": "catalog:", | ||
| "@codspeed/vitest-plugin": "^5.4.0", |
There was a problem hiding this comment.
Every other dev dependency in this file uses the
catalog: specifier; @codspeed/vitest-plugin uses a bare semver range instead. Using catalog: keeps the version centralised in the workspace catalog and consistent with the project's conventions.
| "@codspeed/vitest-plugin": "^5.4.0", | |
| "@codspeed/vitest-plugin": "catalog:", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/node/package.json
Line: 77
Comment:
Every other dev dependency in this file uses the `catalog:` specifier; `@codspeed/vitest-plugin` uses a bare semver range instead. Using `catalog:` keeps the version centralised in the workspace catalog and consistent with the project's conventions.
```suggestion
"@codspeed/vitest-plugin": "catalog:",
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
This PR adds continuous performance benchmarking to node_reqwest using CodSpeed with vitest benchmarks in simulation mode.
What changed
Refactored normalize utilities into a standalone module
Extracted
normalizeHeaders,normalizePem, andnormalizeBodyfrompackages/node/export/agent.tsinto a newpackages/node/export/normalize.ts. These are pure TypeScript functions that don't depend on the native addon, making them independently testable and benchmarkable. Theagent.tsmodule now imports fromnormalize.ts-- no behavior change.Added 18 vitest benchmarks across three suites
normalizeHeaders(7 benchmarks): empty input, object with string values, object with array values, object with many headers, flat string array pairs, Map iterable, mixed value typesnormalizePem(6 benchmarks): undefined input, single string, single Buffer, array of strings, array of Buffers, mixed arraynormalizeBody(5 benchmarks): null input, string body, Buffer body, Uint8Array body, large string bodyAdded CodSpeed CI workflow (
.github/workflows/codspeed.yaml)Runs benchmarks on pushes to
main, pull requests, and manual dispatch. Uses OIDC authentication and simulation mode for deterministic, low-variance measurements.Added CodSpeed badge to README.
Notes on runners
This repository is owned by a personal GitHub account, so CodSpeed's dedicated macro runners are not available. The workflow uses
ubuntu-latestrunners. Since these benchmarks run in simulation mode (CPU instruction counting), this does not affect measurement accuracy -- simulation mode produces deterministic results regardless of the runner hardware.Next steps
corecrate and the Node.js binding layer, add benchmarks for the native addon calls (e.g., Agent construction, HTTP dispatch)agentDispatchis implemented