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

Clean up the IPC/MPSC split #848

Closed
nox opened this issue Feb 8, 2017 · 6 comments
Closed

Clean up the IPC/MPSC split #848

nox opened this issue Feb 8, 2017 · 6 comments

Comments

@nox
Copy link
Member

@nox nox commented Feb 8, 2017

Features are supposed to be additive, the two interfaces for IPC and MPSC channels are so different that code can fail to build if the other kind of channel was expected.

I think the MPSC system should always be built, and IPC be feature-gated. Why was the split needed anyway @glennw?

@nox
Copy link
Member Author

@nox nox commented Feb 8, 2017

IMO we should have a crate with a trait to describe a kind of channel, and with a sensible API that fits both MPSC and IPC.

Taking into account servo/ipc-channel#138, we could have something like that:

pub trait Channel<T> {
    type Sender: Sender<T>;
    type Receiver: Receiver<T>;

    fn channel() -> Self;
    fn sender(&self) -> &Self::Sender;
    fn receiver(&self) -> &Self::Receiver;
}

pub trait Sender<T> {
    type Error: Error;
    fn send(&self, t: &T) -> Result<(), Self::Error>;
}

trait Receiver<T> {
    type Error: Error;
    fn recv(&self) -> Result<T, Self::Error>;
}

pub struct Mpsc<T> {
    sender: mpsc::Sender<T>,
    receiver: mpsc::Receiver<T>,
}

impl<T: Clone> Channel<T> for Mpsc<T> {
    type Error = mpsc::SendError<T>;
    type Sender = MpscSender<T>;
    type Receiver = mpsc::Receiver<T>;

    fn channel() -> Self {
        let (sender, receiver) = mpsc::channel();
        Mpsc { sender: MpscSender::new(sender), receiver: receiver }
    }

    fn sender(&self) -> &Self::Sender {
        &self.sender
    }

    fn receiver(&self) -> &Self::Receiver {
        &self.receiver
    }
}

pub struct MpscSender<T> {
    inner: mpsc::Sender<T>,
}

impl<T: Clone> Sender<T> for MpscSender<T> {
    type Error = mpsc::SendError<T>;

    fn send(&self, t: &T) -> Result<(), Self::Error> {
        self.inner.send(t.clone())
    }
}

impl<T> Receiver<T> for mpsc::Receiver<T> {
    type Error = mpsc::RecvError;

    fn recv(&self) -> Result<T, Self::Error> {
        <Self as mpsc::Receiver<_>>::recv(self)
    }
}

pub struct Ipc<T> {
    sender: IpcSender<T>,
    receiver: IpcReceiver<T>,
}

impl<T: Deserialize + Serialize> Channel<T> for Ipc<T> {
    type Sender = IpcSender<T>;
    type Receiver = IpcReceiver<T>;

    fn channel() -> Self {
        let (sender, receiver) = ipc::channel();
        Ipc { sender: sender, receiver: receiver }
    }

    fn sender(&self) -> &Self::Sender {
        &self.sender
    }

    fn receiver(&self) -> &Self::Receiver {
        &self.receiver
    }
}

impl<T: Serialize> Sender<T> for IpcSender<T> {
    type Error = SerializeError;

    fn send(&self, t: &T) -> Result<(), Self::Error> {
        <Self as IpcSender<_>>::send(self, t)
    }
}

impl<T: Deserialize> Sender<T> for IpcReceiver<T> {
    type Error = DeserializeError;

    fn recv(&self) -> Result<T, Self::Error> {
        <Self as IpcReceiver<_>>::recv(self)
    }
}

I'm not sure if we can have a sufficiently-flexible trait system to parameterise most stuff in WR just by a Channel trait, without the T parameter.

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Feb 8, 2017

I'm not sure if we can have a sufficiently-flexible trait system to parameterise most stuff in WR just by a Channel trait, without the T parameter.

That requires higher-kinded types or associated type constructors. I think the latter is likely to be added to the languages: rust-lang/rfcs#1598 but it’s not there yet.

@antrik
Copy link
Contributor

@antrik antrik commented Feb 8, 2017

@nox note that T: Clone means you can't send a receiver over a channel...

While taking the data by reference here -- along with servo/ipc-channel#138 -- would avoid unnecessary extra copies in the IPC case if the sender wants to keep the data, at the same time it adds an unnecessary clone() in the MPSC case if the sender doesn't need to keep the data. I think the only way to be optimal in all cases would be providing separate send_copy() and send_move() methods, letting the caller choose the more appropriate one in each case...

(But since I have doubts about servo/ipc-channel#138 anyway, this point is probably moot...)

@glennw
Copy link
Member

@glennw glennw commented Feb 8, 2017

It's needed because some projects don't want the dependency / overhead of ipc-channel (they use a different IPC mechanism).

@atouchet
Copy link
Contributor

@atouchet atouchet commented Apr 26, 2020

ipc-channel was removed from WebRender. Is this issue still relevant?

@nical
Copy link
Collaborator

@nical nical commented Apr 26, 2020

Nope!

@nical nical closed this Apr 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.