AtomicWaker: take and wake

[bugadani]

trafficstars

While in embassy tasks are static, in other environments it's usually not the norm. There, keeping the Waker may force the task that registered it to remain allocated, potentially indefinitely. Given that AtomicWaker is used in peripheral drivers that aren't tied to a particular async framework, letting go of the Waker once it's not needed is probably better than keeping it around.

I expect this change to surface some issues around Futures that only register the waker once and expect it to stick around.

[bugadani]

Hmm does that trusted label mean I can ask for...?

bender run

[embassy-ci[bot]]

run: permission denied

[Dirbaio]

bender run

[Dirbaio]

I'm wondering, this alone doesn't fix the whole problem right? we also have to ensure all drivers deregister all their wakers on drop, otherwise they'll stay there forever keeping a reference to the task.

And the extra code to do that would be "waste" when running with embassy-executor.

[bugadani]

No, not the whole problem, but with the follow-up PR the bits are there for HAL authors to do it if they want to, without their own AtomicWaker.

[Dirbaio]

Is this something you're considering doing in esp-hal?

How're you planning of dealing with "the extra code to do that would be "waste" when running with embassy-executor"? just accept the bloat?

[bugadani]

How're you planning of dealing with

We don't have targets with 32k flash, that's how 😅 This isn't exactly about what we consider doing, it's more about not limiting the HAL to statically allocated tasks.

[Dominaezzz]

I'm wondering, this alone doesn't fix the whole problem right? we also have to ensure all drivers deregister all their wakers on drop, otherwise they'll stay there forever keeping a reference to the task.

Once this (and the follow up PR lands), I want to make PRs to embassy-sync to fix more of the problem. For example the Future returned by Signal::wait doesn't free the waker when it drops/cancels. Of course, unlike this PR, everyone would be paying for the extra code to do this. Would you be against this? (I don't want waste the elbow grease)

[bugadani]

So, this PR should either be neutral, or size/perf-positive because there's no need to write the Waker back into the Cell. However, the next PR will not be - take or deregister or whatever we end up with is a net loss for embassy. So what if we added a non-static-tasks feature to let other ecosystems work? The implication is that the next PR would have to be reverted from take to deregister so that the function could be no-op if the feature is not set.