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

Thoughts on a Web APIs (fetch) based http server (replacing node:http) #18

Open
cbnsndwch opened this issue Sep 16, 2024 · 11 comments
Open

Comments

@cbnsndwch
Copy link

cbnsndwch commented Sep 16, 2024

After this tweet from @mjackson:

Somebody please stop me from building an actual fetch-based HTTP server for Node.js

— MJ (@mjackson) September 9, 2024

I started thinking about what it would take to actually replace node:http with an http server that directly implements Web APIs as described in README.md:

What exactly we mean by "web standards"? It means we use:

The Web Streams API instead of Node.js streams
Uint8Array instead of Node.js Buffers
The Web Crypto API instead of the Node.js crypto library
Blob and File instead of some bespoke runtime-specific API

Took a deep dive into the source code of node:http and node:net to see what we would be working on and here are my thoughts:

No soup for you!

The files that make up node:http rely heavily on Node internal objects and functions that are not available to userland JS.

  • the global primordials object
  • the internalBinding('MODULE_ID)` function
  • Make compile time macros in C++
  • the http_parser module (internal binding)
  • functions, constants, and symbols from internal/http, internal/async_hooks, internal/errors, etc

Replicating the functionality of node:http while still relying on node:net is then gonna require adding a new built-in module to Node and getting it merged. It won't be possible directly from userland

Enter the multi-(language)-verse

One alternative I thought of is building a native addon that bundles a different http server and exposes a Web API-based interface to JS, while staying as close to node:http as possible:

There are viable options in Rust (https://hyper.rs/, https://actix.rs/) and in Go (https://gofiber.io/, https://github.com/go-chi/chi, https://gorilla.github.io/) that can be compiled as Node native modules.

Encore.ts seems to be doing something like this but they've gone the "batteries included" route where the value prop only makes sense if you switch your entire runtime and app architecture to their library. Ideally whatever I would build would only aim to provide an alternative to node:http and not become an entire stack.

This path would be a lot of work so before I keep going down the rabbit hole your feedback would be greatly appreciated 🤓

@cbnsndwch cbnsndwch changed the title Thoughts on a fetch-API based http server (replacing node:http) Thoughts on a Web API-s )fetch) based http server (replacing node:http) Sep 16, 2024
@cbnsndwch cbnsndwch changed the title Thoughts on a Web API-s )fetch) based http server (replacing node:http) Thoughts on a Web API-s (fetch) based http server (replacing node:http) Sep 16, 2024
@cbnsndwch cbnsndwch changed the title Thoughts on a Web API-s (fetch) based http server (replacing node:http) Thoughts on a Web APIs (fetch) based http server (replacing node:http) Sep 16, 2024
@cbnsndwch
Copy link
Author

PS: I know I just did the entire opposite of what the tweet said but I think there may be something worth exploring here. Sorry @mjackson 😁

@mjackson
Copy link
Owner

Sorry for the slowness on my reply here. I told you I didn't want to get sucked into this! 😅

Having said that, the path I was thinking about going was to use node:net together with milo for HTTP parsing. All server logic could be written in JavaScript/TypeScript.

I'd personally prefer to have the core server logic written in JavaScript/TypeScript (instead of e.g. Rust) to make it a little easier for JS devs to contribute.

I think there may be something worth exploring here

Yes, I definitely agree there is something worth exploring here. My current focus is making Remix and tools for Remix at the application layer that work with any JS runtime (Node.js, Deno, Bun, etc.), and we already have a thin wrapper around node:http that gives us good enough performance on Node.js, so I'm not very inclined to work on this myself. But if you've got the itch to contribute and you'd like to see something happen here I'd be happy to act as an advisor (and maybe even contribute some code as needed). But I'd need you to be the main driver.

@cbnsndwch
Copy link
Author

Makes sense! Will take a look and post back

@cbnsndwch
Copy link
Author

Oooh boy, it's been quite a journey so far haha!

Stopping over here to say I'm still looking into it every chance that I get. Keep the seat warm, if you will. Here's what I've learnt so far:

  • milo's setup for WASM is cool but has a heavy perf hit (in my tests, while following their docs) that kinda defeats the purpose of the while work it takes to make it run
  • the base rust library is still faaaaaast. It is not published as a standalone create for some reason (some other package is squatting on the crate name) and they're doing some dark magic with rust macros that makes it more complex to setup in local dev than i would've liked. I've however been able to mostly cleanup the stuff that's irrelevant and repackage it as an napi-rs package that would accomplish a similar level of portability as WASM. Working on a basic setup so I can run some initial tests.
  • not giving up on httparse from hyper.rs just yet
  • also looking at a couple pure js HTTP parser implementations just to get a benchmark of whether the N-API route is truly worth it

@mjackson
Copy link
Owner

mjackson commented Oct 9, 2024

Thanks for the report! I know very little about WASM, but it makes sense to me that there would be a perf hit there. AFAIK they are planning on using milo in node core sometime soon, but they're almost certainly not planning on using the WASM version.

Would it be worth it to do a spike with the WASM version just to get the ball rolling?

@cbnsndwch
Copy link
Author

Did some more digging into node:http to try and suss out what else besides the parser would be needed and found out that it actually allows overriding the IncomingRequest constructor the server uses 🤓

That gave and idea of simplifying the adapter pattern you've got going on from IcomingMessage to Request

Just by skipping a lot of checks undici makes when instantiating a Request object that don't make sense for this use case (e.g. because the body would always be a stream) I was able to get about 5% more req/s than the base implementation here

My hypothesis is that by giving node:http an optimized constructor that fulfills undici's interface I can beat the typical perf from Express by avoiding all the header looping and the double copying of body stream, without having to get down to the native level.

Seems promising, will test some more and come back with results

@mjackson
Copy link
Owner

mjackson commented Nov 9, 2024

it actually allows overriding the IncomingRequest constructor the server uses

Just to make sure I understand... are you saying I can tell node:http how to construct request objects so I don't need to use createRequest in node-fetch-server?

@cbnsndwch
Copy link
Author

cbnsndwch commented Nov 10, 2024

Yep! Check out the option object accepted by http.createServer. It takes optional IncomingMessage and ServerResponse constructors that can be used to replace the default implementations:

https://nodejs.org/docs/latest-v20.x/api/http.html#httpcreateserveroptions-requestlistener

It feels like a hack but by passing in a class that extends IncomingMessage and that also matches the Request interface it could be used in a fetch-compliant listener.

ServerResponse is a different beast though, because node:http will construct it upfront and expect headers and body to be written to it so listeners that return new Response(...) would still need the copy-out mechanism

@cbnsndwch
Copy link
Author

cbnsndwch commented Nov 22, 2024

Welp definitely confirmed that passing in a custom request constructor to createServer does work, and that it avoids copying over the request data

Perf is still not great which may be due to still needing to copy the response back to the ServerResponse from node.

Will try to add tracking to different parts of the listener to confirm

@cbnsndwch
Copy link
Author

Here's what it's looking like now on my laptop, in this run about 17.6% better perf than Express on WSL2. Will try to clean up the code a bit and push it to my fork for feedback:

Platform: Linux 5.15.167.4-microsoft-standard-WSL2
CPU: Intel(R) Core(TM) i9-14900HX
Date: Date: 11/24/2024, 1:34:43 PM
Saving stats to: ~/fetchification/packages/node-fetch-server-2/bench/runs

Running benchmark for node:[email protected] ...

Waiting 5 seconds for the server to start ...
Running 30s test @ http://127.0.0.1:3000/
  12 threads and 400 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    15.64ms   76.95ms   1.56s    98.50%
    Req/Sec     4.40k   668.00    18.81k    96.15%
  1558723 requests in 30.05s, 319.60MB read
Requests/sec:  51866.90
Transfer/sec:     10.63MB

Running benchmark for [email protected] ...

Waiting 5 seconds for the server to start ...
Running 30s test @ http://127.0.0.1:3000/
  12 threads and 400 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    26.50ms   99.51ms   1.99s    98.45%
    Req/Sec     2.08k   426.17     8.28k    94.34%
  721170 requests in 30.05s, 147.87MB read
  Socket errors: connect 0, read 0, write 0, timeout 104
Requests/sec:  23999.21
Transfer/sec:      4.92MB

Running benchmark for [email protected] ...

Waiting 5 seconds for the server to start ...
Running 30s test @ http://127.0.0.1:3000/
  12 threads and 400 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    28.76ms   94.99ms   2.00s    98.60%
    Req/Sec     1.72k   431.00     8.67k    94.83%
  593894 requests in 30.09s, 143.29MB read
  Socket errors: connect 0, read 0, write 0, timeout 121
Requests/sec:  19739.43
Transfer/sec:      4.76MB

@cbnsndwch
Copy link
Author

@mjackson #33

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

No branches or pull requests

2 participants