Benchmarks and Improvements for parseRequestURL function #711
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SVilgelm
force-pushed
the
bench-parseRequestURL
branch
from
f13ee9a to
28f64e2
Compare
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #711 +/- ##
==========================================
+ Coverage 96.57% 96.62% +0.05%
==========================================
Files 12 12
Lines 1607 1632 +25
==========================================
+ Hits 1552 1577 +25
Misses 35 35
Partials 20 20
☔ View full report in Codecov by Sentry. |
|
@jeevatkm I hope this improvement will help the users. The change is relatively huge, it would be great if you can run some additional tests, or I can add additional tests |
SVilgelm
force-pushed
the
bench-parseRequestURL
branch
3 times, most recently
from
2b10e62 to
6674c2f
Compare
SVilgelm
force-pushed
the
bench-parseRequestURL
branch
from
b20765d to
7cf1a8f
Compare
|
rebased |
This comment was marked as outdated.
This comment was marked as outdated.
SVilgelm
force-pushed
the
bench-parseRequestURL
branch
2 times, most recently
from
a57888a to
f92bf9f
Compare
|
sure, I'll check later today, biking 🚴 around |
Thanks, take your time 👍 safe biking! |
SVilgelm
force-pushed
the
bench-parseRequestURL
branch
from
f92bf9f to
73e6f07
Compare
This comment was marked as outdated.
This comment was marked as outdated.
```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_parseRequestURL_PathParams-16 524658 2260 ns/op 448 B/op 9 allocs/op PASS ok github.com/go-resty/resty/v2 2.327s ```
```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_parseRequestURL_QueryParams-16 865923 1371 ns/op 416 B/op 13 allocs/op PASS ok github.com/go-resty/resty/v2 2.491s ```
* Use the map to collect all replacements and use replace all path parameters using O(1) logic
* Add additional unit tests to cover empty `{}` and not closed `{bar` path parameters
```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_parseRequestURL_PathParams-16 785971 1410 ns/op 320 B/op 6 allocs/op
PASS
ok github.com/go-resty/resty/v2 1.445s
```
* improve the loging by adding the query parameters from the request first, then adding the parameters from the client and skip if already exists * additional unit tests for the query parameters ```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_parseRequestURL_QueryParams-16 1000000 1158 ns/op 352 B/op 9 allocs/op PASS ok github.com/go-resty/resty/v2 2.473s ```
reusing a buffer from the pool decreases the allocs and memory usage ```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_parseRequestURL_PathParams-16 753834 1367 ns/op 256 B/op 5 allocs/op Benchmark_parseRequestURL_QueryParams-16 1000000 1167 ns/op 352 B/op 9 allocs/op PASS ok github.com/go-resty/resty/v2 2.373s ```
SVilgelm
force-pushed
the
bench-parseRequestURL
branch
from
73e6f07 to
402986b
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The benchmarks for the applying PathParams and adding QueryParams.
Original results:
% 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_parseRequestURL_PathParams-16 524658 2260 ns/op 448 B/op 9 allocs/op Benchmark_parseRequestURL_QueryParams-16 865923 1371 ns/op 416 B/op 13 allocs/opAfter 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_parseRequestURL_PathParams-16 753834 1367 ns/op 256 B/op 5 allocs/op Benchmark_parseRequestURL_QueryParams-16 1000000 1167 ns/op 352 B/op 9 allocs/opThe applying of PathParams has been improved by using O(1) logic instead of O(N). (N the number of PathParams + RawPathParams in both client and request). Instead of calling
strings.Replacefor each path parameter, the current logic collects all needed parameters in a map then replaces all parameters in URL by searching for the curly brackets and replacing. It scans the whole URL just once, from first to last positions.The improvements of adding the QueryParams have been done by swapping the processing order. First - process the request's QueryParams, only then process the client's QueryParams and skip already existed instead of deleting.