-
Notifications
You must be signed in to change notification settings - Fork 2k
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
hyper: simplify handlers #9728
base: master
Are you sure you want to change the base?
hyper: simplify handlers #9728
Conversation
… layers This commit makes it possible to turn off the removal of intermediate docker layers when building the tfb containers by adding a `--force-rm` flag to the tfb script. This is useful in situations where you want to inspect the intermediate layers for debugging purposes, or for caching builds of dependencies as a docker layer to speed up the build process. Note that the default behavior is to remove the intermediate layers to avoid filling up the disk with unused layers on the citrine server. Fixes: TechEmpower#9718
This change reduces the time it takes to build the hyper Docker image by caching the dependency builds. This is done by building a dummy binary before copying the source code into the image.
- move the server header to the router and add it to all responses. - move the response boxing to the router simplifying the return types of each handler. hyperium/http-body#150 will simplify the router code even futher to the following: ```rust let mut response = match request.uri().path() { "/ping" => ping()?.box_body(), "/json" => json::get()?.box_body(), "/db" => single_query::get().await?.box_body(), "/queries" => multiple_queries::get(request.uri().query()).await?.box_body(), "/fortunes" => fortunes::get().await?.box_body(), "/plaintext" => plaintext::get()?.box_body(), _ => not_found_error()?.box_body(), }; ```
The following frameworks were updated, pinging maintainers: |
Local benchmark results (Macbook Pro M2 Max) Before the recent hyper changes: hyper (JSON + Plaintext):JSON responses per second, (unspecified, hostname = 48c41e9f7187)
Plaintext responses per second, (unspecified, hostname = 48c41e9f7187)
Fortunes responses per second, (unspecified, hostname = e0169ecd4a21)
After this change: hyper (current-thread runtime):JSON responses per second, (unspecified, hostname = b7f8f718c8cc)
Database-access responses per second, single query, (unspecified, hostname = b7f8f718c8cc)
Responses per second, multiple queries, (unspecified, hostname = b7f8f718c8cc)
Fortunes responses per second, (unspecified, hostname = b7f8f718c8cc)
Plaintext responses per second, (unspecified, hostname = b7f8f718c8cc)
hyper (multi-thread runtime):JSON responses per second, (unspecified, hostname = cda50185af70)
Database-access responses per second, single query, (unspecified, hostname = cda50185af70)
Responses per second, multiple queries, (unspecified, hostname = cda50185af70)
Fortunes responses per second, (unspecified, hostname = cda50185af70)
Plaintext responses per second, (unspecified, hostname = cda50185af70)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good.
There's definitely a performance degradation in the results, and I haven't investigated why yet. The goal of the changes was to make the source a bit more maintainable, and then work on improving the perf aspects on top of that in ways where the code which impacts performance can be a bit more obvious and measured (with test variants). Because hyper is also the base for many of the rust frameworks, having numbers for each of the tests allows this to be used as the framework overhead in those tests (using the "versus" field in the configs", which would help show which parts of the perf are just pure hyper and which are from framework overhead (or in some cases improvements). |
each handler.
hyperium/http-body#150 will simplify the router
code even futher to the following:
Note: includes commits from #9727. Relevant commit that's only this change is a50128f