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

Optimize charsetOrUtf8 #946

Merged
merged 1 commit into from
Jun 14, 2018
Merged

Optimize charsetOrUtf8 #946

merged 1 commit into from
Jun 14, 2018

Conversation

vkostyukov
Copy link
Collaborator

@vkostyukov vkostyukov commented Jun 14, 2018 •
edited

This little method is being called on each body decode and it's now 4x faster.

Benchmark                                                        Mode  Cnt    Score     Error   Units
HttpMessageBenchmark.fastChartset                                avgt   10   79.129 ±   5.363   ns/op
HttpMessageBenchmark.fastChartset:·gc.alloc.rate.norm            avgt   10   72.000 ±   0.001    B/op
HttpMessageBenchmark.slowCharset                                 avgt   10  290.393 ± 125.117   ns/op
HttpMessageBenchmark.slowCharset:·gc.alloc.rate.norm             avgt   10  372.000 ±  19.124    B/op

@codecov-io
Copy link

codecov-io commented Jun 14, 2018 •
edited

Codecov Report

Merging #946 into master will increase coverage by 0.07%.
The diff coverage is 92.3%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #946      +/-   ##
=========================================
+ Coverage   82.23%   82.3%   +0.07%     
=========================================
  Files          50      50              
  Lines         805     814       +9     
  Branches       44      51       +7     
=========================================
+ Hits          662     670       +8     
- Misses        143     144       +1
Impacted Files Coverage Δ
core/src/main/scala/io/finch/endpoint/body.scala 66.66% <100%> (ø) ⬆️
...ore/src/main/scala/io/finch/internal/package.scala 97.14% <91.66%> (-2.86%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc7dfca...7fc5c15. Read the comment docs.

@vkostyukov vkostyukov merged commit 3697d89 into master Jun 14, 2018
@vkostyukov vkostyukov deleted the vk/minor-optimizations branch June 14, 2018 21:06
finaglehelper pushed a commit to twitter/finagle that referenced this pull request Mar 26, 2019
@kevinoliver
finagle-base-http: Optimize Message.charset
e6e455c 
Problem

`c.t.f.http.Message.charset` can be implemented more efficiently.

Solution

Optimize it to avoid most allocations and thus run faster.
Hat-tip to @vkostyukov who implemented this in finch [1].

Result

In the case where "charset=" is not included in the content type
header, it runs 69% faster and allocations drop from 104 bytes/op
to 0.

In the case where "charset=" is included in the content type
header, it runs 81% faster and allocations drop 83%, from
432 bytes/op to 72.

[1] finagle/finch#946

JIRA Issues: CSL-7619

Differential Revision: https://phabricator.twitter.biz/D291072
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@vkostyukov @codecov-io