Round 8 #31

Closed
wants to merge 3 commits into
from

Projects

None yet

3 participants

@jasnell
Member
jasnell commented Jan 8, 2017

@mcollina @nodejs/http2 ... took a few steps back and refactored the internals significantly. Yields a fairly significant performance boost. Will be continuing to evolve this over the next couple of weeks

@indutny @jasnell indutny stream_base: homogenize req_wrap_obj use
Always use `req_wrap_obj` to allow calling `req->Done()` in stream
implementations.
81f5e98
@jasnell jasnell referenced this pull request Jan 8, 2017
Closed

Round7 #24

@mcollina
Member
mcollina commented Jan 9, 2017

I'm starting to think we should not inherit from Readable and Writable, but just from Stream (if at all) for the incoming request and response. Firstly,Readable  and Writable  do a lot of buffering, which we are already doing inside the Duplex stream. Secondly, there are plenty of calls to resume(), which is a code smell to me. This is "streams 1" API.

require('http') use Stream and streams 1 API.

@jasnell
Member
jasnell commented Jan 9, 2017

I'm all for whatever would make it easier and more performant. Most of the performance limitation in this right now is due to the Streams interface along with the need to make the high level API similar to the existing http/1 API. For instance, I'd really prefer getting rid of the separate Http2Request and Http2Response objects entirely and just use the Http2Stream object for both, but doing so would require a more significant high level API change.

@mcollina
Member

@jasnell would it be feasible to have a lower-level Http2Stream that can be sufficient to build apps only on HTTP2, and then provide a compat layer to match require('http')? If it's what you are talking about, go for it.

@jasnell
Member
jasnell commented Jan 10, 2017

It certainly would make things easier. The trick is designing that lower level interface in a way that would allow the higher level interface to still be performant. Also, even with the lower level interface, we need to decide how closely we want to bind it to the HTTP specific semantics vs. just exposing the Frame/Stream/Session model. The choice there is going to decide what kinds of performance optimizations we can make. For example, the decision about whether or not to attach a DataProvider to a response needs to be made when we initiate the response and can be based on a number of factors (what status code are we using? do we know if there is data pending to be written? is the Stream interface on the JavaScript side been ended yet?). If we always say there is a DataProvider, then we end up with a situation where there is always at least one potentially empty DATA frame sent on every stream, which is wasteful. The point is, the performance profile is going to depend entirely on how closely bound to the HTTP semantics we need this to be, and the more we push the HTTP semantics into the JS side, the fewer HTTP-specific performance optimizations we can make in the native code.

@mcollina
Member

@jasnell I think this is a great topic for #26 .

@mcollina mcollina referenced this pull request Jan 12, 2017
Open

HTTP2 meeting #26

@jasnell
Member
jasnell commented Jan 12, 2017

Landed in master

@jasnell jasnell closed this Jan 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment