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.

Sign up for GitHub

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

Jump to bottom

Improve ToString by 20% #62

Merged
merged 4 commits into from
Feb 9, 2024
Merged

Improve ToString by 20% #62

merged 4 commits into from
Feb 9, 2024

Conversation

Fenny
Copy link
Member

@Fenny Fenny commented Feb 8, 2024 •
edited
Loading

  • Avoid unnecessary pointer dereferencing in convert.go#ToString
  • Add all types in ToString tests and benchmarks
ok  	github.com/gofiber/utils/v2	2.314s	coverage: 75.4% of statements

**Before**
Benchmark_ToString-24            3499046               339.2 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            3552510               336.8 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            3589656               335.2 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            3564043               337.8 ns/op            96 B/op          6 allocs/op
**After**
Benchmark_ToString-24            4211290               285.7 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            4296464               282.5 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            4279908               283.5 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            4216419               283.5 ns/op            96 B/op          6 allocs/op

@Fenny
Improve ToString by 20%
4f8cdff 
Avoid unnecessary pointer dereferencing

#Before
```
Benchmark_ToString-24            3499046               339.2 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            3552510               336.8 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            3589656               335.2 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            3564043               337.8 ns/op            96 B/op          6 allocs/op
```

#After
```
Benchmark_ToString-24            4211290               285.7 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            4296464               282.5 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            4279908               283.5 ns/op            96 B/op          6 allocs/op
Benchmark_ToString-24            4216419               283.5 ns/op            96 B/op          6 allocs/op
```
efectn
efectn reviewed Feb 8, 2024
View reviewed changes
convert.go Show resolved Hide resolved
convert.go Outdated Show resolved Hide resolved
@gaby
Copy link
Member

gaby commented Feb 9, 2024

The previous benchmark had 10x more performance. https://github.com/gofiber/utils/actions/runs/7753577848/job/21145172120

Is this related to the change in the benchmark code?

@Fenny
Copy link
Member Author

Fenny commented Feb 9, 2024

The previous benchmark had 10x more performance. https://github.com/gofiber/utils/actions/runs/7753577848/job/21145172120

Is this related to the change in the benchmark code?

Yes, see #62 (comment)

efectn
efectn reviewed Feb 9, 2024
View reviewed changes
convert_test.go Show resolved Hide resolved
@ReneWerner87 ReneWerner87 merged commit 8e706d8 into gofiber:master Feb 9, 2024
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efectn efectn efectn left review comments

@ReneWerner87 ReneWerner87 Awaiting requested review from ReneWerner87

Assignees
No one assigned
Labels
🧹 Updates
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Fenny @gaby @ReneWerner87 @efectn