Remove will_wake calls - #1037
Conversation
We used `will_wake` in correctness checks to make sure we don't hang the future/stream forever if a wrong waker is used, but the contract for this function is best-effort and it started generating more and more false positives. The recent one, found by @aleiserson, is inside rust-lang (rust-lang/rust#119863), so it is very hard to get around. We don't have a good replacement for this check unless we implement our own waker, but it is probably too much for now
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1037 +/- ##
=======================================
Coverage 90.58% 90.59%
=======================================
Files 173 173
Lines 26089 26082 -7
=======================================
- Hits 23632 23628 -4
+ Misses 2457 2454 -3 ☔ View full report in Codecov by Sentry. |
martinthomson
left a comment
martinthomson
left a comment
There was a problem hiding this comment.
That's unfortunate. I guess the real expectation is that we maintain multiple wakers. That seems like a language feature that is really a bug: in that it imposes (unreasonable) costs on those using the API.
yea I am not sure I understand the motivation for making it best-effort, probably some hard constraint on the scheduling side. Or it was never designed to avoid false negatives, they were really only interested in true positives. |
|
related to #1036 |
andyleiserson
left a comment
andyleiserson
left a comment
There was a problem hiding this comment.
There is also a comment around line 58 of ordering_sender.rs "here used to be a check that new waker will wake the same task", that can probably be deleted. (This PR and the linked issue seem like sufficient documentation of the demise of will_wake checks.)
yea I was contemplating about it and was really on the fence about that one. Now the balance is changed so I'll remove it |
Use `clone_from` for micro optimization if we can
3 participants
We used
will_wakein correctness checks to make sure we don't hang the future/stream forever if a wrong waker is used, but the contract for this function is best-effort and it started generating more and more false positives.The recent one, found by @aleiserson, is inside rust-lang (rust-lang/rust#119863), so it is very hard to get around.
We don't have a good replacement for this check unless we implement our own waker, but it is probably too much for now