Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a basic event loop without libuv #10054

Merged
merged 3 commits into from
Oct 25, 2013

Conversation

alexcrichton
Copy link
Member

This is more progress towards #9128 and all its related tree of issues. This implements a new BasicLoop on top of pthreads synchronization primitives (wrapped in LittleLock). This also removes the wonky callback_ms function from the interface of the event loop.

After #9901 is taking forever to land, I'm going to try to do all this runtime work in much smaller chunks than before. Right now this will not work unless #9901 lands first, but I'm close to landing it (hopefully), and I wanted to go ahead and get this reviewed before throwing it at bors later on down the road.

This "pausible idle callback" is also a bit of a weird idea, but it wasn't as difficult to implement as callback_ms so I'm more semi-ok with it.

@brson
Copy link
Contributor

brson commented Oct 24, 2013

Looks great. Can you replace more of the event loops in rt::sched tests with the BasicLoop since these tests don't depend on I/O?

My only reservation is that this exposes a condition variable via Exclusive, making it more than just a mutex. As long as we consider unstable::sync to be runtime implementation details then like, whatever. I can't really pinpoint what rubs me the wrong way about this though. Still, convert some of the tests and r=me.

The PausibleIdleCallback must have some handle into the event loop, and because
struct destructors are run in order of top-to-bottom in order of fields, this
meant that the event loop was getting destroyed before the idle callback was
getting destroyed.

I can't confirm that this fixes a problem in how we use libuv, but it does
semantically fix a problem for usage with other event loops.
This is a peculiar function to require event loops to implement, and it's only
used in one spot during tests right now. Instead, a possibly more robust apis
for timers should be used rather than requiring all event loops to implement a
curious-looking function.
@alexcrichton
Copy link
Member Author

I replaced run_in_newsched_task to use BasicLoop by default, and then I went through libstd and updated all necessary locations to use a new run_in_uv_task instead if they needed I/O services. The most unfortunate migrations were for task spawning, which requires making an empty hash set, using the task's RNG, using the OS's rng, requiring I/O services.

This would probably get alleviated if we start implementing "graceful fallback" onto native thread-blocking implementations, but that can probably happen later.

r? @brson on this strategy (just to make sure I'm sane)

It's not guaranteed that there will always be an event loop to run, and this
implementation will serve as an incredibly basic one which does not provide any
I/O, but allows the scheduler to still run.

cc rust-lang#9128
bors added a commit that referenced this pull request Oct 25, 2013
This is more progress towards #9128 and all its related tree of issues. This implements a new `BasicLoop` on top of pthreads synchronization primitives (wrapped in `LittleLock`). This also removes the wonky `callback_ms` function from the interface of the event loop.

After #9901 is taking forever to land, I'm going to try to do all this runtime work in much smaller chunks than before. Right now this will not work unless #9901 lands first, but I'm close to landing it (hopefully), and I wanted to go ahead and get this reviewed before throwing it at bors later on down the road.

This "pausible idle callback" is also a bit of a weird idea, but it wasn't as difficult to implement as callback_ms so I'm more semi-ok with it.
@bors bors closed this Oct 25, 2013
@bors bors merged commit 64a5c3b into rust-lang:master Oct 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants