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

Perf test between fasthttp and net/http #1610

Closed
3 tasks
youngbupark opened this issue May 28, 2020 · 13 comments
Closed
3 tasks

Perf test between fasthttp and net/http #1610

youngbupark opened this issue May 28, 2020 · 13 comments

Comments

@youngbupark
Copy link

In what area(s)?

/area runtime

Describe the proposal

This proposes the perf test for fasthttp and net/http. We have used fasthttp due to the performance benefit. However, most of 3rd party pkgs support net/http so that it is hard to use them without the proper conversion of request context.

For example, our rate limiter implementation uses tollboth rate limitting pkg which requires net/http. so we added the below conversion code to convert fasthttp request to net/http request. this is the unnecessary implementation, which degrades the performance.

https://github.com/dapr/components-contrib/blob/42a7af4c0c7227df19fd07b6a94d40ba7c252383/middleware/http/nethttpadaptor/nethttpadaptor.go#L16

Since we could not leverage the existing implementation, we need to implement our own implementation for fasthttp such as trace header propagation logic in opencensus.

Therefore, we will run the dapr perf test for net/http and fasthttp implementation. then we will decide to switch from fasthttp to net/http based on the data.

  • migrate fasthttp to net/http for testing
  • run dapr perf test
  • make a decision based on the perf result
@youngbupark
Copy link
Author

@yaron2
Copy link
Member

yaron2 commented May 28, 2020

Like we discussed, I support doing this perf test, but the reason given here is not a good argument.

For example, our rate limiter implementation uses tollboth rate limitting pkg which requires net/http. so we added the below conversion code to convert fasthttp request to net/http request. this is the unnecessary implementation, which degrades the performance.

We should not switch an HTTP server because an optional and non-core feature (rate limiting) of Dapr uses a third party dependency that uses a package from net/http.

Also, the performance hit from converting the requests is at this point theoretical. If its a zero-alloc operation (or near-zero), which I believe it should be, there won't be a perf hit at all.
And again, that perf hit is only if the user is using rate limiting with Dapr, in a scenario that is designed in itself to reduce throughput.

@amanbha
Copy link
Contributor

amanbha commented May 28, 2020

Yes, lets do the perf run, If the perf differences are not significant between FastHttp and net/Http then I would suggest to use net/Http as net/Http is part of golang, and will continue to be developed, improved, I am not sure if I can say that for FastHttp, this is specially important as our public apis use FastHttp for middleware.

I see following on fasthttp page:

Are there known net/http advantages comparing to fasthttp?
Yes:
net/http API is stable, while fasthttp API constantly evolves.
net/http handles more HTTP corner cases.
net/http should contain less bugs, since it is used and tested by much wider audience.

Given that Dapr is a runtime which will be used by different type of user applications and will underpin integrations with lots of other products, I would lean towards using net/http for stability reasons.

@yaron2
Copy link
Member

yaron2 commented May 28, 2020

Yes, lets do the perf run, If the perf differences are not significant between FastHttp and net/Http then I would suggest to use net/Http as net/Http is part of golang, and will continue to be developed, improved, I am not sure if I can say that for FastHttp, this is specially important as our public apis use FastHttp for middleware.

I see following on fasthttp page:

Are there known net/http advantages comparing to fasthttp?
Yes:
net/http API is stable, while fasthttp API constantly evolves.
net/http handles more HTTP corner cases.
net/http should contain less bugs, since it is used and tested by much wider audience.

Given that Dapr is a runtime which will be used by different type of user applications and will underpin integrations with lots of other products, I would lean towards using net/http for stability reasons.

I would agree, if we had a reason to think fasthttp wasn't stable. It's a top performer on TechEmpower, used in production and actively maintained.

It's mainly focused at backend servers and not front end servers, which is exactly our use case.
TechEmpower benchmarks show it's about x10 more peformant than net/http.. and performance is critical to Dapr.

@youngbupark
Copy link
Author

Let's assume both are stable. Let's run perf-test first and decide the direction based on Data :)

@youngbupark
Copy link
Author

youngbupark commented Jun 1, 2020

@yaron2 @amanbha

I got the interesting result from the POC which migrates from fasthttp to go net/http. The test has been done in local environment so we may need to run the test in k8s cluster again.

As we know, the default mux in go doesn't support regex to route the request, so I selected go-chi which uses only go std libs including net/http. chi has been proven in the multiple production environments and it is one of performant routing implementations.

image

Test condition

  1. Test Machine : 2019 macbook pro
  2. Dapr implementation
    Removed default middlewares/metrics/traces for http server/client in both:
  3. Test tools: Artillery
    load test config - send 10 concurrent requests user every 1 sec for 120 secs
config:
  target: "http://localhost:61118"
  phases:
    - duration: 120
      arrivalRate: 10
scenarios:
  - flow:
      - post:
          url: "/v1.0/invoke/nodeapp/method/neworder"
          json:
            data:
              orderId: "10"
  1. go version : 1.14.3

