Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: 224 action hangs in some cases for go fiber benchmarks #225

Merged
merged 2 commits into from
Feb 2, 2024
+177 −2
Merged

fix: 224 action hangs in some cases for go fiber benchmarks #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb593e1
fix: 224 action hangs in some cases for go fiber benchmarks
ktrz Feb 1, 2024
90e43a6
* add backwards compatibility for benchmarks that used to have multip…
ktrz Feb 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/extract.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
}

interface PullRequest {
[key: string]: any;

Check warning on line 32 in src/extract.ts

View workflow job for this annotation

GitHub Actions / Run linting and formatting check 

Unexpected any. Specify a different type
number: number;
html_url?: string;
body?: string;
Expand Down Expand Up @@ -353,7 +353,7 @@
// "A benchmark result line has the general form: <name> <iterations> <value> <unit> [<value> <unit>...]"
// "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 =
/^(?<name>Benchmark\w+(?:\/?[\w()$%^&*-=,]*?)*?)(?<procs>-\d+)?\s+(?<times>\d+)\s+(?<remainder>.+)$/;
/^(?<name>Benchmark\w+[\w()$%^&*-=|,[\]{}"#]*?)(?<procs>-\d+)?\s+(?<times>\d+)\s+(?<remainder>.+)$/;
Copy link
Member Author

@ktrz ktrz Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds some additional characters that were present in Go Fiber benchmarks as well as removes the nested (()*?)*? which was causing the regex to hang in some instances


for (const line of lines) {
const m = line.match(reExtract);
Expand All @@ -363,6 +363,13 @@
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) {
Expand All @@ -371,7 +378,7 @@
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;
Expand Down Expand Up @@ -430,7 +437,7 @@
const extra = `mean: ${mean} ${meanUnit}\nrounds: ${stats.rounds}`;
return { name, value, unit, range, extra };
});
} catch (err: any) {

Check warning on line 440 in src/extract.ts

View workflow job for this annotation

GitHub Actions / Run linting and formatting check 

Unexpected any. Specify a different type
throw new Error(
`Output file for 'pytest' must be JSON file generated by --benchmark-json option: ${err.message}`,
);
Expand All @@ -441,7 +448,7 @@
let json: GoogleCppBenchmarkJson;
try {
json = JSON.parse(output);
} catch (err: any) {

Check warning on line 451 in src/extract.ts

View workflow job for this annotation

GitHub Actions / Run linting and formatting check 

Unexpected any. Specify a different type
throw new Error(
`Output file for 'googlecpp' must be JSON file generated by --benchmark_format=json option: ${err.message}`,
);
Expand Down Expand Up @@ -569,7 +576,7 @@
return ret;
}

function extractJuliaBenchmarkHelper([_, bench]: JuliaBenchmarkGroup, labels: string[] = []): BenchmarkResult[] {

Check warning on line 579 in src/extract.ts

View workflow job for this annotation

GitHub Actions / Run linting and formatting check 

'_' is defined but never used
const res: BenchmarkResult[] = [];
for (const key in bench.data) {
const value = bench.data[key];
Expand Down Expand Up @@ -600,7 +607,7 @@
let json: JuliaBenchmarkJson;
try {
json = JSON.parse(output);
} catch (err: any) {

Check warning on line 610 in src/extract.ts

View workflow job for this annotation

GitHub Actions / Run linting and formatting check 

Unexpected any. Specify a different type
throw new Error(
`Output file for 'julia' must be JSON file generated by BenchmarkTools.save("output.json", suit::BenchmarkGroup) : ${err.message}`,
);
Expand All @@ -618,7 +625,7 @@
let json: JmhBenchmarkJson[];
try {
json = JSON.parse(output);
} catch (err: any) {

Check warning on line 628 in src/extract.ts

View workflow job for this annotation

GitHub Actions / Run linting and formatting check 

Unexpected any. Specify a different type
throw new Error(`Output file for 'jmh' must be JSON file generated by -rf json option: ${err.message}`);
}
return json.map((b) => {
Expand All @@ -635,7 +642,7 @@
let json: BenchmarkDotNetBenchmarkJson;
try {
json = JSON.parse(output);
} catch (err: any) {

Check warning on line 645 in src/extract.ts

View workflow job for this annotation

GitHub Actions / Run linting and formatting check 

Unexpected any. Specify a different type
throw new Error(
`Output file for 'benchmarkdotnet' must be JSON file generated by '--exporters json' option or by adding the JsonExporter to your run config: ${err.message}`,
);
Expand All @@ -656,7 +663,7 @@
return json.map(({ name, value, unit, range, extra }) => {
return { name, value, unit, range, extra };
});
} catch (err: any) {

Check warning on line 666 in src/extract.ts

View workflow job for this annotation

GitHub Actions / Run linting and formatting check 

Unexpected any. Specify a different type
throw new Error(
`Output file for 'custom-(bigger|smaller)-is-better' must be JSON file containing an array of entries in BenchmarkResult format: ${err.message}`,
);
Expand Down
6 changes: 6 additions & 0 deletions test/data/extract/go_fiber_output.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
162 changes: 162 additions & 0 deletions test/extract.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,12 @@ describe('extractResult()', function () {
value: 40537.456,
extra: '30001 times',
},
{
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the backward compatible metric as before v1.18.0 it was resulting like that in case of multiple metrics

name: 'BenchmarkFib11',
unit: 'ns/op\t 3.000 auxMetricUnits',
value: 262.7,
extra: '4587789 times\n16 procs',
},
{
name: 'BenchmarkFib11 - ns/op',
unit: 'ns/op',
Expand All @@ -221,6 +227,12 @@ describe('extractResult()', function () {
value: 3,
extra: '4587789 times\n16 procs',
},
{
name: 'BenchmarkFib22',
unit: 'ns/op\t 4.000 auxMetricUnits',
value: 31915,
extra: '37653 times\n16 procs',
},
{
name: 'BenchmarkFib22 - ns/op',
unit: 'ns/op',
Expand All @@ -235,6 +247,156 @@ describe('extractResult()', function () {
},
],
},
{
tool: 'go',
file: 'go_fiber_output.txt',
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',
},
{
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 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',
},
{
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 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',
},
{
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 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',
},
{
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 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',
},
{
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 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',
},
{
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: [
Expand Down
Loading