Merge common impls of submit_one_event into submit_event_chain
#551
Conversation
| fn submit_one_event(&mut self, queue: &mut VecDeque<UringDescriptor>) -> Option<bool> { | ||
| let source_map = &mut *self.source_map.borrow_mut(); | ||
| let now = Instant::now(); | ||
|
|
||
| while let Some(chain) = peek_one_chain(queue, self.size) { | ||
| return if let Some(sqes) = self.ring.sq().prepare_sqes(chain.len() as u32) { | ||
| let ops = extract_one_chain(source_map, queue, chain, now); | ||
| if ops.is_empty() { | ||
| // all the sources in the ring were cancelled | ||
| continue; | ||
| } | ||
|
|
||
| for (op, mut sqe) in ops.into_iter().zip(sqes.into_iter()) { | ||
| let allocator = self.allocator.clone(); | ||
| fill_sqe( | ||
| &mut sqe, | ||
| &op, | ||
| move |size| allocator.new_buffer(size), | ||
| source_map, | ||
| ); | ||
| } | ||
| Some(true) | ||
| } else { | ||
| None | ||
| }; | ||
| } | ||
| Some(false) | ||
| submit_event_chain( | ||
| &mut *self.source_map.borrow_mut(), | ||
| &mut self.ring, | ||
| self.allocator.clone(), | ||
| queue, | ||
| self.size, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Looks like we could go one step further and remove submit_one_event from the trait and avoid the indirection of the vtable
There was a problem hiding this comment.
How do we do that? submit_one_event is used inside submit_one_event which is used in the default trait implementation where we cannot refer concrete fields like self.ring.
|
Hey @vmingchen -- In the future, can you either fill out the pull request template or at least delete it (if it's not relevant, though that's also debatable..)? It was surprising to come read this and realize it was just the template unchanged. |
Sorry about that. Edited the PR description. |
|
anything missing for merging this ? |
|
@glommer this seems OK to merge. |
What does this PR do?
Address #548
Motivation
Refactor code to reduce code duplication.
Related issues
A list of issues either fixed, containing architectural discussions, otherwise relevant
for this Pull Request.
Additional Notes
Pure refactoring; no behavior change.
Checklist
[] I have added unit tests to the code I am submitting
[] My unit tests cover both failure and success scenarios
[] If applicable, I have discussed my architecture