Skip to content

Support doorbell batching for one-sided operations#78

Closed
arajni3 wants to merge 24 commits intojonhoo:mainfrom
arajni3:main
Closed

Support doorbell batching for one-sided operations#78
arajni3 wants to merge 24 commits intojonhoo:mainfrom
arajni3:main

Conversation

@arajni3
Copy link

@arajni3 arajni3 commented Sep 24, 2025

To avoid a dynamic allocation, we use a local array parameterized by a constant batch limit size.

}

/// Make a convenient `MemorySlicer`, useful for buffer pools that need to own `MemoryRegions`.
pub fn mk_slicer(&self) -> MemorySlicer {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this needed? it seems unrelated to the doorbell batching, so would be good to make a different PR and discuss there

pub fn post_one_sided_batch_single_type(
&mut self,
is_read: bool,
wrids_imms: &mut [(u64, Option<u32>)],
Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be good to simplify this function interface and avoid having two functions. could we instead introduce something like this:

struct SendWR<L: usize> {
  pub is_read: bool, // we should use a proper enum here instead,
  pub wr_id: u64,
  pub imm_data: Option<u32>,
  pub locals: SmallVec::<[LocalMemorySlice; L]>,
  pub remote: RemoteMemorySlice,
};

then a single post function that accepts &[SendWR] might be enough for this use case and also the use case in #72


/// The doorbell batching API(s) will process only up to these many work requests at a time
/// to prevent inefficient dynamic heap allocations.
pub const DOORBELL_BATCH_LIMIT: usize = 32;
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of specifying this as a constant, could we make this a template argument to the post(&[SendWR]) function so that the user has control over it. we can default to 32

@arajni3 arajni3 closed this Oct 14, 2025
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.

2 participants