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

Client Performance Not Great OOTB #2117

Closed
jamesward opened this issue Apr 24, 2023 · 25 comments
Closed

Client Performance Not Great OOTB #2117

jamesward opened this issue Apr 24, 2023 · 25 comments
Labels
💎 Bounty bug Something isn't working 💰 Rewarded

Comments

@jamesward
Copy link
Contributor

Describe the bug
Setting up a Client has high overhead ~2 seconds and without connection pooling, there is ~300ms of overhead on each request. It'd be great to have better out-of-the-box performance.

To Reproduce
Project to test performance: https://github.com/jamesward/zio-http-client-perf

On my connection / machine here are some results compared against the JDK's java.net.http.HttpClient hitting https://google.com

java.net.http.HttpClient

  • 1 req = 1375ms
  • 10 sequential = 708ms / req

Client.default

  • 1 req = 3038ms
  • 10 sequential = 896ms / req

FixedConnectionPool size 10

  • 1 req = 3105ms
  • 10 sequential = 560ms / req

Expected behaviour
Lower Client creation overhead and lower client reuse overhead (perhaps Client.default should use a connection pool)?

@jamesward jamesward added the bug Something isn't working label Apr 24, 2023
@jdegoes
Copy link
Member

jdegoes commented Apr 25, 2023

To be specific, performance goals of this ticket should be:

  • Almost no overhead with connection pooling (e.g. connection right away and spin up connection pool in the background)
  • Achieve performance better than java.net.http.HttpClient for both single request, and 10 sequential requests (currently the first is 2x slower, while the latter is 200ms slower)

Essentially, out of the box experience should be measurably better than what one could obtain using java.net.http.HttpClient, for both single-use scenarios, and batch / high-volume scenarios.

/bounty $600

@algora-pbc
Copy link

algora-pbc bot commented Apr 25, 2023

## 💎 $600 bounty • ZIO

### Steps to solve:
1. Start working: Comment /attempt #2117 with your implementation plan
2. Submit work: Create a pull request including /claim #2117 in the PR body to claim the bounty
3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

### Additional opportunities:
* 🔴 Livestream on Algora TV while solving this bounty & earn $200 upon merge! Make sure to have your camera and microphone on. Comment /livestream once live

Thank you for contributing to zio/zio-http!

Add a bountyShare on socials

Attempt Started (GMT+0) Solution
🔴 @lackhoa Sep 29, 2023, 6:01:03 AM WIP
🟡 @abebeos Oct 13, 2023, 7:20:08 AM WIP
🟡 @lazaridiscom Oct 13, 2023, 7:10:40 AM WIP
🟢 @kyri-petrou #2919

@jamesward
Copy link
Contributor Author

Yup! Thanks for the clarified goals.

@tachyonicbytes
Copy link

Hi! I would like to work on this!

I saw that comments on newer PRs say that I should type /attempt #2217 in order to start, but the older ones don't. Should I do it anyway?

@adamgfraser
Copy link
Contributor

@tachyonicbytes I think either way is fine! Most important thing is just to let other people know you are working on it which you already have! 😃

@tachyonicbytes
Copy link

Yeah, algora-pbc[bot] already gave me the rocket emoji, so I guess I did what it wanted

@dkovalenko
Copy link

We are comparing such a different implementations. Do we want to have something like .build() from java HttpClient on ZClient? In this case we could do some stuff like preparing thread pools and other heavy initialization eagerly.
Also I see that default Client.config pool of 10.

image

If we compare default vs .withDisabledConnectionPool I got that:

 1 http java = 383ms / req
 1 http zio = 2937ms / req
 1 http withDisabledConnectionPool = 2224ms / req

@lackhoa
Copy link
Contributor

lackhoa commented Sep 29, 2023

Is anyone still working on this? If not then I'll take it!
/attempt #2117

Options

@lackhoa
Copy link
Contributor

lackhoa commented Sep 29, 2023

@jdegoes Hi, I want to gain more context on this:

Essentially, out of the box experience should be measurably better than what one could obtain using java.net.http.HttpClient

Why do you think we should be measurably faster (And by how much in your estimate?). Where does the win come from :)?

@algora-pbc
Copy link

algora-pbc bot commented Oct 4, 2023

Note: The user @lackhoa is already attempting to complete issue #2117 and claim the bounty. If you attempt to complete the same issue, there is a chance that @lackhoa will complete the issue first, and be awarded the bounty. We recommend discussing with @lackhoa and potentially collaborating on the same solution versus creating an alternate solution.

@algora-pbc
Copy link

algora-pbc bot commented Oct 6, 2023

@lackhoa: Reminder that in 7 days the bounty will become up for grabs, so please submit a pull request before then 🙏

@jdegoes
Copy link
Member

jdegoes commented Oct 6, 2023

@abebeos What problems are you having with the bot?

@lackhoa
Copy link
Contributor

lackhoa commented Oct 6, 2023

Hi @jdegoes, I've reproduced the benchmark in the ticket. The main problem with the original benchmark is that it includes the client shutdown time, for example:

req(urlHttps).provide(Client.default).timed

Our client does take a bit of time to shutdown, but that shouldn't matter in practice because it should only be done at the end of the app anyway. Plus we've reduced that time in #2410, so it shouldn't take 2s anymore.

Here's my updated benchmark, which only takes into account client creation plus request time:

Sample output

