Skip to content

Commit 2a2de7a

Browse files
GuillaumeLagrangeGuillaumeLagrange
committed
feat: use instrument hooks for measurements
1 parent 6940327 commit 2a2de7a

File tree

18 files changed

+315
-96
lines changed

18 files changed

+315
-96
lines changed

‎.prettierignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@ node_modules
22
.rollup.cache
33
dist
44
generated
5+
packages/core/src/native_core/instruments/hooks

‎packages/benchmark.js-plugin/src/index.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {
2-
Measurement,
2+
InstrumentHooks,
33
mongoMeasurement,
44
optimizeFunction,
55
optimizeFunctionSync,
@@ -90,7 +90,7 @@ export function withCodSpeed(item: unknown): unknown {
9090
}
9191

9292
function withCodSpeedBenchmark(bench: Benchmark): WithCodSpeedBenchmark {
93-
if (!Measurement.isInstrumented()) {
93+
if (!InstrumentHooks.isInstrumented()) {
9494
const rawRun = bench.run;
9595
bench.run = (options?: Benchmark.Options) => {
9696
console.warn(
@@ -120,7 +120,7 @@ function withCodSpeedBenchmark(bench: Benchmark): WithCodSpeedBenchmark {
120120
}
121121

122122
function withCodSpeedSuite(suite: Benchmark.Suite): WithCodSpeedSuite {
123-
if (!Measurement.isInstrumented()) {
123+
if (!InstrumentHooks.isInstrumented()) {
124124
const rawRun = suite.run;
125125
suite.run = (options?: Benchmark.Options) => {
126126
console.warn(
@@ -198,18 +198,20 @@ async function runBenchmarks({
198198
await mongoMeasurement.start(uri);
199199
global.gc?.();
200200
await (async function __codspeed_root_frame__() {
201-
Measurement.startInstrumentation();
201+
const benchmarkId = InstrumentHooks.startBenchmark();
202202
await benchPayload();
203-
Measurement.stopInstrumentation(uri);
203+
InstrumentHooks.stopBenchmark();
204+
InstrumentHooks.setExecutedBenchmark(benchmarkId, uri);
204205
})();
205206
await mongoMeasurement.stop(uri);
206207
} else {
207208
optimizeFunctionSync(benchPayload);
208209
await mongoMeasurement.start(uri);
209210
(function __codspeed_root_frame__() {
210-
Measurement.startInstrumentation();
211+
const benchmarkId = InstrumentHooks.startBenchmark();
211212
benchPayload();
212-
Measurement.stopInstrumentation(uri);
213+
InstrumentHooks.stopBenchmark();
214+
InstrumentHooks.setExecutedBenchmark(benchmarkId, uri);
213215
})();
214216
await mongoMeasurement.stop(uri);
215217
}
@@ -231,7 +233,7 @@ async function runBenchmarks({
231233
export async function setupInstruments(
232234
body: SetupInstrumentsRequestBody
233235
): Promise<SetupInstrumentsResponse> {
234-
if (!Measurement.isInstrumented()) {
236+
if (!InstrumentHooks.isInstrumented()) {
235237
console.warn("[CodSpeed] No instrumentation found, using default mongoUrl");
236238

237239
return { remoteAddr: body.mongoUrl };

‎packages/benchmark.js-plugin/tests/index.integ.test.ts

Lines changed: 45 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ jest.mock("@codspeed/core", () => {
1515
beforeEach(() => {
1616
mockReset(mockCore);
1717
jest.clearAllMocks();
18+
// Set up mock return values
19+
mockCore.InstrumentHooks.startBenchmark.mockReturnValue(1);
1820
});
1921

2022
const benchOptions: Benchmark.Options = {
@@ -23,7 +25,7 @@ const benchOptions: Benchmark.Options = {
2325

2426
describe("Benchmark", () => {
2527
it("simple benchmark", async () => {
26-
mockCore.Measurement.isInstrumented.mockReturnValue(false);
28+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(false);
2729
const bench = withCodSpeed(
2830
new Benchmark(
2931
"RegExp",
@@ -37,11 +39,11 @@ describe("Benchmark", () => {
3739
bench.on("complete", onComplete);
3840
await bench.run();
3941
expect(onComplete).toHaveBeenCalled();
40-
expect(mockCore.Measurement.startInstrumentation).not.toHaveBeenCalled();
41-
expect(mockCore.Measurement.stopInstrumentation).not.toHaveBeenCalled();
42+
expect(mockCore.InstrumentHooks.startBenchmark).not.toHaveBeenCalled();
43+
expect(mockCore.InstrumentHooks.stopBenchmark).not.toHaveBeenCalled();
4244
});
4345
it("check core methods are called", async () => {
44-
mockCore.Measurement.isInstrumented.mockReturnValue(true);
46+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(true);
4547

4648
const bench = withCodSpeed(
4749
new Benchmark(
@@ -56,13 +58,15 @@ describe("Benchmark", () => {
5658
bench.on("complete", onComplete);
5759
await bench.run();
5860
expect(onComplete).toHaveBeenCalled();
59-
expect(mockCore.Measurement.startInstrumentation).toHaveBeenCalled();
60-
expect(mockCore.Measurement.stopInstrumentation).toHaveBeenCalledWith(
61+
expect(mockCore.InstrumentHooks.startBenchmark).toHaveBeenCalled();
62+
expect(mockCore.InstrumentHooks.stopBenchmark).toHaveBeenCalled();
63+
expect(mockCore.InstrumentHooks.setExecutedBenchmark).toHaveBeenCalledWith(
64+
expect.any(Number),
6165
"packages/benchmark.js-plugin/tests/index.integ.test.ts::RegExpSingle"
6266
);
6367
});
6468
it("check error handling", async () => {
65-
mockCore.Measurement.isInstrumented.mockReturnValue(true);
69+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(true);
6670
const bench = withCodSpeed(
6771
new Benchmark(
6872
"throwing",
@@ -79,7 +83,7 @@ describe("Benchmark", () => {
7983
async (instrumented) => {
8084
const logSpy = jest.spyOn(console, "log");
8185
const warnSpy = jest.spyOn(console, "warn");
82-
mockCore.Measurement.isInstrumented.mockReturnValue(instrumented);
86+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(instrumented);
8387
await withCodSpeed(
8488
new Benchmark(
8589
"RegExpSingle",
@@ -108,7 +112,7 @@ describe("Benchmark", () => {
108112
}
109113
);
110114
it("should call setup and teardown", async () => {
111-
mockCore.Measurement.isInstrumented.mockReturnValue(true);
115+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(true);
112116
const setup = jest.fn();
113117
const teardown = jest.fn();
114118
const bench = withCodSpeed(
@@ -128,7 +132,7 @@ describe("Benchmark", () => {
128132

129133
describe("Benchmark.Suite", () => {
130134
it("simple suite", async () => {
131-
mockCore.Measurement.isInstrumented.mockReturnValue(false);
135+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(false);
132136
const suite = withCodSpeed(new Benchmark.Suite());
133137
suite.add(
134138
"RegExp",
@@ -141,11 +145,11 @@ describe("Benchmark.Suite", () => {
141145
suite.on("complete", onComplete);
142146
await suite.run({ maxTime: 0.1, initCount: 1 });
143147
expect(onComplete).toHaveBeenCalled();
144-
expect(mockCore.Measurement.startInstrumentation).not.toHaveBeenCalled();
145-
expect(mockCore.Measurement.stopInstrumentation).not.toHaveBeenCalled();
148+
expect(mockCore.InstrumentHooks.startBenchmark).not.toHaveBeenCalled();
149+
expect(mockCore.InstrumentHooks.stopBenchmark).not.toHaveBeenCalled();
146150
});
147151
it("check core methods are called", async () => {
148-
mockCore.Measurement.isInstrumented.mockReturnValue(true);
152+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(true);
149153
const suite = withCodSpeed(new Benchmark.Suite()).add(
150154
"RegExp",
151155
function () {
@@ -156,13 +160,15 @@ describe("Benchmark.Suite", () => {
156160
const onComplete = jest.fn();
157161
suite.on("complete", onComplete);
158162
await suite.run({ maxTime: 0.1, initCount: 1 });
159-
expect(mockCore.Measurement.startInstrumentation).toHaveBeenCalled();
160-
expect(mockCore.Measurement.stopInstrumentation).toHaveBeenCalledWith(
163+
expect(mockCore.InstrumentHooks.startBenchmark).toHaveBeenCalled();
164+
expect(mockCore.InstrumentHooks.stopBenchmark).toHaveBeenCalled();
165+
expect(mockCore.InstrumentHooks.setExecutedBenchmark).toHaveBeenCalledWith(
166+
expect.any(Number),
161167
"packages/benchmark.js-plugin/tests/index.integ.test.ts::RegExp"
162168
);
163169
});
164170
it("check suite name is in the uri", async () => {
165-
mockCore.Measurement.isInstrumented.mockReturnValue(true);
171+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(true);
166172
await withCodSpeed(new Benchmark.Suite("thesuite"))
167173
.add(
168174
"RegExp",
@@ -175,15 +181,19 @@ describe("Benchmark.Suite", () => {
175181
/o/.test("Hello World!");
176182
}, benchOptions)
177183
.run();
178-
expect(mockCore.Measurement.stopInstrumentation).toHaveBeenCalledWith(
184+
expect(mockCore.InstrumentHooks.stopBenchmark).toHaveBeenCalled();
185+
expect(mockCore.InstrumentHooks.setExecutedBenchmark).toHaveBeenCalledWith(
186+
expect.any(Number),
179187
"packages/benchmark.js-plugin/tests/index.integ.test.ts::thesuite::RegExp"
180188
);
181-
expect(mockCore.Measurement.stopInstrumentation).toHaveBeenCalledWith(
189+
expect(mockCore.InstrumentHooks.stopBenchmark).toHaveBeenCalled();
190+
expect(mockCore.InstrumentHooks.setExecutedBenchmark).toHaveBeenCalledWith(
191+
expect.any(Number),
182192
"packages/benchmark.js-plugin/tests/index.integ.test.ts::thesuite::unknown_1"
183193
);
184194
});
185195
it("check error handling", async () => {
186-
mockCore.Measurement.isInstrumented.mockReturnValue(true);
196+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(true);
187197
const bench = withCodSpeed(new Benchmark.Suite("thesuite")).add(
188198
"throwing",
189199
() => {
@@ -197,7 +207,7 @@ describe("Benchmark.Suite", () => {
197207
async (instrumented) => {
198208
const logSpy = jest.spyOn(console, "log");
199209
const warnSpy = jest.spyOn(console, "warn");
200-
mockCore.Measurement.isInstrumented.mockReturnValue(instrumented);
210+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(instrumented);
201211
await withCodSpeed(new Benchmark.Suite("thesuite"))
202212
.add(
203213
"RegExp",
@@ -229,35 +239,41 @@ describe("Benchmark.Suite", () => {
229239
}
230240
);
231241
it("check nested file path is in the uri when bench is registered in another file", async () => {
232-
mockCore.Measurement.isInstrumented.mockReturnValue(true);
242+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(true);
233243
const suite = withCodSpeed(new Benchmark.Suite("thesuite"));
234244
registerBenchmarks(suite);
235245
const onComplete = jest.fn();
236246
suite.on("complete", onComplete);
237247
await suite.run({ maxTime: 0.1, initCount: 1 });
238-
expect(mockCore.Measurement.startInstrumentation).toHaveBeenCalled();
239-
expect(mockCore.Measurement.stopInstrumentation).toHaveBeenCalledWith(
248+
expect(mockCore.InstrumentHooks.startBenchmark).toHaveBeenCalled();
249+
expect(mockCore.InstrumentHooks.stopBenchmark).toHaveBeenCalled();
250+
expect(mockCore.InstrumentHooks.setExecutedBenchmark).toHaveBeenCalledWith(
251+
expect.any(Number),
240252
"packages/benchmark.js-plugin/tests/registerBenchmarks.ts::thesuite::RegExp"
241253
);
242254
});
243255
it("check that benchmarks with same name have different URIs when registered in different files", async () => {
244-
mockCore.Measurement.isInstrumented.mockReturnValue(true);
256+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(true);
245257
const suite = withCodSpeed(new Benchmark.Suite("thesuite"));
246258
registerBenchmarks(suite);
247259
registerOtherBenchmarks(suite);
248260
const onComplete = jest.fn();
249261
suite.on("complete", onComplete);
250262
await suite.run({ maxTime: 0.1, initCount: 1 });
251-
expect(mockCore.Measurement.startInstrumentation).toHaveBeenCalled();
252-
expect(mockCore.Measurement.stopInstrumentation).toHaveBeenCalledWith(
263+
expect(mockCore.InstrumentHooks.startBenchmark).toHaveBeenCalled();
264+
expect(mockCore.InstrumentHooks.stopBenchmark).toHaveBeenCalled();
265+
expect(mockCore.InstrumentHooks.setExecutedBenchmark).toHaveBeenCalledWith(
266+
expect.any(Number),
253267
"packages/benchmark.js-plugin/tests/registerBenchmarks.ts::thesuite::RegExp"
254268
);
255-
expect(mockCore.Measurement.stopInstrumentation).toHaveBeenCalledWith(
269+
expect(mockCore.InstrumentHooks.stopBenchmark).toHaveBeenCalled();
270+
expect(mockCore.InstrumentHooks.setExecutedBenchmark).toHaveBeenCalledWith(
271+
expect.any(Number),
256272
"packages/benchmark.js-plugin/tests/registerOtherBenchmarks.ts::thesuite::RegExp"
257273
);
258274
});
259275
it("should call setupCore and teardownCore only once after run()", async () => {
260-
mockCore.Measurement.isInstrumented.mockReturnValue(true);
276+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(true);
261277
const suite = withCodSpeed(new Benchmark.Suite("thesuite"));
262278
registerBenchmarks(suite);
263279
registerOtherBenchmarks(suite);
@@ -271,7 +287,7 @@ describe("Benchmark.Suite", () => {
271287
expect(mockCore.teardownCore).toHaveBeenCalledTimes(1);
272288
});
273289
it("should call setup and teardown", async () => {
274-
mockCore.Measurement.isInstrumented.mockReturnValue(true);
290+
mockCore.InstrumentHooks.isInstrumented.mockReturnValue(true);
275291
const setup = jest.fn();
276292
const teardown = jest.fn();
277293

‎packages/core/binding.gyp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,30 @@
88
"cflags_cc!": [
99
"-fno-exceptions"
1010
],
11+
"cflags": [
12+
"-Wno-maybe-uninitialized",
13+
"-Wno-unused-variable",
14+
"-Wno-unused-parameter",
15+
"-Wno-unused-but-set-variable",
16+
"-Wno-type-limits"
17+
],
18+
"cflags_cc": [
19+
"-Wno-maybe-uninitialized",
20+
"-Wno-unused-variable",
21+
"-Wno-unused-parameter",
22+
"-Wno-unused-but-set-variable",
23+
"-Wno-type-limits"
24+
],
1125
"sources": [
12-
"src/native_core/measurement/measurement.cc",
1326
"src/native_core/linux_perf/linux_perf.cc",
1427
"src/native_core/linux_perf/linux_perf_listener.cc",
28+
"src/native_core/instruments/hooks_wrapper.cc",
29+
"src/native_core/instruments/hooks/dist/core.c",
1530
"src/native_core/native_core.cc"
1631
],
1732
"include_dirs": [
18-
"<!@(node -p \"require('node-addon-api').include\")"
33+
"<!@(node -p \"require('node-addon-api').include\")",
34+
"src/native_core/instruments/hooks/includes"
1935
],
2036
"defines": [
2137
"NAPI_DISABLE_CPP_EXCEPTIONS"

‎packages/core/moon.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ tasks:
1313
inputs:
1414
- "src/native_core/**/*.cc"
1515
- "src/native_core/**/*.h"
16+
- "src/native_core/**/*.c"
1617
- "binding.gyp"
1718
outputs:
1819
- "prebuilds"

‎packages/core/src/index.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,7 @@ export const isBound = native_core.isBound;
1111
export const mongoMeasurement = new MongoMeasurement();
1212

1313
export const setupCore = () => {
14-
native_core.Measurement.stopInstrumentation(
15-
`Metadata: codspeed-node ${__VERSION__}`
16-
);
14+
native_core.InstrumentHooks.setIntegration("codspeed-node", __VERSION__);
1715
linuxPerf.start();
1816
checkV8Flags();
1917
};
@@ -29,4 +27,4 @@ export type {
2927
export { getV8Flags, tryIntrospect } from "./introspection";
3028
export { optimizeFunction, optimizeFunctionSync } from "./optimization";
3129
export * from "./utils";
32-
export const Measurement = native_core.Measurement;
30+
export const InstrumentHooks = native_core.InstrumentHooks;

‎packages/core/src/native_core/index.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import path from "path";
2+
import { InstrumentHooks } from "./instruments/hooks";
23
import { LinuxPerf } from "./linux_perf/linux_perf";
3-
import { Measurement } from "./measurement/measurement";
4+
45
interface NativeCore {
5-
Measurement: Measurement;
6+
InstrumentHooks: InstrumentHooks;
67
LinuxPerf: typeof LinuxPerf;
78
}
89

@@ -22,12 +23,12 @@ try {
2223
};
2324
} catch (e) {
2425
native_core = {
25-
Measurement: {
26+
InstrumentHooks: {
2627
isInstrumented: () => false,
27-
// eslint-disable-next-line @typescript-eslint/no-empty-function
28-
startInstrumentation: () => {},
29-
// eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-empty-function
30-
stopInstrumentation: (at) => {},
28+
startBenchmark: () => 1,
29+
stopBenchmark: () => 1,
30+
setExecutedBenchmark: () => 1,
31+
setIntegration: () => 1,
3132
},
3233
LinuxPerf: class LinuxPerf {
3334
start() {

‎packages/core/src/native_core/instrument-hooks

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎packages/core/src/native_core/instruments/hooks.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export interface InstrumentHooks {
2+
isInstrumented: () => boolean;
3+
startBenchmark: () => number;
4+
stopBenchmark: () => number;
5+
setExecutedBenchmark: (pid: number, uri: string) => number;
6+
setIntegration: (name: string, version: string) => number;
7+
}

0 commit comments

Comments
 (0)