Skip to content

Conversation

@Juneezee
Copy link
Contributor

We should use (*regexp.Regexp).MatchString instead of (*regexp.Regexp).Match([]byte(...)) when matching string to avoid unnecessary []byte conversions and reduce allocations. We have been using MatchString in other places, e.g. the cut transformer:

if regex.MatchString(pe.Key) {

Example benchmark:

var grepRegex = regexp.MustCompile("foo.*")

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

func BenchmarkMatchString(b *testing.B) {
	for i := 0; i < b.N; i++ {
		if match := grepRegex.MatchString("foo bar baz"); !match {
			b.Fail()
		}
	}
}
goos: linux
goarch: amd64
pkg: github.com/johnkerl/miller/pkg/transformers
cpu: AMD Ryzen 7 PRO 4750U with Radeon Graphics
BenchmarkMatch-16          	 5700908	       210.3 ns/op	      16 B/op	       1 allocs/op
BenchmarkMatchString-16    	 8006731	       156.4 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/johnkerl/miller/pkg/transformers	2.857s

We should use `(*regexp.Regexp).MatchString` instead of
`(*regexp.Regexp).Match([]byte(...))` when matching string to avoid
unnecessary `[]byte` conversions and reduce allocations.

Example benchmark:

var grepRegex = regexp.MustCompile("foo.*")

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

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

goos: linux
goarch: amd64
pkg: github.com/johnkerl/miller/pkg/transformers
cpu: AMD Ryzen 7 PRO 4750U with Radeon Graphics
BenchmarkMatch-16          	 5700908	       210.3 ns/op	      16 B/op	       1 allocs/op
BenchmarkMatchString-16    	 8006731	       156.4 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/johnkerl/miller/pkg/transformers	2.857s

Signed-off-by: Eng Zer Jun <[email protected]>
@johnkerl
Copy link
Owner

Thanks @Juneezee !!!! :D

@johnkerl johnkerl merged commit 4b34f80 into johnkerl:main Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants