-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Adds http server API #1147
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
Adds http server API #1147
Conversation
7edc422
to
ce1ccac
Compare
I think |
6fff10c
to
bdcf9e6
Compare
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.
Some thoughts...
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.
LGTM... exciting!
Does not yet support streaming bodies.
Benchmarks on this hyper integration were not encouraging. Due to the large number of channels used to invert control from hyper, the system spends a lot of time in pthread_cond_wait. I am exploring a different strategy now for the HTTP server - do the parsing in JS and distribute it as an external module. WIP is at https://github.com/denoland/net/blob/master/http.ts |
I don't know what you're up to but an ideal solution is going to be just doing as much of it in native code[1] (Rust, C, whatever). I've said this about a million times by now. Otherwise you pretty much just end up with a copy of Node.js where things that should have been native code just end up as JS wrappers of JS wrappers of JS wrappers wrapping away the majority of all peformance. I don't know what pthread_cond_wait has to do with anything of this - it shouldn't be part of your code flow. I don't know why that's even called. It shouldn't. Feels like you want to agree with everyone yet you don't really listen and follow through with what was talked about. If you don't agree just say, "I don't agree I'm not going to listen to your input", instead of letting people think you listened. [1]: |
@ry could it be related to rust-lang/futures-rs#1170 (comment) ? Perhaps that suggestion helps your benchmark (reduces the time in @alexhultman It's great to see alternative options, the good thing is deno doesn't have to be wedded to an implementation yet. |
You're missing the point, it's not regarding <insert any implementation>, it's about how you bind that implementation to JavaScript efficiently. You can swap implementation all you want, it's not going to make any difference unless you nail down a good performing binding. |
I've said that about a billion times before: #205 (comment) People just think making something efficient is taking a brand name with a nice logo and like attaching that brand name to something else and boom now you have something efficient no matter what you did to that brand name. People essentially think deno is going to be Hyper for Rust because it wraps it. Guess what it's not even close. That Hyper perf. is completely lost due to inefficient V8 bindings. I said that 4 months ago and here we are today. I had proof of concept code back then, I have it today. People argue that Tokio can do 2 million req/sec yet deno caps out at 20k. ...and the conclusion is: "we can swap out Tokio for something we wrote in TypeScript". It's delusion. Look in that issue report from June - people are completely obsessed with "Rust = performance" and assume that just because you pick that brand name you're done. Guess what? If you're making a JavaScript runtime, then the JavaScript is always going to be your bottleneck. It doesn't matter one bit what kind of "high performance" Rust server you pick unless you cannot solve the binding efficiently. Deno currently does that worse than Node.js - things are moving in the wrong way. And now there's talk about reimplementing everything in TypeScript. Why would you rewrite Http servers in TypeScript when there already are finished Rust servers? If it sucks in performance now, it's going to suck even more when rewritten in TypeScript. |
Fixes #726