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

Allow creating a body from Array[Byte] #2581

Merged
merged 3 commits into from
Jan 4, 2024

Conversation

kyri-petrou
Copy link
Collaborator

I was doing some profiling on a simple "Hello world" app for large byte payloads (~65kb), and one of the things that stood out was that there was a very big memory allocation due to the copying of the Chunk[Byte] into an Array[Byte] (see screenshot at the bottom).

I think that this allocation can be avoided in 1 of 2 ways:

  1. Extract the underlying Array[Byte] from Chunk.Arr[Byte] in the UnsafeBytes#unsafeAsArray method
  2. Expose a new method allowing users to create a body directly via an array

In this PR, I went with (2) mostly because it eliminates the possibility of the array wrapped in a Chunk to be mutated. Having said that, (1) would not require changes in user's code to see the performance improvement so I'm happy to change this PR and go with option 1 instead.

I also added some extra benchmarks to showcase that besides the memory allocations, this change also improves the throughput of responses with large payloads. In the case of a 100kb payload, that's ~5% improvement in throughput. Note that the copying of the array happens prior to any compression taking place, so it's likely that many users will see some benefit from this change

[info] Benchmark                                           Mode  Cnt      Score    Error  Units
[info] ServerInboundHandlerBenchmark.benchmarkLargeArray  thrpt    5  12104.157 ± 78.598  ops/s
[info] ServerInboundHandlerBenchmark.benchmarkLargeChunk  thrpt    5  11519.093 ± 99.231  ops/s

Flamegraph:

image

@codecov-commenter
Copy link

codecov-commenter commented Jan 3, 2024

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (be67a7d) 64.39% compared to head (fc85318) 64.34%.

Files Patch % Lines
zio-http/src/main/scala/zio/http/Body.scala 44.44% 5 Missing ⚠️
...rc/main/scala/zio/http/netty/NettyBodyWriter.scala 0.00% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2581      +/-   ##
==========================================
- Coverage   64.39%   64.34%   -0.06%     
==========================================
  Files         140      140              
  Lines        8314     8326      +12     
  Branches     1504     1517      +13     
==========================================
+ Hits         5354     5357       +3     
- Misses       2960     2969       +9     

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

@jdegoes
Copy link
Member

jdegoes commented Jan 4, 2024

@kyri-petrou Good to merge when CI Is passing!

@987Nabil 987Nabil merged commit f2ce5cf into zio:main Jan 4, 2024
14 checks passed
@kyri-petrou kyri-petrou deleted the body-from-array branch January 4, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants