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

perf(gen): use ht.ParseForm instead of ParseForm method #626

Merged
merged 5 commits into from
Oct 17, 2022
Merged

perf(gen): use ht.ParseForm instead of ParseForm method #626

merged 5 commits into from
Oct 17, 2022

Conversation

tdakkota
Copy link
Member

@tdakkota tdakkota commented Oct 15, 2022 •
edited
Loading 

name                time/op
ParseForm/Custom-4     289µs ± 3%
ParseForm/Std-4        562µs ± 3%

name                speed
ParseForm/Custom-4  42.5MB/s ± 3%
ParseForm/Std-4     21.9MB/s ± 3%

name                alloc/op
ParseForm/Custom-4     231kB ± 0%
ParseForm/Std-4        496kB ± 0%

name                allocs/op
ParseForm/Custom-4     1.06k ± 0%
ParseForm/Std-4        2.12k ± 0%

@codecov
Copy link

codecov bot commented Oct 15, 2022 •
edited
Loading

Codecov Report

Base: 73.71% // Head: 73.73% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (cdc0974) compared to base (0a8beab).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #626      +/-   ##
==========================================
+ Coverage   73.71%   73.73%   +0.02%     
==========================================
  Files         166      167       +1     
  Lines       11459    11468       +9     
==========================================
+ Hits         8447     8456       +9     
  Misses       2578     2578              
  Partials      434      434
Impacted Files Coverage Δ
http/http.go 58.82% <ø> (ø)
http/multipart.go 84.00% <ø> (ø)
http/form.go 100.00% <100.00%> (ø)
http/multipart_file.go 85.71% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tdakkota tdakkota force-pushed the perf/do-not-use-parseform branch 3 times, most recently from d90dc45 to 617895f Compare October 15, 2022 17:23
@tdakkota
fix(http): clone MultipartFile headers correctly
ee137a9
@tdakkota
feat(ht): provide an optimized version of ParseForm
93cf2f9 
Default `ParseForm` implementation allocates an unnecessary copy
of the request body, which could be huge.
Also, it copies parsed values twice, to `PostForm` field
and `Form` fields.
@tdakkota
test(http): add ParseForm benchmark
914163c
@tdakkota
perf(gen): use ht.ParseForm instead of ParseForm method
d04031a 
Benchmark:
```
name                time/op
ParseForm/Custom-4     289µs ± 3%
ParseForm/Std-4        562µs ± 3%

name                speed
ParseForm/Custom-4  42.5MB/s ± 3%
ParseForm/Std-4     21.9MB/s ± 3%

name                alloc/op
ParseForm/Custom-4     231kB ± 0%
ParseForm/Std-4        496kB ± 0%

name                allocs/op
ParseForm/Custom-4     1.06k ± 0%
ParseForm/Std-4        2.12k ± 0%
```
@tdakkota
chore: commit generated files
cdc0974
@tdakkota tdakkota force-pushed the perf/do-not-use-parseform branch from 617895f to cdc0974 Compare October 16, 2022 02:12
@tdakkota tdakkota marked this pull request as ready for review October 16, 2022 21:33
@ernado ernado merged commit 2df2f53 into ogen-go:main Oct 17, 2022
@tdakkota tdakkota deleted the perf/do-not-use-parseform branch October 17, 2022 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ernado ernado ernado approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@tdakkota @ernado