Skip to content

v3(enhancement): remove utils.Trim* because stdlib has same performance in go1.19#2030

Merged
trim21 merged 17 commits intogofiber:v3-betafrom
trim21:v3-util-genetic-byteSeq
Aug 20, 2022
Merged

v3(enhancement): remove utils.Trim* because stdlib has same performance in go1.19#2030
trim21 merged 17 commits intogofiber:v3-betafrom
trim21:v3-util-genetic-byteSeq

Conversation

@trim21
Copy link
Contributor

@trim21 trim21 commented Aug 19, 2022 •
edited
Loading

This pr remove Trim, TrimRight and TrimLeft.

Stdlib functions have save performance now.

@trim21 trim21 marked this pull request as ready for review August 19, 2022 14:47
@trim21 trim21 mentioned this pull request Aug 19, 2022
Merged
@ReneWerner87
Copy link
Member

ReneWerner87 commented Aug 19, 2022

can you check my last comments from the old one ?
https://github.com/gofiber/fiber/blob/v3-beta/utils/bytes.go#L59

@trim21 trim21 changed the title v3(tests): use a table driven test for utils.EqualFold v3(enhancement): convert more utils to generic byteSeq function. Aug 19, 2022
@trim21 trim21 marked this pull request as draft August 19, 2022 15:02
@trim21 trim21 marked this pull request as ready for review August 19, 2022 15:12
efectn
efectn reviewed Aug 19, 2022
View reviewed changes
Copy link
Member

@efectn efectn left a comment

Choose a reason for hiding this comment

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

can you update benchmarks on README.md

@trim21
Copy link
Contributor Author

trim21 commented Aug 19, 2022

do you considering using https://github.com/stretchr/testify instead of current AssertEqual 🤔

@trim21
Copy link
Contributor Author

trim21 commented Aug 19, 2022

can you update benchmarks on README.md

done

@trim21
Copy link
Contributor Author

trim21 commented Aug 19, 2022 •
edited
Loading

I just keep the old tests and benchmark as bytes inputs

