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

Introduce Socket and refactor connection caching #86

Merged
merged 32 commits into from
Jan 13, 2025

Conversation

thatstoasty
Copy link
Collaborator

@thatstoasty thatstoasty commented Jan 10, 2025

  • Introduce Socket stuct to encapsulate a File Descriptor which is operating as a socket.
  • Refactor NoTLSListener and SysConnection to use Socket, as well as updating downstream functions to account for the fact that Socket is only movable and not copyable.
  • Removed resolve_internet_address, we don't support UDP or Unix Addrs so it's not needed.
  • Remove HostPort, it was a vestige from tuples not working.
  • Added new unit and integration tests.
  • Moved bench and integration tests into respective folders.
  • Added a benchmark ci job, the results are much worse than our laptops, well... because it's limited in resources.
  • Added OwningList and PoolManager structs to rework connection caching in the client. Previously it made use of copying connections, which no longer works. This required creating new structs to manage objects which need to be transferred often.
  • Added an allow_redirects attribute to the client, it should optionally follow redirects.
  • Combined some common response functions by using default arg values.
  • Refactored ByteWriter and ByteReader logic so we don't accidentally loop or select indices that are out of bounds. Also letting the error propagate up if trying to select past the end of the reader instead of just returning 0 which may hide the issue.
  • Renamed skip_newlines to skip_carriage_return, to indicate that it does not skip newlines \n but it skips \r\n. It does not handle \n any differently than other bytes.
  • Changed some double underscore struct members to single underscore, since mojo does not have name mangling there is no need for double underscore, one is fine.

I'm figuring out why the fastapi server doesn't work in the ci job, works locally. Will fix soon.
Also, I'm interested in hearing any thoughts on how we can handle the logging. It would be nice to give the option to the user to choose the logger level at compile time? Perhaps the logger can be parametrized on log level, and we get the value from the mojo build/run args.

@thatstoasty thatstoasty marked this pull request as draft January 10, 2025 20:24
@thatstoasty thatstoasty marked this pull request as ready for review January 11, 2025 22:33
@thatstoasty
Copy link
Collaborator Author

🔥🐝 Lightbug is listening on http://0.0.0.0:8080
Ready to accept connections...
Running benchmark
Running 10s test @ http://0.0.0.0:8080/
1 threads and 1 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 94.72us 311.38us 6.88ms 97.96%
Req/Sec 16.61k 0.93k 17.85k 61.39%
166799 requests in 10.10s, 26.09MB read
Requests/sec: 16514.40
Transfer/sec: 2.58MB


Benchmark results

name , met (ms) , iters , min (ms) , mean (ms) , max (ms) , duration (ms)
HeaderEncode , 0.001473, 819252, 0.001473, 0.001473, 0.001473, 1207.032000
HeaderParse , 0.011066, 100000, 0.011066, 0.011066, 0.011066, 1106.639000
RequestEncode , 0.011640, 100000, 0.011640, 0.011640, 0.011640, 1164.050000
RequestParse , 0.029314, 39439, 0.029314, 0.029314, 0.029314, 1156.102000
ResponseEncode, 0.010841, 100000, 0.010841, 0.010841, 0.010841, 1084.138000
ResponseParse , 0.019715, 60125, 0.019715, 0.019715, 0.019715, 1185.344000

lightbug_http/client.mojo Show resolved Hide resolved
lightbug_http/http/__init__.mojo Show resolved Hide resolved
lightbug_http/libc.mojo Show resolved Hide resolved
@bgreni
Copy link
Collaborator

bgreni commented Jan 12, 2025

Wow a pretty incredible effort here, looks pretty good but I didn't have time to take a super close look.

Relating to your comment on logging, I think parameterizing the log level is a good idea that way we can avoid a lot of unnecessary branching.

.github/workflows/test.yml Outdated Show resolved Hide resolved
benchmark/bench.mojo Show resolved Hide resolved
lightbug_http/cookie/cookie.mojo Show resolved Hide resolved
lightbug_http/cookie/cookie.mojo Show resolved Hide resolved
lightbug_http/http/response.mojo Outdated Show resolved Hide resolved
tests/lightbug_http/test_host_port.mojo Outdated Show resolved Hide resolved
lightbug_http/server.mojo Show resolved Hide resolved
lightbug_http/socket.mojo Outdated Show resolved Hide resolved
lightbug_http/net.mojo Outdated Show resolved Hide resolved
@thatstoasty
Copy link
Collaborator Author

The logger struct has been parametrized using params passed in at compilation time, seems to be working well! The branching can be handled during the compilation step now

@thatstoasty thatstoasty changed the title WIP: Socket changes Introduce Socket and refactor connection caching Jan 12, 2025
@saviorand saviorand merged commit b41e88e into Lightbug-HQ:main Jan 13, 2025
3 checks passed
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.

3 participants