[info] --- Iteration: 0 ---
[info] No connection pool: {single (1 requests): 844 ms/req}; batch (10 requests): 44 ms/req
[info] Java HttpClient: {single (1 requests): 253 ms/req}; batch (10 requests): 40 ms/req
[info] Default Client: {single (1 requests): 100 ms/req}; batch (10 requests): 23 ms/req

...

[info] --- Iteration: 108 ---
[info] No connection pool: {single (109 requests): 46 ms/req}; batch (1090 requests): 37 ms/req
[info] Java HttpClient: {single (109 requests): 49 ms/req}; batch (1090 requests): 37 ms/req
[info] Default Client: {single (109 requests): 39 ms/req}; batch (1090 requests): 21 ms/req

So our single request time is already better than the Java client. But as you can see, the very first request is somehow very slow. I have no idea why, because all subsequent requests are fast (even if you change the host, so it doesn't have anything to do with connection pooling or DNS).

I've also looked at ZKeyedPool and ZPool, and the connections are already done in the background already.

Anyway in conclusion, I think we still have a problem with start-up time, but IMO it's a much smaller problem (because it's only a one-time cost, not per request). So I'll stash this one and come back when there's more input.

@lackhoa
Copy link
Contributor

lackhoa commented Oct 7, 2023

@abebeos What misunderstanding did I add to the issue? Or do I have to pay thousands of dollars to find out the answer?

@lackhoa
Copy link
Contributor

lackhoa commented Oct 7, 2023

No it is clearly not the main issue. Why do you think that it is?

@lackhoa
Copy link
Contributor

lackhoa commented Oct 7, 2023

No it is clearly not the main issue. Why do you think that it is?

... (no further comment)

Ok, if you don’t want to talk, please do me a favor and never @ mention my name again. I don’t want to interact with you ever. You don’t say anything interesting anyway.

@jamesward
Copy link
Contributor Author

I've updated my repo to the latest deps and re-run the tests which produce these results:

1 https curl client = 636ms / req
10 https curl client = 537ms / req
1 https jdk client = 1006ms / req
10 https jdk client = 155ms / req
1 https netty epoll client = 359ms / req
10 https netty epoll client = 578ms / req
1 https netty nio client = 242ms / req
10 https netty nio client = 533ms / req
1 http zio = 2879ms / req
10 http zio = 457ms / req
1 https zio = 2738ms / req
10 https zio = 348ms / req
1 https zio fixed pool = 2742ms / req
10 https zio fixed pool = 372ms / req

@jamesward
Copy link
Contributor Author

jamesward commented Oct 8, 2023

Based on a suggestion from @abebeos I've bumped my test to a recent zio-http SNAPSHOT and for OOTB usage I'm seeing significantly better performance!

# Baseline
1 https jdk client = 451ms / req
10 https jdk client = 31ms / req

# 3.0.0-RC2
1 http zio = 2410ms / req
10 http zio = 266ms / req
1 https zio = 2352ms / req
10 https zio = 256ms / req
1 https zio fixed pool = 2186ms / req
10 https zio fixed pool = 255ms / req

# 3.0.0-RC2+73-6411c97a-SNAPSHOT
1 http zio = 666ms / req
10 http zio = 74ms / req
1 https zio = 237ms / req
10 https zio = 64ms / req
1 https zio fixed pool = 2160ms / req
10 https zio fixed pool = 350ms / req

Those initial numbers are looking much better for the OOTB usage. Not sure what changed / fixed this since RC2.

@adamgfraser
Copy link
Contributor

@abebeos The bounty is for actually making the performance faster than java.net.http.HttpClient in both single use and batch scenarios, as clearly stated above. You have not done that but have merely restated the issue. Creating benchmarks and identifying drivers is something I would expect to be a part of any PR that addresses this issue.

@ghost
Copy link

ghost commented Oct 10, 2023

The bounty is for actually making the performance faster than java.net.http.HttpClient in both single use and batch scenarios

Please be aware that you are delusional if you believe that this is feasible.

Even a $60_000 bounty could not achieve this result.

You have not done that but have merely restated the issue.

"merely restated"...

Wow!

Best case scenario: you (@adamgfraser & @jdegoes ) are just to busy to take time to understand a problem-domain.

I rest my case and leave it to the code-monkeys that consume anything served as-is!

Cu around.

@jamesward I may still keep a look on your benchmark, mostly out of curiosity (e.g. how a simple test-runner would be implemented in scala, jamesward/zio-http-client-perf#5 (comment))

@algora-pbc
Copy link

algora-pbc bot commented Oct 13, 2023

The bounty is up for grabs! Everyone is welcome to /attempt #2117 🙌

@algora-pbc
Copy link

algora-pbc bot commented Oct 20, 2023

@lazaridiscom: Reminder that in 7 days the bounty will become up for grabs, so please submit a pull request before then 🙏

@algora-pbc
Copy link

algora-pbc bot commented Oct 20, 2023

@abebeos: Reminder that in 7 days the bounty will become up for grabs, so please submit a pull request before then 🙏

Copy link

algora-pbc bot commented Jun 20, 2024

💡 @kyri-petrou submitted a pull request that claims the bounty. You can visit your bounty board to reward.

Copy link

algora-pbc bot commented Jun 20, 2024

🎉🎈 @kyri-petrou has been awarded $600! 🎈🎊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💎 Bounty bug Something isn't working 💰 Rewarded
Projects
None yet
Development

No branches or pull requests

7 participants