trim21
trim21 commented Aug 19, 2022
View reviewed changes
ctx.go
// indicating that the request was issued by a client library (such as jQuery).
func (c *DefaultCtx) XHR() bool {
return utils.EqualFoldBytes(utils.UnsafeBytes(c.Get(HeaderXRequestedWith)), []byte("xmlhttprequest"))
return utils.EqualFold(c.Get(HeaderXRequestedWith), "xmlhttprequest")
Copy link
Contributor Author

@trim21 trim21 Aug 19, 2022

Choose a reason for hiding this comment

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

why do we use EqualFoldBytes instead of EqualFold in the first place?

@trim21
Copy link
Contributor Author

trim21 commented Aug 20, 2022 •
edited
Loading

I saw the updated benchmark result, I think we can remove these Trim* functions now 🤔

go 1.19

Benchmark_TrimRightBytes/fiber-16               100000000               10.66 ns/op            8 B/op          1 allocs/op
Benchmark_TrimRightBytes/default-16             100000000               10.84 ns/op            8 B/op          1 allocs/op

Benchmark_TrimLeftBytes/fiber-16                100000000               10.72 ns/op            8 B/op          1 allocs/op
Benchmark_TrimLeftBytes/default-16              100000000               11.29 ns/op            8 B/op          1 allocs/op

Benchmark_TrimBytes/fiber-16                    74933964                15.43 ns/op           16 B/op          1 allocs/op
Benchmark_TrimBytes/default-16                  74931624                15.86 ns/op           16 B/op          1 allocs/op

Benchmark_TrimRight/fiber-16                    484963700                2.471 ns/op           0 B/op          0 allocs/op
Benchmark_TrimRight/default-16                  454740096                2.614 ns/op           0 B/op          0 allocs/op

Benchmark_TrimLeft/fiber-16                     488699834                2.451 ns/op           0 B/op          0 allocs/op
Benchmark_TrimLeft/default-16                   423590457                2.835 ns/op           0 B/op          0 allocs/op

Benchmark_Trim/fiber-16                         296457481                4.023 ns/op           0 B/op          0 allocs/op
Benchmark_Trim/default-16                       322653328                3.804 ns/op           0 B/op          0 allocs/op
Benchmark_Trim/default.trimspace-16             248519290                4.811 ns/op           0 B/op          0 allocs/op

1.18 , our version is a little bit faster.

goos: windows
goarch: amd64
pkg: github.com/gofiber/fiber/v3/utils
cpu: AMD Ryzen 7 5800X 8-Core Processor
Benchmark_TrimRightBytes/fiber-16               100000000               10.49 ns/op            8 B/op          1 allocs/op
Benchmark_TrimRightBytes/default-16             99912576                11.57 ns/op            8 B/op          1 allocs/op

Benchmark_TrimLeftBytes/fiber-16                100000000               10.42 ns/op            8 B/op          1 allocs/op
Benchmark_TrimLeftBytes/default-16              99905920                11.55 ns/op            8 B/op          1 allocs/op

Benchmark_TrimBytes/fiber-16                    70524762                16.44 ns/op           16 B/op          1 allocs/op
Benchmark_TrimBytes/default-16                  70530980                16.72 ns/op           16 B/op          1 allocs/op

Benchmark_TrimRight/fiber-16                    865098051                1.398 ns/op           0 B/op          0 allocs/op
Benchmark_TrimRight/default-16                  487361299                2.470 ns/op           0 B/op          0 allocs/op

Benchmark_TrimLeft/fiber-16                     760135455                1.569 ns/op           0 B/op          0 allocs/op
Benchmark_TrimLeft/default-16                   419346405                2.895 ns/op           0 B/op          0 allocs/op

Benchmark_Trim/fiber-16                         309708110                3.745 ns/op           0 B/op          0 allocs/op
Benchmark_Trim/default-16                       302284438                4.009 ns/op           0 B/op          0 allocs/op
Benchmark_Trim/default.trimspace-16             250067830                4.784 ns/op           0 B/op          0 allocs/op
PASS
ok      github.com/gofiber/fiber/v3/utils       17.530s

@trim21 trim21 requested a review from efectn August 20, 2022 03:32
@efectn
Copy link
Member

efectn commented Aug 20, 2022

I saw the updated benchmark result, I think we can remove these Trim* functions now 🤔

go 1.19

Benchmark_TrimRightBytes/fiber-16               100000000               10.66 ns/op            8 B/op          1 allocs/op
Benchmark_TrimRightBytes/default-16             100000000               10.84 ns/op            8 B/op          1 allocs/op

Benchmark_TrimLeftBytes/fiber-16                100000000               10.72 ns/op            8 B/op          1 allocs/op
Benchmark_TrimLeftBytes/default-16              100000000               11.29 ns/op            8 B/op          1 allocs/op

Benchmark_TrimBytes/fiber-16                    74933964                15.43 ns/op           16 B/op          1 allocs/op
Benchmark_TrimBytes/default-16                  74931624                15.86 ns/op           16 B/op          1 allocs/op

Benchmark_TrimRight/fiber-16                    484963700                2.471 ns/op           0 B/op          0 allocs/op
Benchmark_TrimRight/default-16                  454740096                2.614 ns/op           0 B/op          0 allocs/op

Benchmark_TrimLeft/fiber-16                     488699834                2.451 ns/op           0 B/op          0 allocs/op
Benchmark_TrimLeft/default-16                   423590457                2.835 ns/op           0 B/op          0 allocs/op

Benchmark_Trim/fiber-16                         296457481                4.023 ns/op           0 B/op          0 allocs/op
Benchmark_Trim/default-16                       322653328                3.804 ns/op           0 B/op          0 allocs/op
Benchmark_Trim/default.trimspace-16             248519290                4.811 ns/op           0 B/op          0 allocs/op

1.18 , our version is a little bit faster.

goos: windows
goarch: amd64
pkg: github.com/gofiber/fiber/v3/utils
cpu: AMD Ryzen 7 5800X 8-Core Processor
Benchmark_TrimRightBytes/fiber-16               100000000               10.49 ns/op            8 B/op          1 allocs/op
Benchmark_TrimRightBytes/default-16             99912576                11.57 ns/op            8 B/op          1 allocs/op

Benchmark_TrimLeftBytes/fiber-16                100000000               10.42 ns/op            8 B/op          1 allocs/op
Benchmark_TrimLeftBytes/default-16              99905920                11.55 ns/op            8 B/op          1 allocs/op

Benchmark_TrimBytes/fiber-16                    70524762                16.44 ns/op           16 B/op          1 allocs/op
Benchmark_TrimBytes/default-16                  70530980                16.72 ns/op           16 B/op          1 allocs/op

Benchmark_TrimRight/fiber-16                    865098051                1.398 ns/op           0 B/op          0 allocs/op
Benchmark_TrimRight/default-16                  487361299                2.470 ns/op           0 B/op          0 allocs/op

Benchmark_TrimLeft/fiber-16                     760135455                1.569 ns/op           0 B/op          0 allocs/op
Benchmark_TrimLeft/default-16                   419346405                2.895 ns/op           0 B/op          0 allocs/op

Benchmark_Trim/fiber-16                         309708110                3.745 ns/op           0 B/op          0 allocs/op
Benchmark_Trim/default-16                       302284438                4.009 ns/op           0 B/op          0 allocs/op
Benchmark_Trim/default.trimspace-16             250067830                4.784 ns/op           0 B/op          0 allocs/op
PASS
ok      github.com/gofiber/fiber/v3/utils       17.530s

I agree

@trim21 trim21 changed the title v3(enhancement): convert more utils to generic byteSeq function. v3(enhancement): remove utils.Trim* because stdlib has save performance now Aug 20, 2022
@trim21 trim21 changed the title v3(enhancement): remove utils.Trim* because stdlib has save performance now v3(enhancement): remove utils.Trim* because stdlib has same performance now Aug 20, 2022
@trim21 trim21 changed the title v3(enhancement): remove utils.Trim* because stdlib has same performance now v3(enhancement): remove utils.Trim* because stdlib has same performance in go1.19 Aug 20, 2022
@trim21 trim21 merged commit b161f80 into gofiber:v3-beta Aug 20, 2022
@trim21 trim21 deleted the v3-util-genetic-byteSeq branch August 20, 2022 05:52
@efectn efectn added this to the v3 milestone Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@efectn efectn Awaiting requested review from efectn

Assignees

No one assigned

Labels

🧹 Updates v3

Projects

v3
Status: Done

Milestone

v3.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@trim21 @ReneWerner87 @efectn