Test repro steps: using 1. hello world sample

  1. Run nodejs app in 1.hello world sample
  2. Run daprd without python app
  3. run load test
# for net/http implementation
artillery run load.yaml --output nethttp.json
# for fasthttp implementation
artillery run load.yaml --output fasthttp.json

Test result:

At p99 latency, the latency result of net/http+go-chi router is close to the latency of fasthttp+fasthttp/router. At max latency, net/http is ~30ms faster than fasthttp. I ran each test at least three times, but I was not able to see the significant errors among them.

Note: mDNS adds 1 second timeout so we should subtract it. (We need to file the bug for this)

net/http+go-chi router result

    "latency": {
      "min": 1004.3,
      "max": 1012.5,
      "median": 1006.8,
      "p95": 1008.2,
      "p99": 1009.3
    },

fasthttp result

    "latency": {
      "min": 1004.5,
      "max": 1043.8,
      "median": 1007.2,
      "p95": 1008.7,
      "p99": 1009.6
    },

Review comment

When I got the result at the first time, I thought my test was wrong. so I ran the same load test more than five times for fasthttp, but there were no significant variation.

As you see the test codes, I removed metrics/tracing codes for both fasthttp and net/http testing so that metrics/tracing did not impact the test result.

Even if I consider header conversion change code as an inconsistent test factor, the max latency in fasthttp result can not explain why the max latency of fasthttp is ~30ms slower than the latency of net/http.

This simple load test shows that fasthttp is not 10x faster than net/http for Dapr.

For the further investigation, I would like to run dapr perf test and get cpu/mem usage metrics in k8s cluster to get more accurate data.

Action items

  • Run dapr perf test for net/http on k8s cluster
  • Run dapr perf test for fasthttp on k8s cluster
  • Compare CPU/memory usages

@yaron2
Copy link
Member

yaron2 commented Jun 1, 2020

Interesting, but your test environment and conditions are unreliable and are not sufficient as proof.

I ran a similar test yesterday using our perf test framework on a k8s cluster and have seen a severe perf degredation with nethttp which was also using more memory.

@yaron2
Copy link
Member

yaron2 commented Jun 1, 2020

Also, looking at the test setup, there are many things hacky about it.

First, 10 req per second is not a good metric. Latency shouldn't be affected at all for either web server under these conditions.

Second, using a local machine for both client and server is not recommended.
Third, testing on self hosted mode with the 1 second delay bug makes the results even more skewed.

Forth, we should look at p50/75/90.

The tests need to run with our perf test suite in a k8s cluster under at least 1000 req per second, with both pods on different machines.

@youngbupark
Copy link
Author

youngbupark commented Jun 1, 2020

Interesting, but your test environment and conditions are unreliable and are not sufficient as proof.

I agree that local environment was unreliable but i mentioned multiple test results did not have significant variation. so i cannot ignore the result.

I ran a similar test yesterday using our perf test framework on a k8s cluster and have seen a severe perf degredation with nethttp which was also using more memory.

Can you please share your code and test number?
I will run dapr perf test with the code that i used.

@youngbupark
Copy link
Author

youngbupark commented Jun 1, 2020

Also, looking at the test setup, there are many things hacky about it.

First, 10 req per second is not a good metric. Latency shouldn't be affected at all for either web server under these conditions.

This is not the final test. I wanted to run some load test before running actual perf test. If this simple test showed big difference, i would not even try to run perf test in k8s. That’s why I add action items for further investigation.

Second, using a local machine for both client and server is not recommended.
Third, testing on self hosted mode with the 1 second delay bug makes the results even more skewed.

1second delay is added to both test results. As i mentioned, i will run dapr perf test for both codes.

Forth, we should look at p50/75/90.

Why should we look at only 75/90 ?

The tests need to run with our perf test suite in a k8s cluster under at least 1000 req per second, with both pods on different machines.

As i planned, i will run dapr perf test against two codes. I wanted to make sure that additional effort to run dapr perf test is meaningful.

Actual test result will come soon.

@youngbupark
Copy link
Author

@yaron2 you can find the test code i used above; one is for nethttp and another is for fasthttp. I disabled tracing and metrics For both. Please share your test code. I would like to see what I missed...

@youngbupark youngbupark moved this from To do to In progress in 0.9.0 Milestone Jun 2, 2020
@youngbupark
Copy link
Author

@yaron2 @amanbha I will create the ADR and close this issue.

@youngbupark youngbupark self-assigned this Jun 12, 2020
@yaron2
Copy link
Member

yaron2 commented Jun 12, 2020

@yaron2 @amanbha I will create the ADR and close this issue.

Thanks

0.9.0 Milestone automation moved this from In progress to Review Jun 16, 2020
@orizohar orizohar removed the Epic label Jun 18, 2020
@orizohar orizohar moved this from Review to Done in 0.9.0 Milestone Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

5 participants