Implement a new algorithm to make this crate no_std
#30
Conversation
implement no_std event listeners
|
Now that I can test the benchmarks, it seems that it's regressed to an almost unacceptable degree. I'm not entirely sure where the extra overhead is coming from, I'll start looking into it. Also, I can't reproduce that MIRI failure on my machine. I guess it's time to try out Loom. |
|
Seems Miri failure is due to detached threads in doctest: rust-lang/miri#1371 A workaround that I sometimes use is adding sleep to the end of test: crossbeam-rs/crossbeam#891 |
|
With the newest commit, I have it down to only 30% slower. |
|
|
||
| use loom::sync::{Arc, Condvar, Mutex}; | ||
|
|
||
| /// Re-implementation of `parking::pair` based on loom. |
There was a problem hiding this comment.
Can we upstream this into https://github.com/smol-rs/parking ?
There was a problem hiding this comment.
Sure! It should be a straightforward reimplementation in terms of Loom primitives.
There was a problem hiding this comment.
Alright, it's smol-rs/parking#8
There was a problem hiding this comment.
now that it's upstreamed, can we use that version? (although I don't know how it would interact with dependencies...) Does cargo correctly work with git deps which also have versions (e.g. transforms them into crates.io entries on publishing)?
|
Closing in favor of #33 |
This PR resolves #28 by replacing this crate's algorithm with one that is compatible with
no_std. I realized that the current algorithm is basically a concurrent queue, so I replaced it with ourconcurrent_queuecrate. The underlying implementation is basically the same, but there is a newstdfeature that preventsparkingprimitives from being used. This is a breaking change, since removing thestdfeature removes thewait_*family of functions.For now, this is a draft PR because:
concurrent_queueto be released; right now I'm just usingmasterthe branch where I'm implementing Loom support.The new implementation pasts the existing tests, but I'd like to check to see if it also passes the tests forTest suites pass when I patch in this new version ofasync-channelandasync-lockwhen it is patched in.event-listener.There's a merge conflict I don't feel like resolving right now.Should be fixed by now.I'd like to benchmark the changes before I merge them. The new algorithm may allocate more often than the older algorithm. Although this allocation will likely be amortized over the program's runtime, I'd like to be sure that it doesn't impact performance significantly. See Add basic benchmarks #31.See my comment below.loomtesting, but I haven't gotten around to writing the tests for that yet.Future considerations:
ConcurrentQueue::bounded()is more efficient thanConcurrentQueue::unbounded(), it may be useful to provide a "with maximum length" method that allocates everything up front.