Skip to content
New issue

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

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

Already on GitHub? # to your account

metrics: use regexp.MatchString for metric name check #4047

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

Juneezee
Copy link
Contributor

What?

(*Regexp).Match have a string-based equivalent (*Regexp).MatchString. We should use the string version to avoid unnecessary []byte conversions.

Why?

It's a one-line change that provides a free performance gain.

Benchmark:

func BenchmarkMatch(b *testing.B) {
	for i := 0; i < b.N; i++ {
		if match := compileNameRegex.Match([]byte("my_metric_name1")); !match {
			b.Fail()
		}
	}
}

func BenchmarkMatchString(b *testing.B) {
	for i := 0; i < b.N; i++ {
		if match := compileNameRegex.MatchString("my_metric_name1"); !match {
			b.Fail()
		}
	}
}

Result:

goos: linux
goarch: amd64
pkg: go.k6.io/k6/metrics
cpu: AMD Ryzen 7 PRO 4750U with Radeon Graphics
BenchmarkMatch-16          	 1457095	       820.7 ns/op	      16 B/op	       1 allocs/op
BenchmarkMatchString-16    	 1664458	       726.6 ns/op	       0 B/op	       0 allocs/op
PASS
coverage: 0.2% of statements
ok  	go.k6.io/k6/metrics	3.983s

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes.
  • I have run linter locally (make lint) and all checks pass.
  • I have run tests locally (make tests) and all tests pass.
  • I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

`(*Regexp).Match` have a string-based equivalent
`(*Regexp).MatchString`. We should use the string version to avoid
unnecessary `[]byte` conversions.

Benchmark:

func BenchmarkMatch(b *testing.B) {
	for i := 0; i < b.N; i++ {
		if match := compileNameRegex.Match([]byte("my_metric_name1")); !match {
			b.Fail()
		}
	}
}

func BenchmarkMatchString(b *testing.B) {
	for i := 0; i < b.N; i++ {
		if match := compileNameRegex.MatchString("my_metric_name1"); !match {
			b.Fail()
		}
	}
}

Result:

goos: linux
goarch: amd64
pkg: go.k6.io/k6/metrics
cpu: AMD Ryzen 7 PRO 4750U with Radeon Graphics
BenchmarkMatch-16          	 1457095	       820.7 ns/op	      16 B/op	       1 allocs/op
BenchmarkMatchString-16    	 1664458	       726.6 ns/op	       0 B/op	       0 allocs/op
PASS
coverage: 0.2% of statements
ok  	go.k6.io/k6/metrics	3.983s

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
@Juneezee Juneezee requested a review from a team as a code owner November 10, 2024 15:01
@Juneezee Juneezee requested review from inancgumus and ankur22 and removed request for a team November 10, 2024 15:01
Copy link
Contributor

@ankur22 ankur22 left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM 🚀

@inancgumus inancgumus merged commit 13561fa into grafana:master Nov 13, 2024
22 checks passed
@ankur22 ankur22 added this to the v0.56.0 milestone Jan 6, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants