Golang Benchmarks don't take Package Name into consideration

[gaby]

When running benchmarks in a repo with a lot of packages, if two benchmarks have the same name this github-action doesn't take into account the package name cause the comparison to fail randomly.

This can be seen on this run: https://github.com/gofiber/fiber/actions/runs/10873073504

Where BenchmarkAppendMsgitem shows up multiple times with different values even though they are in different packages.

[ktrz]

Hey @gaby
Thanks for reporting this! I wonder how we could deal with this. I think it will create a similar issue of backward compatibility as we faced with PR #177

We could append the package name to the name of the benchmark so it would be eg

BenchmarkAppendMsgitem (github.com/gofiber/fiber/v3/middleware/cache)

This should create a unique name to compare
As a remedy to not break existing metrics we could treat benchmarks with only one package as special cases and not append the package name in those cases. Or add both cases: old (without suffix) and new (with suffix) so that it can be then dropped in the next release. WDYT?

[gaby]

@ktrz Adding both cases is probably the way to go now. Other option I can think is having a config option to specify if you want benchmarks with suffix or not.

[gaby]

@ktrz Any progress on this issue? Thanks!

[ReneWerner87]

@ktrz what is the status? the package name after the name of the benchmark would be good and would solve the problem

[ReneWerner87]

problem output

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: darwin
goarch: arm64
pkg: github.com/gofiber/fiber/v3/middleware/cache
BenchmarkAppendMsgitem
BenchmarkAppendMsgitem-12    	63634455	        19.01 ns/op	3103.57 MB/s	       0 B/op	       0 allocs/op
BenchmarkAppendMsgitem-12    	66411781	        18.42 ns/op	3202.97 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: darwin
goarch: arm64
pkg: github.com/gofiber/fiber/v3/middleware/csrf
BenchmarkAppendMsgitem
BenchmarkAppendMsgitem-12    	1000000000	         0.2926 ns/op	3417.23 MB/s	       0 B/op	       0 allocs/op
BenchmarkAppendMsgitem-12    	1000000000	         0.2883 ns/op	3468.54 MB/s	       0 B/op	       0 allocs/op
PASS
ok  	github.com/gofiber/fiber/v3/middleware/csrf	0.842s
PASS

current code

github-action-benchmark/src/extract.ts

Lines 342 to 392 in 6bae118

	function extractGoResult(output: string): BenchmarkResult[] {
	const lines = output.split(/\r?\n/g);
	const ret = [];
	// Example:
	// BenchmarkFib20-8 30000 41653 ns/op
	// BenchmarkDoWithConfigurer1-8 30000000 42.3 ns/op
	// Example if someone has used the ReportMetric function to add additional metrics to each benchmark:
	// BenchmarkThing-16 1 95258906556 ns/op 64.02 UnitsForMeasure2 31.13 UnitsForMeasure3
	// 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 =
	/^(?<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;
	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) {
	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 });
	}
	}
	}
	return ret;
	}

possible improvement

function extractGoResult(output: string): BenchmarkResult[] {
    const lines = output.split(/\r?\n/g);
    const results: BenchmarkResult[] = [];
    let currentPackage = '';

    // Regular expression to extract the package path
    const packageRegex = /^pkg:\s+(.+)$/;

    // Regular expression to extract benchmark information
    const benchmarkRegex =
        /^(?<name>Benchmark\w+[\w()$%^&*-=|,[\]{}"#]*?)(?<procs>-\d+)?\s+(?<times>\d+)\s+(?<remainder>.+)$/;

    for (const line of lines) {
        // Check if the line contains the package path
        const packageMatch = line.match(packageRegex);
        if (packageMatch) {
            currentPackage = packageMatch[1];
            continue;
        }

        // Check if the line contains benchmark information
        const benchmarkMatch = line.match(benchmarkRegex);
        if (benchmarkMatch?.groups) {
            const { name, procs, times, remainder } = benchmarkMatch.groups;
            const procsNumber = procs ? procs.slice(1) : null;
            const pieces = remainder.split(/\s+/);

            // For backward compatibility with Go benchmarks that had multiple metrics in the output,
            // but 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 += 2) {
                const value = parseFloat(pieces[i]);
                const unit = pieces[i + 1];
                const fullName = `${currentPackage}.${name}${i > 0 ? ' - ' + unit : ''}`;
                const extra = `${times} times${procsNumber ? `\n${procsNumber} procs` : ''}`.trim();
                results.push({ name: fullName, value, unit, extra });
            }
        }
    }

    return results;
}

I just don't know if the complete package changes the output too much
you could use only part of the package name or write it in a separate property

[ReneWerner87]

for now I have applied a hack by hand in our workflow until you fix the problem in the action and thus help all consumers with this problem

https://github.com/gofiber/fiber/blob/9a2ceb722027464842e248a38efce2c3fd67d1c6/.github/workflows/benchmark.yml#L38-L67