Replace futures-util with async-task #124
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #124 +/- ##
==========================================
+ Coverage 88.23% 88.87% +0.63%
==========================================
Files 15 15
Lines 1556 1672 +116
==========================================
+ Hits 1373 1486 +113
- Misses 183 186 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
Looks like your PR changed some behavior, as a test is no longer passing, and it does not look like a fluke. |
I keep forgetting that mpsc::Sender is Send but !Sync. Wrapping it in a mutex fixes that, at the cost of some overhead. Consider switching to crossbeam-queue or concurrent-queue in the future.
Yeah, I misread how the channels work and made it so it only drained 1 |
src/sources/futures.rs
Outdated
| // Make sure the runnable is never dropped. | ||
| std::mem::forget(e); |
There was a problem hiding this comment.
I added more documentation explaining this situation. Dropping the Runnable in this case is unsound, so the only option is to leak it.
Right now,
calloopusesfutures-util'sFuturesUnorderedtype to construct its executor. However,futures-utilhas quite a few dependencies. This PR migrates to another implementation based on theasync-taskcrate, which is dependency free and builds faster. The implementation also uses fewer synchronization primitives, so it should be faster in theory.I also add
slabas a dependency, but since #123 also adds it we should be alright here.WIP for now