embrio-rs icon indicating copy to clipboard operation
embrio-rs copied to clipboard

async generator changes

Open arcnmx opened this issue 4 years ago • 2 comments

So this is all very WIP and I'm mostly posting for feedback at the moment. The commits/messages iterate through the changes/improvements, but basically the motivations here are:

  • Looking into the overhead of generators/futures/async fns led me to try and improve the codegen/types slightly. These are small things but saving 100 bytes or so for every future constructed tends to be significant when working with flash-constrained devices, it helps the optimizer, etc.
  • The current Invalid state indicates a poisoned future, which is overhead considering it's an extra enum variant that can introduce checks/assertions/panics in poll(). However, this state can only be observed if the call into the c generator constructor fails. Given that make_future() is unsafe, it can simply be made part of the API contract that the closure passed to it must not panic (ideally it does zero significant work and simply returns the generator after moving the pinned context reference into it). This is also an easy(?) assertion to make simply because the macro already knows and controls what it calls make_future() with.
  • We can make static guarantees that a future is no longer in the NotStarted state once a future has been pinned/initialized by using a newtype reference. Though this requires the call site to opt-in via custom traits (hm, is it possible to use IntoFuture or similar instead?), it means properly pinned futures do not need to check the enum discriminator for every (or any) single poll() call.

And a few notes on the wip-ness:

  • I need to shuffle around / reevaluate the safety assertions. I think my conjectures are true, but I'm mostly just experimenting right now.
  • The explicit pin interface is awkward. I made it take self by move because that allows it to be safe/typechecked but that means you need separate storage variable for the pinned data to then be placed in. I'd like to try cleaning this up with a newtype/typestate style wrapper that reuses the existing enum but just narrows the variant with the newtype reference.
    • Also the way in which I use ergo_pin is rather awkward, I'd like to look into moving the transform elsewhere or bringing it into the macro. Just throwing it into the await expansion is an option but also less flexible compared to how ergo_pin lets you bind it properly via let.
  • The naming/organization of the newtypes here could use some cleaning up.

arcnmx avatar Sep 15 '19 14:09 arcnmx