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

Improve parseRequestHeader function and add the unit tests and benchmarks #712

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

SVilgelm
Copy link
Contributor

Original benchmarks:

% go test -benchmem -bench=. -run=^Benchmark
goos: darwin
goarch: amd64
pkg: github.com/go-resty/resty/v2
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
Benchmark_parseRequestHeader-16    1168611    1042 ns/op    496 B/op    8 allocs/op

After the improvements:

% go test -benchmem -bench=. -run=^Benchmark
goos: darwin
goarch: amd64
pkg: github.com/go-resty/resty/v2
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
Benchmark_parseRequestHeader-16    7956481    155.6 ns/op    0 B/op    0 allocs/op

Improved by modifying the original request headers instead of creating a new object

```shell
% go test -benchmem -bench=. -run=^Benchmark
goos: darwin
goarch: amd64
pkg: github.com/go-resty/resty/v2
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
Benchmark_parseRequestHeader-16          1168611              1042 ns/op             496 B/op          8 allocs/op
PASS
ok      github.com/go-resty/resty/v2    2.383s
```
@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (b852413) 96.34% compared to head (60e3194) 96.33%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #712      +/-   ##
==========================================
- Coverage   96.34%   96.33%   -0.02%     
==========================================
  Files          12       12              
  Lines        1615     1610       -5     
==========================================
- Hits         1556     1551       -5     
  Misses         37       37              
  Partials       22       22              
Files Coverage Δ
middleware.go 92.85% <100.00%> (-0.12%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

```shell
% go test -benchmem -bench=. -run=^Benchmark
goos: darwin
goarch: amd64
pkg: github.com/go-resty/resty/v2
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
Benchmark_parseRequestHeader-16          7956481               155.6 ns/op             0 B/op          0 allocs/op
PASS
ok      github.com/go-resty/resty/v2    1.599s
```
@jeevatkm
Copy link
Member

Thanks @SVilgelm, this PR is a comparatively small changeset than URL params PR, I will let you know!

@SVilgelm
Copy link
Contributor Author

Yep, that's why I created it as separated

Copy link
Member

@jeevatkm jeevatkm left a comment

Choose a reason for hiding this comment

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

@SVilgelm this PR looks good to me, thanks!

@jeevatkm jeevatkm merged commit 41199c3 into go-resty:master Sep 26, 2023
3 checks passed
@SVilgelm SVilgelm deleted the parseRequestHeader branch September 26, 2023 13:07
@jeevatkm jeevatkm added this to the v2.9.0 milestone Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants