Skip to content

Commit 1c81dfd

Browse files
ktrzktrz
authored
fix: 224 action hangs in some cases for go fiber benchmarks (#225)
* fix reExtract regexp to avoid catastrophic backtracking * add test cases from fiber that were causing issues * add backwards compatibility for benchmarks that used to have multiple metrics in Go but they were not extracted properly before v1.18.0
1 parent e108afd commit 1c81dfd

File tree

3 files changed

+177
-2
lines changed

3 files changed

+177
-2
lines changed

‎src/extract.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ function extractGoResult(output: string): BenchmarkResult[] {
353353
// "A benchmark result line has the general form: <name> <iterations> <value> <unit> [<value> <unit>...]"
354354
// "The fields are separated by runs of space characters (as defined by unicode.IsSpace), so the line can be parsed with strings.Fields. The line must have an even number of fields, and at least four."
355355
const reExtract =
356-
/^(?<name>Benchmark\w+(?:\/?[\w()$%^&*-=,]*?)*?)(?<procs>-\d+)?\s+(?<times>\d+)\s+(?<remainder>.+)$/;
356+
/^(?<name>Benchmark\w+[\w()$%^&*-=|,[\]{}"#]*?)(?<procs>-\d+)?\s+(?<times>\d+)\s+(?<remainder>.+)$/;
357357

358358
for (const line of lines) {
359359
const m = line.match(reExtract);
@@ -363,6 +363,13 @@ function extractGoResult(output: string): BenchmarkResult[] {
363363
const remainder = m.groups.remainder;
364364

365365
const pieces = remainder.split(/[ \t]+/);
366+
367+
// This is done for backwards compatibility with Go benchmarks that had multiple metrics in output,
368+
// but they were not extracted properly before v1.18.0
369+
if (pieces.length > 2) {
370+
pieces.unshift(pieces[0], remainder.slice(remainder.indexOf(pieces[1])));
371+
}
372+
366373
for (let i = 0; i < pieces.length; i = i + 2) {
367374
let extra = `${times} times`.replace(/\s\s+/g, ' ');
368375
if (procs !== null) {
@@ -371,7 +378,7 @@ function extractGoResult(output: string): BenchmarkResult[] {
371378
const value = parseFloat(pieces[i]);
372379
const unit = pieces[i + 1];
373380
let name;
374-
if (pieces.length > 2) {
381+
if (i > 0) {
375382
name = m.groups.name + ' - ' + unit;
376383
} else {
377384
name = m.groups.name;

‎test/data/extract/go_fiber_output.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Benchmark_Ctx_Accepts/run-[]string{".xml"}-4 4846809 247.6 ns/op 0 B/op 0 allocs/op
2+
Benchmark_Ctx_Accepts/run-[]string{"json",_"xml"}-4 3900769 307.1 ns/op 0 B/op 0 allocs/op
3+
Benchmark_Ctx_Accepts/run-[]string{"application/json",_"application/xml"}-4 5118646 316.1 ns/op 0 B/op 0 allocs/op
4+
Benchmark_Utils_GetOffer/mime_extension#01-4 3067818 390.7 ns/op 48 B/op 2 allocs/op
5+
Benchmark_Utils_GetOffer/mime_extension#02-4 1992943 602.1 ns/op 48 B/op 2 allocs/op
6+
Benchmark_Utils_GetOffer/mime_extension#03-4 4180965 286.3 ns/op 0 B/op 0 allocs/op

‎test/extract.spec.ts

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,12 @@ describe('extractResult()', function () {
209209
value: 40537.456,
210210
extra: '30001 times',
211211
},
212+
{
213+
name: 'BenchmarkFib11',
214+
unit: 'ns/op\t 3.000 auxMetricUnits',
215+
value: 262.7,
216+
extra: '4587789 times\n16 procs',
217+
},
212218
{
213219
name: 'BenchmarkFib11 - ns/op',
214220
unit: 'ns/op',
@@ -221,6 +227,12 @@ describe('extractResult()', function () {
221227
value: 3,
222228
extra: '4587789 times\n16 procs',
223229
},
230+
{
231+
name: 'BenchmarkFib22',
232+
unit: 'ns/op\t 4.000 auxMetricUnits',
233+
value: 31915,
234+
extra: '37653 times\n16 procs',
235+
},
224236
{
225237
name: 'BenchmarkFib22 - ns/op',
226238
unit: 'ns/op',
@@ -235,6 +247,156 @@ describe('extractResult()', function () {
235247
},
236248
],
237249
},
250+
{
251+
tool: 'go',
252+
file: 'go_fiber_output.txt',
253+
expected: [
254+
{
255+
name: `Benchmark_Ctx_Accepts/run-[]string{".xml"}`,
256+
unit: 'ns/op 0 B/op 0 allocs/op',
257+
value: 247.6,
258+
extra: '4846809 times\n4 procs',
259+
},
260+
{
261+
name: `Benchmark_Ctx_Accepts/run-[]string{".xml"} - ns/op`,
262+
unit: 'ns/op',
263+
value: 247.6,
264+
extra: '4846809 times\n4 procs',
265+
},
266+
{
267+
name: `Benchmark_Ctx_Accepts/run-[]string{".xml"} - B/op`,
268+
unit: 'B/op',
269+
value: 0,
270+
extra: '4846809 times\n4 procs',
271+
},
272+
{
273+
name: `Benchmark_Ctx_Accepts/run-[]string{".xml"} - allocs/op`,
274+
unit: 'allocs/op',
275+
value: 0,
276+
extra: '4846809 times\n4 procs',
277+
},
278+
{
279+
name: `Benchmark_Ctx_Accepts/run-[]string{"json",_"xml"}`,
280+
unit: 'ns/op 0 B/op 0 allocs/op',
281+
value: 307.1,
282+
extra: '3900769 times\n4 procs',
283+
},
284+
{
285+
name: `Benchmark_Ctx_Accepts/run-[]string{"json",_"xml"} - ns/op`,
286+
unit: 'ns/op',
287+
value: 307.1,
288+
extra: '3900769 times\n4 procs',
289+
},
290+
{
291+
name: `Benchmark_Ctx_Accepts/run-[]string{"json",_"xml"} - B/op`,
292+
unit: 'B/op',
293+
value: 0,
294+
extra: '3900769 times\n4 procs',
295+
},
296+
{
297+
name: `Benchmark_Ctx_Accepts/run-[]string{"json",_"xml"} - allocs/op`,
298+
unit: 'allocs/op',
299+
value: 0,
300+
extra: '3900769 times\n4 procs',
301+
},
302+
{
303+
name: `Benchmark_Ctx_Accepts/run-[]string{"application/json",_"application/xml"}`,
304+
unit: 'ns/op 0 B/op 0 allocs/op',
305+
value: 316.1,
306+
extra: '5118646 times\n4 procs',
307+
},
308+
{
309+
name: `Benchmark_Ctx_Accepts/run-[]string{"application/json",_"application/xml"} - ns/op`,
310+
unit: 'ns/op',
311+
value: 316.1,
312+
extra: '5118646 times\n4 procs',
313+
},
314+
{
315+
name: `Benchmark_Ctx_Accepts/run-[]string{"application/json",_"application/xml"} - B/op`,
316+
unit: 'B/op',
317+
value: 0,
318+
extra: '5118646 times\n4 procs',
319+
},
320+
{
321+
name: `Benchmark_Ctx_Accepts/run-[]string{"application/json",_"application/xml"} - allocs/op`,
322+
unit: 'allocs/op',
323+
value: 0,
324+
extra: '5118646 times\n4 procs',
325+
},
326+
{
327+
name: `Benchmark_Utils_GetOffer/mime_extension#01`,
328+
unit: 'ns/op 48 B/op 2 allocs/op',
329+
value: 390.7,
330+
extra: '3067818 times\n4 procs',
331+
},
332+
{
333+
name: `Benchmark_Utils_GetOffer/mime_extension#01 - ns/op`,
334+
unit: 'ns/op',
335+
value: 390.7,
336+
extra: '3067818 times\n4 procs',
337+
},
338+
{
339+
name: `Benchmark_Utils_GetOffer/mime_extension#01 - B/op`,
340+
unit: 'B/op',
341+
value: 48,
342+
extra: '3067818 times\n4 procs',
343+
},
344+
{
345+
name: `Benchmark_Utils_GetOffer/mime_extension#01 - allocs/op`,
346+
unit: 'allocs/op',
347+
value: 2,
348+
extra: '3067818 times\n4 procs',
349+
},
350+
{
351+
name: `Benchmark_Utils_GetOffer/mime_extension#02`,
352+
unit: 'ns/op 48 B/op 2 allocs/op',
353+
value: 602.1,
354+
extra: '1992943 times\n4 procs',
355+
},
356+
{
357+
name: `Benchmark_Utils_GetOffer/mime_extension#02 - ns/op`,
358+
unit: 'ns/op',
359+
value: 602.1,
360+
extra: '1992943 times\n4 procs',
361+
},
362+
{
363+
name: `Benchmark_Utils_GetOffer/mime_extension#02 - B/op`,
364+
unit: 'B/op',
365+
value: 48,
366+
extra: '1992943 times\n4 procs',
367+
},
368+
{
369+
name: `Benchmark_Utils_GetOffer/mime_extension#02 - allocs/op`,
370+
unit: 'allocs/op',
371+
value: 2,
372+
extra: '1992943 times\n4 procs',
373+
},
374+
{
375+
name: `Benchmark_Utils_GetOffer/mime_extension#03`,
376+
unit: 'ns/op 0 B/op 0 allocs/op',
377+
value: 286.3,
378+
extra: '4180965 times\n4 procs',
379+
},
380+
{
381+
name: `Benchmark_Utils_GetOffer/mime_extension#03 - ns/op`,
382+
unit: 'ns/op',
383+
value: 286.3,
384+
extra: '4180965 times\n4 procs',
385+
},
386+
{
387+
name: `Benchmark_Utils_GetOffer/mime_extension#03 - B/op`,
388+
unit: 'B/op',
389+
value: 0,
390+
extra: '4180965 times\n4 procs',
391+
},
392+
{
393+
name: `Benchmark_Utils_GetOffer/mime_extension#03 - allocs/op`,
394+
unit: 'allocs/op',
395+
value: 0,
396+
extra: '4180965 times\n4 procs',
397+
},
398+
],
399+
},
238400
{
239401
tool: 'benchmarkjs',
240402
expected: [

0 commit comments

Comments
 (0)