Follow-up from "Draft: Completely overhaul the tor-proto circuit reactor"
There are a bunch of small refactors in !126 (merged) that we should probably do at some point, although they aren't completely critical. I'm testing out the gitlab "create issue to resolve threads" thing, which conveniently creates the following summary:
The following discussions from !126 (merged) should be addressed:
-
@nickm started a discussion: This isn't a new issue, but I do worry about this
send
blocking. Maybe we should make a note of that and try to figure out if it's a problem later. -
@nickm started a discussion: (+1 comment) Do we still need to store layer_fwd and layer_back separately, now that they go in the same object?
-
@nickm started a discussion: These two fields could re-merge, I think. Though that might be better done in a later ticket.
-
@nickm started a discussion: This is now a 200-line function. We should probably break it up in a later MR. (But let's leave it in place for now to keep review simple.)
-
@nickm started a discussion: I wonder if we should avoid these in the common case: this function is very much in our critical path, and it would be good to avoid heap allocations as much as we can. Maybe a
smallvec
would be a good choice. -
@nickm started a discussion: (+1 comment) We should avoid iterating over all the streams if we can; there can be thousands of them, in theory. This might be something to fix up in a later branch, however.
-
@nickm started a discussion: (+1 comment) Possibly creating the circuit could be done outside the run_once function? After all, until the circuit is created, no streams can be attached to it, and no messages other than CREATED{,2,V} or DESTROY are valid. No circuit ever gets created more than once, and every circuit gets created exactly once.