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

Fire fewer homing missiles #10070

Merged
merged 1 commit into from
Oct 26, 2013
Merged

Conversation

alexcrichton
Copy link
Member

This optimizes the home_for_io code path by requiring fewer scheduler
operations in some situtations.

When moving to your home scheduler, this no longer forces a context switch if
you're already on the home scheduler. Instead, the homing code now simply pins
you to your current scheduler (making it so you can't be stolen away). If you're
not on your home scheduler, then we context switch away, sending you to your
home scheduler.

When the I/O operation is done, then we also no longer forcibly trigger a
context switch. Instead, the action is cased on whether the task is homed or
not. If a task does not have a home, then the task is re-flagged as not having a
home and no context switch is performed. If a task is homed to the current
scheduler, then we don't do anything, and if the task is homed to a foreign
scheduler, then it's sent along its merry way.

I verified that there are about a third as many write syscalls done in print
operations now. Libuv uses write to implement async handles, and the homing
before and after each I/O operation was triggering a write on these async
handles. Additionally, using the terrible benchmark of printing 10k times in a
loop, this drives the runtime from 0.6s down to 0.3s (yay!).

@brson
Copy link
Contributor

brson commented Oct 25, 2013

The invariants here are really hard to follow. I wish that we weren't playing this dance of swapping out Task.home, and I don't understand why it's necessary. It seems like we should be able to just look at the IO handle's home and call handle.home.send(TaskFromFriend(task)), and then what is now restore_original_home is just checking whether the task is on the correct scheduler for resuming execution.

If we can get this to work though I don't mind leaving the complexity for now.

This optimizes the `home_for_io` code path by requiring fewer scheduler
operations in some situtations.

When moving to your home scheduler, this no longer forces a context switch if
you're already on the home scheduler. Instead, the homing code now simply pins
you to your current scheduler (making it so you can't be stolen away). If you're
not on your home scheduler, then we context switch away, sending you to your
home scheduler.

When the I/O operation is done, then we also no longer forcibly trigger a
context switch. Instead, the action is cased on whether the task is homed or
not. If a task does not have a home, then the task is re-flagged as not having a
home and no context switch is performed. If a task is homed to the current
scheduler, then we don't do anything, and if the task is homed to a foreign
scheduler, then it's sent along its merry way.

I verified that there are about a third as many `write` syscalls done in print
operations now. Libuv uses write to implement async handles, and the homing
before and after each I/O operation was triggering a write on these async
handles. Additionally, using the terrible benchmark of printing 10k times in a
loop, this drives the runtime from 0.6s down to 0.3s (yay!).
@alexcrichton
Copy link
Member Author

I've updated this with your suggestions and what we talked about on IRC, it's a lot simpler now, and it seems to achieve the same goal (all tests passed locally).

bors added a commit that referenced this pull request Oct 26, 2013
This optimizes the `home_for_io` code path by requiring fewer scheduler
operations in some situtations.

When moving to your home scheduler, this no longer forces a context switch if
you're already on the home scheduler. Instead, the homing code now simply pins
you to your current scheduler (making it so you can't be stolen away). If you're
not on your home scheduler, then we context switch away, sending you to your
home scheduler.

When the I/O operation is done, then we also no longer forcibly trigger a
context switch. Instead, the action is cased on whether the task is homed or
not. If a task does not have a home, then the task is re-flagged as not having a
home and no context switch is performed. If a task is homed to the current
scheduler, then we don't do anything, and if the task is homed to a foreign
scheduler, then it's sent along its merry way.

I verified that there are about a third as many `write` syscalls done in print
operations now. Libuv uses write to implement async handles, and the homing
before and after each I/O operation was triggering a write on these async
handles. Additionally, using the terrible benchmark of printing 10k times in a
loop, this drives the runtime from 0.6s down to 0.3s (yay!).
@bors bors closed this Oct 26, 2013
@bors bors merged commit e4c6523 into rust-lang:master Oct 26, 2013
@alexcrichton alexcrichton deleted the fewer-missiles branch October 26, 2013 08:14
@anasazi
Copy link
Contributor

anasazi commented Nov 8, 2013

I don't think this change maintains thread safety. As far as I can tell, this doesn't change the home of the task to be the I/O home, which means that you can get this sequence of events:

  1. go to I/O home (home_for_io)
  2. start executing I/O action
    a) unblock task and yield (puts task on work queue)
    b) task is stolen
    c) task wakes up on wrong thread and calls I/O function ===> bad things happen here
  3. if the task miraculously survives to restore_original_home, then we fail the task.

In the original version:

  1. go to I/O home (home_for_io) and swap out task's home with I/O home
  2. start executing I/O action
    a) unblock task and yield (puts task on work queue)
    b) task is stolen
    c) task starts to wake up, notices it's on the wrong thread, and sends itself home
    d) wakes up home and does I/O
  3. original home is swapped back

Temporarily setting the task's home to the I/O home is necessary for thread safety in I/O or else the scheduler will happily execute I/O on the wrong thread.

@alexcrichton
Copy link
Member Author

I can see how this would be a problem if the task is ever placed on a work queue, but I don't think that it's ever found in a work queue. During the homing operation, it's found in a specific scheduler's message queue. After the homing operation, it will get descheduled and won't be on any queue. When the task is reawoken, it's awoken with resume_blocked_task_immediately which never places it on a work queue.

I agree that if the task resumption didn't happen with the "immediately" function we would have problems, but so long as the uv bindings all resume immediately, I don't think that a task is on a work queue at any point in time during an I/O operation.

Although you are probably more familiar with the scheduler than I am, so am I missing something?

flip1995 pushed a commit to flip1995/rust that referenced this pull request Sep 5, 2024
…check, r=y21

Extend `implicit_saturating_sub` lint

Fixes rust-lang#10070.

It can serve as base if we want to add equivalent checks for other arithmetic operations.

Also one important note: when writing this lint, I realized that I could check for wrong conditions performed beforehand on subtraction and added another part in the lint. Considering they both rely on the same checks, I kept both in the same place. Not sure if it makes sense though...

changelog: Extend `implicit_saturating_sub` lint
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.

4 participants