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

Further performance improvements #474

Closed
Kaliumhexacyanoferrat opened this issue Apr 9, 2024 · 3 comments · May be fixed by #483
Closed

Further performance improvements #474

Kaliumhexacyanoferrat opened this issue Apr 9, 2024 · 3 comments · May be fixed by #483
Labels
enhancement New feature or request

Comments

@Kaliumhexacyanoferrat
Copy link
Owner

As an operator of a web application written with GenHTTP, I would like my application to provide high throughput, low latency and low memory allocations so I do not need to care about scaling that much.

Example

The current implementation of the ClientHandler wraps the socket obtained for the client into a NetworkStream that is then passed to a PipeReader for parsing purposes.

As all work is currently scheduled on the ThreadPool and the various streams inbetween the socket and the application framework cause a lot of allocations, the performance of the server cannot be improved without fundamentally changing this architecture.

Looking at Kestrel, we should:

  • Use IO threads to read and write from the socket
  • Use ThreadPool to actually handle requests in the application
  • Read the incoming data without streams into a pipe that can directly re-use the memory allocated by the socket
  • Allow to write outgoing data allocation free directly to the socket

See this reference for a potential starting point: https://github.com/aykutalparslan/high-perfomance-tcp-server/

Main question would be how to handle SSL connections without the functionality provided by SslStream.

Acceptance criteria

  • There is an implementation that uses IO threads to write and write to the underlying socket
  • There is an implementation that avoids the use of additional streams to avoid allocations
  • The implementation can be used with SSL while not implementing SSL handling itself
  • If the implementation has side effects when embedding GenHTTP into an UI based application, the new implementation must be activated via opt-in
  • The application tests still pass
  • It has been shown that the application no longer suffers from thread starvation (CPU in waiting state) within benchmarks (e.g. via dotTrace)
  • The application has been tested for allocations in a text-based hello world scenario and it can be shown that there are no massive allocations per request (e.g. via dotMemory)
@Kaliumhexacyanoferrat Kaliumhexacyanoferrat added the enhancement New feature or request label Apr 9, 2024
@Kaliumhexacyanoferrat
Copy link
Owner Author

In Kestrel we have the Transport first (unencrypted) and then SSL on top as a Middleware via SslDuplexPipe. They create an SslStream from this pipe so we can re-use our logic with authentication I guess. Have to dig into this but this seems promising.

@Kaliumhexacyanoferrat Kaliumhexacyanoferrat linked a pull request Apr 24, 2024 that will close this issue
@Kaliumhexacyanoferrat
Copy link
Owner Author

First benchmarks show that this is even a little bit slower but yet comparable to the traditional, way easier approach - so it seems that the implementation of the .NET framework already runs I/O asynchronously or we are missing something else that limits the performance.

https://www.techempower.com/benchmarks/#section=test&shareid=f43754e4-f4a9-4219-9b83-b9757cfb7487&hw=ph&test=plaintext

@Kaliumhexacyanoferrat
Copy link
Owner Author

Will close this for now, #488 will probably bring the JSON test on par with ASP.NET.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant