Skip to content

Commit

Permalink
process: avoid redundant effort to reap orphan processes (#3743)
Browse files Browse the repository at this point in the history
  • Loading branch information
ipetkov authored May 14, 2021
1 parent 0b93bd5 commit e188e99
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 149 deletions.
94 changes: 10 additions & 84 deletions tokio/src/process/unix/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@
//! Process driver

use crate::park::Park;
use crate::process::unix::orphan::ReapOrphanQueue;
use crate::process::unix::GlobalOrphanQueue;
use crate::signal::unix::driver::Driver as SignalDriver;
use crate::signal::unix::{signal_with_handle, SignalKind};
use crate::sync::watch;
use crate::signal::unix::driver::{Driver as SignalDriver, Handle as SignalHandle};

use std::io;
use std::time::Duration;
Expand All @@ -16,51 +13,20 @@ use std::time::Duration;
#[derive(Debug)]
pub(crate) struct Driver {
park: SignalDriver,
inner: CoreDriver<watch::Receiver<()>, GlobalOrphanQueue>,
}

#[derive(Debug)]
struct CoreDriver<S, Q> {
sigchild: S,
orphan_queue: Q,
}

trait HasChanged {
fn has_changed(&mut self) -> bool;
}

impl<T> HasChanged for watch::Receiver<T> {
fn has_changed(&mut self) -> bool {
self.try_has_changed().and_then(Result::ok).is_some()
}
}

// ===== impl CoreDriver =====

impl<S, Q> CoreDriver<S, Q>
where
S: HasChanged,
Q: ReapOrphanQueue,
{
fn process(&mut self) {
if self.sigchild.has_changed() {
self.orphan_queue.reap_orphans();
}
}
signal_handle: SignalHandle,
}

// ===== impl Driver =====

impl Driver {
/// Creates a new signal `Driver` instance that delegates wakeups to `park`.
pub(crate) fn new(park: SignalDriver) -> io::Result<Self> {
let sigchild = signal_with_handle(SignalKind::child(), park.handle())?;
let inner = CoreDriver {
sigchild,
orphan_queue: GlobalOrphanQueue,
};
pub(crate) fn new(park: SignalDriver) -> Self {
let signal_handle = park.handle();

Ok(Self { park, inner })
Self {
park,
signal_handle,
}
}
}

Expand All @@ -76,57 +42,17 @@ impl Park for Driver {

fn park(&mut self) -> Result<(), Self::Error> {
self.park.park()?;
self.inner.process();
GlobalOrphanQueue::reap_orphans(&self.signal_handle);
Ok(())
}

fn park_timeout(&mut self, duration: Duration) -> Result<(), Self::Error> {
self.park.park_timeout(duration)?;
self.inner.process();
GlobalOrphanQueue::reap_orphans(&self.signal_handle);
Ok(())
}

fn shutdown(&mut self) {
self.park.shutdown()
}
}

#[cfg(test)]
mod test {
use super::*;
use crate::process::unix::orphan::test::MockQueue;

struct MockStream {
total_try_recv: usize,
values: Vec<Option<()>>,
}

impl MockStream {
fn new(values: Vec<Option<()>>) -> Self {
Self {
total_try_recv: 0,
values,
}
}
}

impl HasChanged for MockStream {
fn has_changed(&mut self) -> bool {
self.total_try_recv += 1;
self.values.remove(0).is_some()
}
}

#[test]
fn no_reap_if_no_signal() {
let mut driver = CoreDriver {
sigchild: MockStream::new(vec![None]),
orphan_queue: MockQueue::<()>::new(),
};

driver.process();

assert_eq!(1, driver.sigchild.total_try_recv);
assert_eq!(0, driver.orphan_queue.total_reaps.get());
}
}
9 changes: 5 additions & 4 deletions tokio/src/process/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@
pub(crate) mod driver;

pub(crate) mod orphan;
use orphan::{OrphanQueue, OrphanQueueImpl, ReapOrphanQueue, Wait};
use orphan::{OrphanQueue, OrphanQueueImpl, Wait};

mod reap;
use reap::Reaper;

use crate::io::PollEvented;
use crate::process::kill::Kill;
use crate::process::SpawnedChild;
use crate::signal::unix::driver::Handle as SignalHandle;
use crate::signal::unix::{signal, Signal, SignalKind};

use mio::event::Source;
Expand Down Expand Up @@ -73,9 +74,9 @@ impl fmt::Debug for GlobalOrphanQueue {
}
}

impl ReapOrphanQueue for GlobalOrphanQueue {
fn reap_orphans(&self) {
ORPHAN_QUEUE.reap_orphans()
impl GlobalOrphanQueue {
fn reap_orphans(handle: &SignalHandle) {
ORPHAN_QUEUE.reap_orphans(handle)
}
}

Expand Down
Loading

1 comment on commit e188e99

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'sync_mpsc'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: e188e99 Previous: 0b93bd5 Ratio
send_large 68866 ns/iter (± 123583) 30243 ns/iter (± 2973) 2.28

This comment was automatically generated by workflow using github-action-benchmark.

CC: @tokio-rs/maintainers

Please sign in to comment.