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 separate signal and message flows #66

Merged
merged 6 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions orchestra/proc-macro/src/impl_subsystem_ctx_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,27 @@ pub(crate) fn impl_subsystem_context_trait_for(
}
}

async fn recv_signal(&mut self) -> ::std::result::Result<#signal, #error_ty> {
self.signals.next().await.ok_or(#support_crate ::OrchestraError::Context(
"Signal channel is terminated and empty.".to_owned(),
).into())
}

async fn recv_msg(&mut self) -> ::std::result::Result<Self::Message, #error_ty> {
loop {
if let Some((needs_signals_received, msg)) = self.pending_incoming.take() {
self.signals_received.wait_until(|v| v >= needs_signals_received).await;
return Ok(msg);
}
let msg = self.messages.next().await.ok_or(
#support_crate ::OrchestraError::Context(
"Message channel is terminated and empty.".to_owned()
)
)?;
self.pending_incoming = Some((msg.signals_received, msg.message));
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't work, since it requires two borrows of self if both streams are to be polled.

Provide a method that produces two adapters.

fn incoming_spliter(&mut) -> (SplitOfMessageStream<Self::Message>, SplitOfSignalStream) {
}

where both SplitOfMessageStream and SplitOfSignalStream does impl Stream or exposes async fn recv()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This won't work, since it requires two borrows of self if both streams are to be polled.

That is exactly what was discussed earlier in this thread :)
This comment by @sandreim explains how it is supposed to work (and before he explained that I was doubting if it could work myself).


fn sender(&mut self) -> &mut Self::Sender {
&mut self.to_subsystems
}
Expand Down
48 changes: 43 additions & 5 deletions orchestra/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub use futures::{
future::{BoxFuture, Fuse, Future},
poll, select,
stream::{self, select, select_with_strategy, FuturesUnordered, PollNext},
task::{Context, Poll},
task::{AtomicWaker, Context, Poll},
FutureExt, StreamExt,
};
#[doc(hidden)]
Expand Down Expand Up @@ -217,22 +217,54 @@ pub type SubsystemIncomingMessages<M> = self::stream::SelectWithStrategy<
(),
>;

#[derive(Debug, Default)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing documentation of its purpose.

struct SignalsReceivedInner {
waker: AtomicWaker,
value: AtomicUsize,
}

/// Future to wait on for the watermark predicate
pub struct SignalsReceivedWaiter<'a, F: Fn(usize) -> bool> {
owner: &'a SignalsReceivedInner,
predicate: F,
}

impl<F: Fn(usize) -> bool> Future for SignalsReceivedWaiter<'_, F> {
type Output = usize;

fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
self.owner.waker.register(cx.waker());
let value = self.owner.value.load(atomic::Ordering::Acquire);
if (self.predicate)(value) {
Poll::Ready(value)
} else {
Poll::Pending
}
}
}

/// Watermark to track the received signals.
#[derive(Debug, Default, Clone)]
pub struct SignalsReceived(Arc<AtomicUsize>);
pub struct SignalsReceived(Arc<SignalsReceivedInner>);

impl SignalsReceived {
/// Load the current value of received signals.
pub fn load(&self) -> usize {
// It's imperative that we prevent reading a stale value from memory because of reordering.
// Memory barrier to ensure that no reads or writes in the current thread before this load are reordered.
// All writes in other threads using release semantics become visible to the current thread.
self.0.load(atomic::Ordering::Acquire)
self.0.value.load(atomic::Ordering::Acquire)
}

/// Increase the number of signals by one.
pub fn inc(&self) {
let _previous = self.0.fetch_add(1, atomic::Ordering::AcqRel);
let _previous = self.0.value.fetch_add(1, atomic::Ordering::AcqRel);
self.0.waker.wake();
}

/// Wait until a predicate for the watermark is true.
pub fn wait_until<F: Fn(usize) -> bool>(&self, predicate: F) -> SignalsReceivedWaiter<F> {
SignalsReceivedWaiter { owner: &self.0, predicate }
}
}

Expand Down Expand Up @@ -416,9 +448,15 @@ pub trait SubsystemContext: Send + 'static {
/// using `pending!()` macro you will end up with a busy loop!
async fn try_recv(&mut self) -> Result<Option<FromOrchestra<Self::Message, Self::Signal>>, ()>;

/// Receive a message.
/// Receive a signal or a message.
async fn recv(&mut self) -> Result<FromOrchestra<Self::Message, Self::Signal>, Self::Error>;

/// Receive a signal.
async fn recv_signal(&mut self) -> Result<Self::Signal, Self::Error>;
sandreim marked this conversation as resolved.
Show resolved Hide resolved

/// Receive a message.
async fn recv_msg(&mut self) -> Result<Self::Message, Self::Error>;

/// Spawn a child task on the executor.
fn spawn(
&mut self,
Expand Down
Loading