-
Notifications
You must be signed in to change notification settings - Fork 180
fix: include package name for duplicate bench names [#264] #330
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
69ea6d4
7fe60a9
1430858
34121ba
cb8cc78
ae1c338
f5dbe13
c4d55d2
3d0659f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| reviews: | ||
| pre_merge_checks: | ||
| docstrings: | ||
| mode: "off" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| /coverage | ||
| /dist | ||
| /.idea | ||
| /.ai | ||
| target/ | ||
| jmh-result.json | ||
| .env | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -34,7 +34,7 @@ | |||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| interface PullRequest { | ||||||||||||||||||||||
| [key: string]: any; | ||||||||||||||||||||||
| number: number; | ||||||||||||||||||||||
| html_url?: string; | ||||||||||||||||||||||
| body?: string; | ||||||||||||||||||||||
|
|
@@ -343,9 +343,16 @@ | |||||||||||||||||||||
| return ret; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| function extractGoResult(output: string): BenchmarkResult[] { | ||||||||||||||||||||||
| const lines = output.split(/\r?\n/g); | ||||||||||||||||||||||
| const ret = []; | ||||||||||||||||||||||
| export function extractGoResult(output: string): BenchmarkResult[] { | ||||||||||||||||||||||
| // Split into sections by "pkg:" lines, keeping package name with each section | ||||||||||||||||||||||
| const sections = output.split(/^pkg:\s+/m).map((section, index) => { | ||||||||||||||||||||||
| if (index === 0) return { pkg: '', lines: section.split(/\r?\n/g) }; | ||||||||||||||||||||||
| const [pkg, ...rest] = section.split(/\r?\n/g); | ||||||||||||||||||||||
| return { pkg, lines: rest }; | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const hasMultiplePackages = sections.filter((s) => s.pkg).length > 1; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Example: | ||||||||||||||||||||||
| // BenchmarkFib20-8 30000 41653 ns/op | ||||||||||||||||||||||
| // BenchmarkDoWithConfigurer1-8 30000000 42.3 ns/op | ||||||||||||||||||||||
|
|
@@ -356,15 +363,19 @@ | |||||||||||||||||||||
| // reference, "Proposal: Go Benchmark Data Format": https://go.googlesource.com/proposal/+/master/design/14313-benchmark-format.md | ||||||||||||||||||||||
| // "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 = | ||||||||||||||||||||||
| const reExtractRegexp = | ||||||||||||||||||||||
| /^(?<name>Benchmark\w+[\w()$%^&*-=|,[\]{}"#]*?)(?<procs>-\d+)?\s+(?<times>\d+)\s+(?<remainder>.+)$/; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| for (const line of lines) { | ||||||||||||||||||||||
| const m = line.match(reExtract); | ||||||||||||||||||||||
| if (m?.groups) { | ||||||||||||||||||||||
| const procs = m.groups.procs !== undefined ? m.groups.procs.slice(1) : null; | ||||||||||||||||||||||
| const times = m.groups.times; | ||||||||||||||||||||||
| const remainder = m.groups.remainder; | ||||||||||||||||||||||
| // Flatten sections into lines with package context, then process each line | ||||||||||||||||||||||
| return sections | ||||||||||||||||||||||
| .flatMap(({ pkg, lines }) => lines.map((line) => ({ pkg, line }))) | ||||||||||||||||||||||
| .flatMap(({ pkg, line }) => { | ||||||||||||||||||||||
| const match = line.match(reExtractRegexp); | ||||||||||||||||||||||
| if (!match?.groups) return []; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const { name, procs: procsRaw, times, remainder } = match.groups; | ||||||||||||||||||||||
| const procs = procsRaw?.slice(1) ?? null; | ||||||||||||||||||||||
| const extra = procs !== null ? `${times} times\n${procs} procs` : `${times} times`; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const pieces = remainder.split(/[ \t]+/); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -374,25 +385,19 @@ | |||||||||||||||||||||
| 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) { | ||||||||||||||||||||||
| extra += `\n${procs} procs`; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| const value = parseFloat(pieces[i]); | ||||||||||||||||||||||
| const unit = pieces[i + 1]; | ||||||||||||||||||||||
| let name; | ||||||||||||||||||||||
| if (i > 0) { | ||||||||||||||||||||||
| name = m.groups.name + ' - ' + unit; | ||||||||||||||||||||||
| } else { | ||||||||||||||||||||||
| name = m.groups.name; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| ret.push({ name, value, unit, extra }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| const baseName = hasMultiplePackages && pkg ? `${name} (${pkg})` : name; | ||||||||||||||||||||||
| // Chunk into [value, unit] pairs and map to results | ||||||||||||||||||||||
| return chunkPairs(pieces).map(([valueStr, unit], i) => ({ | ||||||||||||||||||||||
| name: i > 0 ? `${baseName} - ${unit}` : baseName, | ||||||||||||||||||||||
| value: parseFloat(valueStr), | ||||||||||||||||||||||
| unit, | ||||||||||||||||||||||
| extra, | ||||||||||||||||||||||
| })); | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return ret; | ||||||||||||||||||||||
| function chunkPairs(arr: string[]): Array<[string, string]> { | ||||||||||||||||||||||
| return Array.from({ length: Math.floor(arr.length / 2) }, (_, i) => [arr[i * 2], arr[i * 2 + 1]]); | ||||||||||||||||||||||
|
Comment on lines
+399
to
+400
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard against odd-length value/unit sequences.
🛡️ Suggested validation function chunkPairs(arr: string[]): Array<[string, string]> {
+ if (arr.length % 2 !== 0) {
+ throw new Error(
+ `Malformed Go benchmark metrics (expected value/unit pairs, got ${arr.length} fields): ${arr.join(' ')}`
+ );
+ }
return Array.from({ length: Math.floor(arr.length / 2) }, (_, i) => [arr[i * 2], arr[i * 2 + 1]]);
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| function extractBenchmarkJsResult(output: string): BenchmarkResult[] { | ||||||||||||||||||||||
|
|
@@ -441,7 +446,7 @@ | |||||||||||||||||||||
| const extra = `mean: ${mean} ${meanUnit}\nrounds: ${stats.rounds}`; | ||||||||||||||||||||||
| return { name, value, unit, range, extra }; | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||
| `Output file for 'pytest' must be JSON file generated by --benchmark-json option: ${err.message}`, | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
|
|
@@ -452,7 +457,7 @@ | |||||||||||||||||||||
| let json: GoogleCppBenchmarkJson; | ||||||||||||||||||||||
| try { | ||||||||||||||||||||||
| json = JSON.parse(output); | ||||||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||
| `Output file for 'googlecpp' must be JSON file generated by --benchmark_format=json option: ${err.message}`, | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
|
|
@@ -580,7 +585,7 @@ | |||||||||||||||||||||
| return ret; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| function extractJuliaBenchmarkHelper([_, bench]: JuliaBenchmarkGroup, labels: string[] = []): BenchmarkResult[] { | ||||||||||||||||||||||
| const res: BenchmarkResult[] = []; | ||||||||||||||||||||||
| for (const key in bench.data) { | ||||||||||||||||||||||
| const value = bench.data[key]; | ||||||||||||||||||||||
|
|
@@ -611,7 +616,7 @@ | |||||||||||||||||||||
| let json: JuliaBenchmarkJson; | ||||||||||||||||||||||
| try { | ||||||||||||||||||||||
| json = JSON.parse(output); | ||||||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||
| `Output file for 'julia' must be JSON file generated by BenchmarkTools.save("output.json", suit::BenchmarkGroup) : ${err.message}`, | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
|
|
@@ -629,7 +634,7 @@ | |||||||||||||||||||||
| let json: JmhBenchmarkJson[]; | ||||||||||||||||||||||
| try { | ||||||||||||||||||||||
| json = JSON.parse(output); | ||||||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||||||
| throw new Error(`Output file for 'jmh' must be JSON file generated by -rf json option: ${err.message}`); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| return json.map((b) => { | ||||||||||||||||||||||
|
|
@@ -646,7 +651,7 @@ | |||||||||||||||||||||
| let json: BenchmarkDotNetBenchmarkJson; | ||||||||||||||||||||||
| try { | ||||||||||||||||||||||
| json = JSON.parse(output); | ||||||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||||||
| 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}`, | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| PASS | ||
| ok github.com/gofiber/fiber/v3/log 0.173s | ||
| PASS | ||
| ok github.com/gofiber/fiber/v3/middleware/adaptor 0.184s | ||
| PASS | ||
| ok github.com/gofiber/fiber/v3/middleware/basicauth 0.173s | ||
| goos: linux | ||
| goarch: amd64 | ||
| pkg: github.com/gofiber/fiber/v3/middleware/cache | ||
| cpu: AMD EPYC 7763 64-Core Processor | ||
| BenchmarkAppendMsgitem-4 21228057 55.76 ns/op 2833.37 MB/s 0 B/op 0 allocs/op | ||
| BenchmarkUnmarshalitem-4 6855655 174.0 ns/op 907.83 MB/s 0 B/op 0 allocs/op | ||
| PASS | ||
| ok github.com/gofiber/fiber/v3/middleware/cache 2.649s | ||
| PASS | ||
| ok github.com/gofiber/fiber/v3/middleware/compress 0.219s | ||
| PASS | ||
| ok github.com/gofiber/fiber/v3/middleware/cors 0.188s | ||
| goos: linux | ||
| goarch: amd64 | ||
| pkg: github.com/gofiber/fiber/v3/middleware/csrf | ||
| cpu: AMD EPYC 7763 64-Core Processor | ||
| BenchmarkAppendMsgitem-4 1000000000 0.3733 ns/op 2678.46 MB/s 0 B/op 0 allocs/op | ||
| BenchmarkUnmarshalitem-4 275056135 4.360 ns/op 229.35 MB/s 0 B/op 0 allocs/op | ||
| PASS | ||
| ok github.com/gofiber/fiber/v3/middleware/csrf 0.842s | ||
| PASS |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| goos: darwin | ||
| goarch: arm64 | ||
| pkg: github.com/example/mypackage | ||
| BenchmarkFib10 | ||
| BenchmarkFib10-8 5000000 325 ns/op | ||
| BenchmarkFib20 | ||
| BenchmarkFib20-8 30000 40537 ns/op | ||
| PASS | ||
| ok github.com/example/mypackage 3.614s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Could sections have the same pkg? I'm thinking if this is correct or if we need a Set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it's possible to have duplicate pkg sections but it's an easy improvement. Thanks!