From eb593e18475f7b87a1f6485b15a05dacd2cfef0e Mon Sep 17 00:00:00 2001 From: Chris Trzesniewski Date: Thu, 1 Feb 2024 15:25:07 +0100 Subject: [PATCH 1/2] fix: 224 action hangs in some cases for go fiber benchmarks * fix reExtract regexp to avoid catastrophic backtracking * add test cases from fiber that were causing issues --- src/extract.ts | 4 +- test/data/extract/go_fiber_output.txt | 6 ++ test/extract.spec.ts | 118 +++++++++++++++++++++++++- 3 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 test/data/extract/go_fiber_output.txt diff --git a/src/extract.ts b/src/extract.ts index ff19f2bee..df4fc5368 100644 --- a/src/extract.ts +++ b/src/extract.ts @@ -353,7 +353,7 @@ function extractGoResult(output: string): BenchmarkResult[] { // "A benchmark result line has the general form: [ ...]" // "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." const reExtract = - /^(?Benchmark\w+(?:\/?[\w()$%^&*-=,]*?)*?)(?-\d+)?\s+(?\d+)\s+(?.+)$/; + /^(?Benchmark\w+[\w()$%^&*-=|,[\]{}"#]*?)(?-\d+)?\s+(?\d+)\s+(?.+)$/; for (const line of lines) { const m = line.match(reExtract); @@ -371,7 +371,7 @@ function extractGoResult(output: string): BenchmarkResult[] { const value = parseFloat(pieces[i]); const unit = pieces[i + 1]; let name; - if (pieces.length > 2) { + if (i > 0) { name = m.groups.name + ' - ' + unit; } else { name = m.groups.name; diff --git a/test/data/extract/go_fiber_output.txt b/test/data/extract/go_fiber_output.txt new file mode 100644 index 000000000..92c4e768f --- /dev/null +++ b/test/data/extract/go_fiber_output.txt @@ -0,0 +1,6 @@ +Benchmark_Ctx_Accepts/run-[]string{".xml"}-4 4846809 247.6 ns/op 0 B/op 0 allocs/op +Benchmark_Ctx_Accepts/run-[]string{"json",_"xml"}-4 3900769 307.1 ns/op 0 B/op 0 allocs/op +Benchmark_Ctx_Accepts/run-[]string{"application/json",_"application/xml"}-4 5118646 316.1 ns/op 0 B/op 0 allocs/op +Benchmark_Utils_GetOffer/mime_extension#01-4 3067818 390.7 ns/op 48 B/op 2 allocs/op +Benchmark_Utils_GetOffer/mime_extension#02-4 1992943 602.1 ns/op 48 B/op 2 allocs/op +Benchmark_Utils_GetOffer/mime_extension#03-4 4180965 286.3 ns/op 0 B/op 0 allocs/op diff --git a/test/extract.spec.ts b/test/extract.spec.ts index 575f7da61..eaff1a9c8 100644 --- a/test/extract.spec.ts +++ b/test/extract.spec.ts @@ -210,7 +210,7 @@ describe('extractResult()', function () { extra: '30001 times', }, { - name: 'BenchmarkFib11 - ns/op', + name: 'BenchmarkFib11', unit: 'ns/op', value: 262.7, extra: '4587789 times\n16 procs', @@ -222,7 +222,7 @@ describe('extractResult()', function () { extra: '4587789 times\n16 procs', }, { - name: 'BenchmarkFib22 - ns/op', + name: 'BenchmarkFib22', unit: 'ns/op', value: 31915, extra: '37653 times\n16 procs', @@ -235,6 +235,120 @@ describe('extractResult()', function () { }, ], }, + { + tool: 'go', + file: 'go_fiber_output.txt', + expected: [ + { + name: `Benchmark_Ctx_Accepts/run-[]string{".xml"}`, + unit: 'ns/op', + value: 247.6, + extra: '4846809 times\n4 procs', + }, + { + name: `Benchmark_Ctx_Accepts/run-[]string{".xml"} - B/op`, + unit: 'B/op', + value: 0, + extra: '4846809 times\n4 procs', + }, + { + name: `Benchmark_Ctx_Accepts/run-[]string{".xml"} - allocs/op`, + unit: 'allocs/op', + value: 0, + extra: '4846809 times\n4 procs', + }, + { + name: `Benchmark_Ctx_Accepts/run-[]string{"json",_"xml"}`, + unit: 'ns/op', + value: 307.1, + extra: '3900769 times\n4 procs', + }, + { + name: `Benchmark_Ctx_Accepts/run-[]string{"json",_"xml"} - B/op`, + unit: 'B/op', + value: 0, + extra: '3900769 times\n4 procs', + }, + { + name: `Benchmark_Ctx_Accepts/run-[]string{"json",_"xml"} - allocs/op`, + unit: 'allocs/op', + value: 0, + extra: '3900769 times\n4 procs', + }, + { + name: `Benchmark_Ctx_Accepts/run-[]string{"application/json",_"application/xml"}`, + unit: 'ns/op', + value: 316.1, + extra: '5118646 times\n4 procs', + }, + { + name: `Benchmark_Ctx_Accepts/run-[]string{"application/json",_"application/xml"} - B/op`, + unit: 'B/op', + value: 0, + extra: '5118646 times\n4 procs', + }, + { + name: `Benchmark_Ctx_Accepts/run-[]string{"application/json",_"application/xml"} - allocs/op`, + unit: 'allocs/op', + value: 0, + extra: '5118646 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#01`, + unit: 'ns/op', + value: 390.7, + extra: '3067818 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#01 - B/op`, + unit: 'B/op', + value: 48, + extra: '3067818 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#01 - allocs/op`, + unit: 'allocs/op', + value: 2, + extra: '3067818 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#02`, + unit: 'ns/op', + value: 602.1, + extra: '1992943 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#02 - B/op`, + unit: 'B/op', + value: 48, + extra: '1992943 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#02 - allocs/op`, + unit: 'allocs/op', + value: 2, + extra: '1992943 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#03`, + unit: 'ns/op', + value: 286.3, + extra: '4180965 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#03 - B/op`, + unit: 'B/op', + value: 0, + extra: '4180965 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#03 - allocs/op`, + unit: 'allocs/op', + value: 0, + extra: '4180965 times\n4 procs', + }, + ], + }, { tool: 'benchmarkjs', expected: [ From 90e43a6f5aeb71a214051cfe3be8b12709e8ddef Mon Sep 17 00:00:00 2001 From: Chris Trzesniewski Date: Thu, 1 Feb 2024 15:55:05 +0100 Subject: [PATCH 2/2] * add backwards compatibility for benchmarks that used to have multiple metrics in Go but they were not extracted properly before v1.18.0 --- src/extract.ts | 7 +++++++ test/extract.spec.ts | 48 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/extract.ts b/src/extract.ts index df4fc5368..1bfb47ac2 100644 --- a/src/extract.ts +++ b/src/extract.ts @@ -363,6 +363,13 @@ function extractGoResult(output: string): BenchmarkResult[] { const remainder = m.groups.remainder; const pieces = remainder.split(/[ \t]+/); + + // This is done for backwards compatibility with Go benchmarks that had multiple metrics in output, + // but they were not extracted properly before v1.18.0 + if (pieces.length > 2) { + pieces.unshift(pieces[0], remainder.slice(remainder.indexOf(pieces[1]))); + } + for (let i = 0; i < pieces.length; i = i + 2) { let extra = `${times} times`.replace(/\s\s+/g, ' '); if (procs !== null) { diff --git a/test/extract.spec.ts b/test/extract.spec.ts index eaff1a9c8..f653f940f 100644 --- a/test/extract.spec.ts +++ b/test/extract.spec.ts @@ -211,6 +211,12 @@ describe('extractResult()', function () { }, { name: 'BenchmarkFib11', + unit: 'ns/op\t 3.000 auxMetricUnits', + value: 262.7, + extra: '4587789 times\n16 procs', + }, + { + name: 'BenchmarkFib11 - ns/op', unit: 'ns/op', value: 262.7, extra: '4587789 times\n16 procs', @@ -223,6 +229,12 @@ describe('extractResult()', function () { }, { name: 'BenchmarkFib22', + unit: 'ns/op\t 4.000 auxMetricUnits', + value: 31915, + extra: '37653 times\n16 procs', + }, + { + name: 'BenchmarkFib22 - ns/op', unit: 'ns/op', value: 31915, extra: '37653 times\n16 procs', @@ -241,6 +253,12 @@ describe('extractResult()', function () { expected: [ { name: `Benchmark_Ctx_Accepts/run-[]string{".xml"}`, + unit: 'ns/op 0 B/op 0 allocs/op', + value: 247.6, + extra: '4846809 times\n4 procs', + }, + { + name: `Benchmark_Ctx_Accepts/run-[]string{".xml"} - ns/op`, unit: 'ns/op', value: 247.6, extra: '4846809 times\n4 procs', @@ -259,6 +277,12 @@ describe('extractResult()', function () { }, { name: `Benchmark_Ctx_Accepts/run-[]string{"json",_"xml"}`, + unit: 'ns/op 0 B/op 0 allocs/op', + value: 307.1, + extra: '3900769 times\n4 procs', + }, + { + name: `Benchmark_Ctx_Accepts/run-[]string{"json",_"xml"} - ns/op`, unit: 'ns/op', value: 307.1, extra: '3900769 times\n4 procs', @@ -277,6 +301,12 @@ describe('extractResult()', function () { }, { name: `Benchmark_Ctx_Accepts/run-[]string{"application/json",_"application/xml"}`, + unit: 'ns/op 0 B/op 0 allocs/op', + value: 316.1, + extra: '5118646 times\n4 procs', + }, + { + name: `Benchmark_Ctx_Accepts/run-[]string{"application/json",_"application/xml"} - ns/op`, unit: 'ns/op', value: 316.1, extra: '5118646 times\n4 procs', @@ -295,6 +325,12 @@ describe('extractResult()', function () { }, { name: `Benchmark_Utils_GetOffer/mime_extension#01`, + unit: 'ns/op 48 B/op 2 allocs/op', + value: 390.7, + extra: '3067818 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#01 - ns/op`, unit: 'ns/op', value: 390.7, extra: '3067818 times\n4 procs', @@ -313,6 +349,12 @@ describe('extractResult()', function () { }, { name: `Benchmark_Utils_GetOffer/mime_extension#02`, + unit: 'ns/op 48 B/op 2 allocs/op', + value: 602.1, + extra: '1992943 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#02 - ns/op`, unit: 'ns/op', value: 602.1, extra: '1992943 times\n4 procs', @@ -331,6 +373,12 @@ describe('extractResult()', function () { }, { name: `Benchmark_Utils_GetOffer/mime_extension#03`, + unit: 'ns/op 0 B/op 0 allocs/op', + value: 286.3, + extra: '4180965 times\n4 procs', + }, + { + name: `Benchmark_Utils_GetOffer/mime_extension#03 - ns/op`, unit: 'ns/op', value: 286.3, extra: '4180965 times\n4 procs',