Stack usage reduction in apartment switching, and lifetime fixes #1272
Conversation
The big deal here is that we fix `co_await apartment_context` so that it consumes no stack[1] if you are already in the destination context. This is a big win for loops that do apartment switching. To do this, `impl::resume_apartment` now returns `true` if it wants the caller to resume the coroutine synchronously, rather than doing it directly. If the caller is `await_suspend`, then it can propagate that result to the coroutine infrastructure and avoid stack build-up. Fixed a lifetime issue in `apartment_awaiter`, which copied the context too soon. It needs to copy it in the `await_suspend`, because the coroutine is to resume synchronously in the new apartment, which under the old code would destruct the context while `await_suspend` was still using it. This shifts a refcount update from one place to another, so no net change. We no longer need to extend the lifetime of the `IAsyncAction` now that we detect and defer synchronous resumption. This deletes a refcount update. [1] Older builds of MSVC have code generation issues for `bool await_suspend`, so we deftly avoid them here, in the same way we avoided them for `co_await IAsyncAction`. (Though the existing code in many awaiters such as `final_suspend_awaiter` still use `bool await_suspend`, so maybe we'll just hope everybody migrates to MSVC 16.11 or later?)
|
I thought I heard somewhere that apartment_context wasn't recommended anymore and people should prefer a CoreDispatcher or DispatcherQueue. Is this correct? |
kennykerr
left a comment
There was a problem hiding this comment.
Thanks, I had some concerns about stack unwinding but I think that's only a concern with dispatchers.
This is just a low-level COM-specific awaiter suitable for switching apartments. A given app model or API may require a specific dispatcher but |
It's certainly time to start ditching some of the older workarounds. |
The big deal here is that we fix
co_await apartment_contextso that it consumes no stack[1] if you are already in the destination context. This is a big win for loops that do apartment switching.To do this,
impl::resume_apartmentnow returnstrueif it wants the caller to resume the coroutine synchronously, rather than doing it directly. If the caller isawait_suspend, then it can propagate that result to the coroutine infrastructure and avoid stack build-up.Fixed a lifetime issue in
apartment_awaiter, which copied the context too soon. It needs to copy it in theawait_suspend, because the coroutine is to resume synchronously in the new apartment, which under the old code would destruct the context whileawait_suspendwas still using it. This shifts a refcount update from one place to another, so no net change.We no longer need to extend the lifetime of the
IAsyncActionnow that we detect and defer synchronous resumption. This deletes a refcount update.[1] Older builds of MSVC have code generation issues for
bool await_suspend, so we deftly avoid them here, in the same way we avoided them forco_await IAsyncAction. (Though the existing code in many awaiters such asfinal_suspend_awaiterstill usebool await_suspend, so maybe it's okay to assume MSVC 16.11 or later?)