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

feat: Add scheduling options for listener lists. #149

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jevolk
Copy link

@jevolk jevolk commented Dec 25, 2024

Two schedulers are available:

  • FIFO: The original round-robin queuing; listeners are inserted at the back.
  • LIFO: The new most-recent queuing; listeners are inserted at the front.

LIFO queuing is beneficial for cache-efficiency with workloads that are tolerant of starvation. The same listener is repeatedly drawn from the list until the load dictates additional listeners be drawn from the list. These listeners expand outward as a "hot set" for optimal reuse of resources rather than continuously drawing from the coldest resources in a FIFO schedule.

src/lib.rs Outdated
@@ -127,6 +127,16 @@ use sync::WithMut;
use notify::NotificationPrivate;
pub use notify::{IntoNotification, Notification};

/// Queue schedule of a listener.
#[derive(Clone, Copy, Debug, PartialEq)]
pub enum Sched {
Copy link
Member

Choose a reason for hiding this comment

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

This should be #[non_exhaustive].

@@ -127,6 +127,16 @@ use sync::WithMut;
use notify::NotificationPrivate;
pub use notify::{IntoNotification, Notification};

/// Queue schedule of a listener.
#[derive(Clone, Copy, Debug, PartialEq)]
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer a name more descriptive than Sched. Maybe QueueStrategy?

src/intrusive.rs Outdated
Comment on lines 247 to 259
match mem::replace(replacing, Some(entry.into())) {
None => *replacing = Some(entry.into()),
Some(t) if self.sched == Sched::Lifo => unsafe {
t.as_ref().prev.set(Some(entry.into()))
},
Some(t) if self.sched == Sched::Fifo => unsafe {
t.as_ref().next.set(Some(entry.into()))
},
Some(_) => unimplemented!("unimplemented scheduling type"),
};
}
Copy link
Member

Choose a reason for hiding this comment

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

Would prefer it if this block became:

match (mem::replace(replacing, Some(entry.into())), self.sched) {
    /* ... */
}

Two strategies are available:
- FIFO: The original round-robin queuing; listeners are inserted at the back.
- LIFO: The new most-recent queuing; listeners are inserted at the front.

LIFO queuing is beneficial for cache-efficiency with workloads that are
tolerant of starvation. The same listener is repeatedly drawn from the list
until the load dictates additional listeners be drawn from the list. These
listeners expand outward as a "hot set" for optimal reuse of resources rather
than continuously drawing from the coldest resources in a FIFO schedule.

Signed-off-by: Jason Volk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants