ringbahn/iou

Can an API on CompletionQueue return an iterator of CQEs?

Closed this issue ยท 7 comments

Should CompletionQueue::wait_for_cqes and IoUring::wait_for_cqes actually return a vector of completion events, and then advance the queue by count entries?

Consider the following example, where I send 16 NOPs and then wait for 16 events, 16 times.

fn wait_multiple() {
    let mut iou = IoUring::new(16).expect("Can't create queue");
    let (mut sq, mut cq, _) = iou.queues();

    for _ in 0..16 {
        let mut sqe = sq.next_sqe().unwrap();

        unsafe {
            sqe.prep_nop();
        }
    }

    // I'd expect the second iteration to block, but it doesn't
    for n in 0..16 {
        println!("[{}]: Waiting for 16 events", n);
        let cqe = cq.wait_for_cqes(16).unwrap();
    }
}

It looks like Iou is correctly sending count entries, but then it just wraps the first result, which automatically advances the CompletionQueue by one entry when dropped.

I took a crack at returning an array of completions which seems to work, although I'm not sure if advancing by 1 for each dropped event in the vector is the right strategy, or even sound.

The correct way to use this API would be to then call peek_for_cqe 15 times. Returning a vec would require a heap allocation, which is not ideal.

We could find a way to drop the CQEs as one modification instead of 16, to reduce the number of allocations. Probably we'd expose the io_uring_cq_advance function as an (unsafe?) API on IoUring and CompletionQueue and then users would have to mem::forget their CQEs and call that explicitly.

We could also possibly add an API which constructs an Iterator of CQEs. Not sure how to optimize advancing in that case.

Oh OK that totally make sense, thank you for the clarification!

I do think an iterator of CQEs is possible, and possibly it can even advance once, instead of N times.

I think we could even use the same iterator type for a "wait" and "peek" multiple interface, by having the wait one just call the wait API before constructing the iterator.

I'm going to re-open and re-name to track this possibility.

The tricky bit:

Iterators have to be able to return multiple of their items with overlapping lifetimes. That is, you would have to get multiple CQEs out of the iterator. This is problematic with the way that CQE advances when it drops, because you could drop them in a different order from the order they actually appear in. So it would have to solve the drop once problem.

We'd want to make CQEs contain some flag to check whether they should advance, and only advance in their drop based on the setting of the flag. Possibly this flag could be simply whether or not the pointer to the io_uring is null or not. Not too hard.

Then, we'd need to track the number of CQEs we have returned in the iterator impl, so that it knows how far to advance.

This all seem plausible to me. And an iterator interface would be much nicer than the one we have now. I'll mess around and think about it more.

That sounds really cool. Happy to help test if it does prove feasible.

The problem is that it can't be an iterator because each CQE needs to borrow from the iterator, to make sure that cq_advance is not called until after all the CQEs have dropped. It's still a better interface though. I have a branch, you can write code like this essentially:

while let Some(cqe) = ring.wait_for_cqes(n).next_cqe() {
     // process CQE
}

// OR

ring.wait_for_cqes(n).for_each(|cqe| {
    // process CQE
})

This will be implemented in iou 0.3