tcp: Explain why tcp::poll_tcb() does not use coroutines #2544
Conversation
| @@ -815,6 +815,8 @@ tcp<InetTraits>::tcp(inet_type& inet) | |||
|
|
|||
| template <typename InetTraits> | |||
| future<> tcp<InetTraits>::poll_tcb(ipaddr to, lw_shared_ptr<tcb> tcb) { | |||
| // Note that get_l2_dst_address() returns a ready future most of the time, so | |||
| // chaining a callback is mostly free. | |||
| return _inet.get_l2_dst_address(to).then([this, tcb = std::move(tcb)] (ethernet_address dst) { | |||
There was a problem hiding this comment.
There are hundreds of call sites where this holds. I don't think they all merit such annotation.
There was a problem hiding this comment.
We've seen in the past (I guess more in the ScyllaDB project than in Seastar itself) people randomly "cleaning up" working continuation-based code to use coroutines, so maybe if some place thoughtfully used continuations instead of couroutines because of a performance benefit and this code is performance-critical (so performance is more important than elegance), then maybe a comment would be helpful. I don't know if this specific function is an example of this.
Obviously, if someone was working on the Seastar tutorial, it is important to discuss there the cases where continuations perform better than coroutines - and the opposite cases where coroutines perform better than continuations.
This will prevent PRs like #2543 in the future.
cc @avikivity