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.

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

✨ v3 (enhancement): add []byte support to utils.EqualFold #2029

Merged
merged 2 commits into from
Aug 19, 2022

Conversation

trim21
Copy link
Contributor

@trim21 trim21 commented Aug 19, 2022

Please provide enough information so that others can review your pull request:

replace utils.EqualFold signature from func EqualFold(b, s string) bool to func EqualFold[S byteSeq](b, s S) bool

and add a private interface to utils.

type byteSeq interface {
	~string | ~[]byte
}

Explain the details for making this change. What existing problem does the pull request solve?

this will make utils.EqualFold support comparing bytes like bytes.EqualFold

And golang compiler will go monomorphization, which we will have a zero performance cost.

We can also make function like Trim* to be generic functions too but I'd like to just change this first for discussion.

Commit formatting

Use emojis on commit messages so it provides an easy way of identifying the purpose or intention of a commit. Check out the emoji cheatsheet here: https://gitmoji.carloscuesta.me/

before:

❯❯ ~\..\..\fiber git:(bind) go test -bench EqualFold -benchmem -run=node .\utils\
goos: windows
goarch: amd64
pkg: github.com/gofiber/fiber/v3/utils
cpu: AMD Ryzen 7 5800X 8-Core Processor
Benchmark_EqualFoldBytes/fiber-16               24507901                48.94 ns/op            0 B/op          0 allocs/op
Benchmark_EqualFoldBytes/default-16              8056410               149.1 ns/op             0 B/op          0 allocs/op
Benchmark_EqualFold/fiber-16                    29971826                39.97 ns/op            0 B/op          0 allocs/op
Benchmark_EqualFold/default-16                   9706066               124.3 ns/op             0 B/op          0 allocs/op
PASS
ok      github.com/gofiber/fiber/v3/utils       5.342s

after:

❯❯ ~\..\..\fiber git:(bind) go test -bench EqualFold -benchmem -run=node .\utils\
goos: windows
goarch: amd64
pkg: github.com/gofiber/fiber/v3/utils
cpu: AMD Ryzen 7 5800X 8-Core Processor
Benchmark_EqualFoldBytes/fiber-16               24196328                48.71 ns/op            0 B/op          0 allocs/op
Benchmark_EqualFoldBytes/default-16              8000298               149.1 ns/op             0 B/op          0 allocs/op
Benchmark_EqualFold/fiber-16                    29581348                40.24 ns/op            0 B/op          0 allocs/op
Benchmark_EqualFold/default-16                   9373461               124.6 ns/op             0 B/op          0 allocs/op
PASS
ok      github.com/gofiber/fiber/v3/utils       5.286s

@trim21 trim21 marked this pull request as ready for review August 19, 2022 14:25
@efectn
Copy link
Member

efectn commented Aug 19, 2022

I think it's good idea to use generics for Fiber utils.

@trim21
Copy link
Contributor Author

trim21 commented Aug 19, 2022

I don't know why, But I find that using []byte (BytesEqualFold(b, s []byte) bool) is even slower than using generic byteSeq...

goos: windows
goarch: amd64
pkg: github.com/gofiber/fiber/v3/utils
cpu: AMD Ryzen 7 5800X 8-Core Processor
Benchmark_BytesEqualFold
Benchmark_BytesEqualFold/fiber
Benchmark_BytesEqualFold/fiber-16               29250812                40.94 ns/op            0 B/op          0 allocs/op
Benchmark_BytesEqualFold/fiber-16               29241974                40.81 ns/op            0 B/op          0 allocs/op
Benchmark_BytesEqualFold/fiber-16               29241760                40.97 ns/op            0 B/op          0 allocs/op
Benchmark_BytesEqualFold/fiber-16               28878572                40.91 ns/op            0 B/op          0 allocs/op
Benchmark_BytesEqualFold/fiber-bytes-equal-fold
Benchmark_BytesEqualFold/fiber-bytes-equal-fold-16              24204332                49.42 ns/op            0 B/op          0 allocs/op
Benchmark_BytesEqualFold/fiber-bytes-equal-fold-16              23744884                49.71 ns/op            0 B/op          0 allocs/op
Benchmark_BytesEqualFold/fiber-bytes-equal-fold-16              24976584                49.38 ns/op            0 B/op          0 allocs/op
Benchmark_BytesEqualFold/fiber-bytes-equal-fold-16              23978371                49.14 ns/op            0 B/op          0 allocs/op
Benchmark_BytesEqualFold/default
Benchmark_BytesEqualFold/default-16                              9628075               123.2 ns/op             0 B/op          0 allocs/op
Benchmark_BytesEqualFold/default-16                              9762962               123.7 ns/op             0 B/op          0 allocs/op
Benchmark_BytesEqualFold/default-16                              9604748               123.5 ns/op             0 B/op          0 allocs/op
Benchmark_BytesEqualFold/default-16                              9665220               123.7 ns/op             0 B/op          0 allocs/op
PASS
ok      github.com/gofiber/fiber/v3/utils       15.386s

@efectn efectn added this to the v3 milestone Aug 19, 2022
@ReneWerner87 ReneWerner87 merged commit 73d0b71 into gofiber:v3-beta Aug 19, 2022
@trim21 trim21 deleted the v3-util-genetic-byteSeq branch August 19, 2022 14:40
@ReneWerner87
Copy link
Member

by the way i like such changes when the functions can do more and the performance stays the same

now can't we remove the old EqualFold function for bytes ?

we can also try to replace other old functions with many parameters with configuration items instead of 3-4 arguments so that they are extensible

@ReneWerner87
Copy link
Member

@trim21
Copy link
Contributor Author

trim21 commented Aug 19, 2022

Oh, I didn't find there is a EqualFoldBytes for bytes... I'll remove it in #2030

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants