Support specifying stream priority via session::submit() #881
Conversation
src/asio_client_session.cc
Outdated
| } | ||
|
|
||
| nghttp2_priority_spec_init(&spec_, spec_.stream_id, spec_.weight, | ||
| spec_.exclusive); |
There was a problem hiding this comment.
I think it would be a bit nicer to do this in ctor, rather than here.
src/asio_client_session.cc
Outdated
| } | ||
|
|
||
| void session::read_timeout(const boost::posix_time::time_duration &t) { | ||
| impl_->read_timeout(t); | ||
| } | ||
|
|
||
| priority_spec::priority_spec(int32_t weight, int32_t stream_id, int exclusive) |
There was a problem hiding this comment.
I think it would be nice to arrange the order of the parameter in the same order as nghttp2_priroty_spec_init, that is, stream_id, weight, and then exclusive.
Since exclusive is effectively boolean, I prefer bool type for exclusive to generic int.
There was a problem hiding this comment.
yeah - i'd gone back and forth with consistency because it seemed like, for the simplest use, the weight would be the only specified parameter. but happy to keep it the same.
src/asio_client_session.cc
Outdated
| : valid_(true) { | ||
| spec_.weight = weight; | ||
| spec_.stream_id = stream_id; | ||
| spec_.exclusive = exclusive; |
There was a problem hiding this comment.
I think these 3 lines can be removed. nghttp2_priority_spec_init basically does that.
There was a problem hiding this comment.
whoops! meant to remove that. thanks for catching.
|
Thank you! Merged now. |
i noticed that the C++ API doesn't pass along priority (just passes
nullptrcurrently), so i thought a clean way to do this (without exposing the underlying nghttp2 types) would be to just create a simple wrapper that could be passed along, whose sentinel value would result in the samenullptrargument tonghttp2_submit_request(i.e. no specified priority), but when properly constructed, would pass annghttp2_priority_spec*.for convenience, it also seemed reasonable that
priority_specctor should be easily usable by providing the most common arguments first, i.e., trivially used to represent weight (e.g.priority_spec(4.0)).i need this on my end, so happy to make any adjustments you'd like!