From 96e50eb79575c93f216542f62c6bd8e72763266c Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Fri, 6 May 2022 17:02:45 -0700 Subject: [PATCH] base: return SmallVec directly from platform wait impls On Windows, this changes the EventContext::wait() return type from Vec to SmallVec; this removes an extra conversion step, since the only caller, WaitContext::wait(), constructed a SmallVec from the return value anyway. On Linux, this lets us get rid of the PollEventsOwned structure, since the SmallVec contains the events and does not need to point back at a separate type for storage. BUG=b:213153157 TEST=tools/dev_container tools/presubmit --all Change-Id: I7857a306ad71be020af309d4186d9e3e651fcc05 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3633110 Reviewed-by: Noah Gold Tested-by: kokoro Reviewed-by: Keiichi Watanabe Commit-Queue: Daniel Verkamp --- base/src/sys/unix/poll.rs | 78 +++++++++------------------------- base/src/sys/windows/events.rs | 44 ------------------- base/src/sys/windows/wait.rs | 14 +++--- base/src/wait_context.rs | 12 +----- 4 files changed, 30 insertions(+), 118 deletions(-) diff --git a/base/src/sys/unix/poll.rs b/base/src/sys/unix/poll.rs index 9ab2cf498d..d47f93ffc9 100644 --- a/base/src/sys/unix/poll.rs +++ b/base/src/sys/unix/poll.rs @@ -18,9 +18,12 @@ use libc::{ EPOLLRDHUP, EPOLL_CLOEXEC, EPOLL_CTL_ADD, EPOLL_CTL_DEL, EPOLL_CTL_MOD, }; use log::warn; +use smallvec::SmallVec; use super::{errno_result, Result}; -use crate::{AsRawDescriptor, EventType, FromRawDescriptor, IntoRawDescriptor, RawDescriptor}; +use crate::{ + AsRawDescriptor, EventType, FromRawDescriptor, IntoRawDescriptor, RawDescriptor, TriggeredEvent, +}; const POLL_CONTEXT_MAX_EVENTS: usize = 16; @@ -188,16 +191,6 @@ pub struct PollEvents<'a, T> { } impl<'a, T: PollToken> PollEvents<'a, T> { - /// Copies the events to an owned structure so the reference to this (and by extension - /// `PollContext`) can be dropped. - pub fn to_owned(&self) -> PollEventsOwned { - PollEventsOwned { - count: self.count, - events: RefCell::new(*self.events), - tokens: PhantomData, - } - } - /// Iterates over each event. pub fn iter(&self) -> PollEventIter, T> { PollEventIter { @@ -244,24 +237,6 @@ impl<'a, T: PollToken> IntoIterator for &'a PollEvents<'_, T> { } } -/// A deep copy of the event records from `PollEvents`. -pub struct PollEventsOwned { - count: usize, - events: RefCell<[epoll_event; POLL_CONTEXT_MAX_EVENTS]>, - tokens: PhantomData, // Needed to satisfy usage of T -} - -impl PollEventsOwned { - /// Takes a reference to the events so that they can be iterated via methods in `PollEvents`. - pub fn as_ref(&self) -> PollEvents { - PollEvents { - count: self.count, - events: self.events.borrow(), - tokens: PhantomData, - } - } -} - /// Watching events taken by PollContext. pub struct WatchingEvents(u32); @@ -506,25 +481,7 @@ impl IntoRawDescriptor for EpollContext { /// Used to poll multiple objects that have file descriptors. /// -/// # Example -/// -/// ``` -/// # use base::platform::{Result, Event, PollContext, PollEvents}; -/// # fn test() -> Result<()> { -/// let evt1 = Event::new()?; -/// let evt2 = Event::new()?; -/// evt2.write(1)?; -/// -/// let ctx: PollContext = PollContext::new()?; -/// ctx.add(&evt1, 1)?; -/// ctx.add(&evt2, 2)?; -/// -/// let pollevents: PollEvents = ctx.wait()?; -/// let tokens: Vec = pollevents.iter_readable().map(|e| e.token()).collect(); -/// assert_eq!(&tokens[..], &[2]); -/// # Ok(()) -/// # } -/// ``` +/// See [`crate::WaitContext`] for an example that uses the cross-platform wrapper. pub struct PollContext { epoll_ctx: EpollContext, @@ -679,10 +636,7 @@ impl PollContext { /// return immediately. The consequence of not handling an event perpetually while calling /// `wait` is that the callers loop will degenerated to busy loop polling, pinning a CPU to /// ~100% usage. - /// - /// # Panics - /// Panics if the returned `PollEvents` structure is not dropped before subsequent `wait` calls. - pub fn wait(&self) -> Result> { + pub fn wait(&self) -> Result; 16]>> { self.wait_timeout(Duration::new(i64::MAX as u64, 0)) } @@ -690,11 +644,19 @@ impl PollContext { /// /// This may return earlier than `timeout` with zero events if the duration indicated exceeds /// system limits. - pub fn wait_timeout(&self, timeout: Duration) -> Result> { + pub fn wait_timeout(&self, timeout: Duration) -> Result; 16]>> { let events = self.epoll_ctx.wait_timeout(&self.events, timeout)?; let hangups = events.iter_hungup().count(); self.check_for_hungup_busy_loop(hangups); - Ok(events) + Ok(events + .iter() + .map(|event| TriggeredEvent { + token: event.token(), + is_readable: event.readable(), + is_writable: event.writable(), + is_hungup: event.hungup(), + }) + .collect()) } } @@ -726,9 +688,9 @@ mod tests { let mut evt_count = 0; while evt_count < 2 { - for event in ctx.wait().unwrap().iter_readable() { + for event in ctx.wait().unwrap().iter().filter(|e| e.is_readable) { evt_count += 1; - match event.token() { + match event.token { 1 => { evt1.read().unwrap(); ctx.delete(&evt1).unwrap(); @@ -757,8 +719,8 @@ mod tests { } let mut evt_count = 0; while evt_count < EVT_COUNT { - for event in ctx.wait().unwrap().iter_readable() { - evts[event.token()].read().unwrap(); + for event in ctx.wait().unwrap().iter().filter(|e| e.is_readable) { + evts[event.token].read().unwrap(); evt_count += 1; } } diff --git a/base/src/sys/windows/events.rs b/base/src/sys/windows/events.rs index 860ace30f3..5a06329b09 100644 --- a/base/src/sys/windows/events.rs +++ b/base/src/sys/windows/events.rs @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -use std::{clone::Clone, default::Default, marker::Copy}; - pub use super::wait::*; use super::{PollToken, RawDescriptor}; use crate::descriptor::AsRawDescriptor; @@ -34,48 +32,6 @@ impl Clone for EventTrigger { } } -/// Represents an event that has been signaled and waited for via a wait function. -#[derive(Copy, Clone, Debug)] -pub struct TriggeredEvent { - pub token: T, - pub is_readable: bool, - pub is_writable: bool, - pub is_hungup: bool, -} - -impl Default for TriggeredEvent { - fn default() -> Self { - TriggeredEvent { - token: T::from_raw_token(0), - is_readable: false, - is_writable: false, - is_hungup: false, - } - } -} - -impl TriggeredEvent { - /// Gets the token associated in `PollContext::add` with this event. - pub fn token(&self) -> T { - T::from_raw_token(self.token.as_raw_token()) - } - - /// True if the `fd` associated with this token in `PollContext::add` is readable. - pub fn readable(&self) -> bool { - self.is_readable - } - - /// True if the `fd` associated with this token in `PollContext::add` is writable. - pub fn writable(&self) -> bool { - self.is_writable - } - - /// True if the `fd` associated with this token in `PollContext::add` has been hungup on. - pub fn hungup(&self) -> bool { - self.is_hungup - } -} - #[cfg(test)] mod tests { use super::{super::Event, *}; diff --git a/base/src/sys/windows/wait.rs b/base/src/sys/windows/wait.rs index 43e5cde04e..821bf7301c 100644 --- a/base/src/sys/windows/wait.rs +++ b/base/src/sys/windows/wait.rs @@ -4,6 +4,7 @@ use std::{cmp::min, collections::HashMap, os::windows::io::RawHandle, sync::Arc, time::Duration}; +use smallvec::SmallVec; use sync::Mutex; use winapi::{ shared::{ @@ -13,9 +14,10 @@ use winapi::{ um::{synchapi::WaitForMultipleObjects, winbase::WAIT_OBJECT_0}, }; -use super::{errno_result, Error, Event, EventTrigger, PollToken, Result, TriggeredEvent}; +use super::{errno_result, Error, Event, EventTrigger, PollToken, Result}; use crate::descriptor::{AsRawDescriptor, Descriptor}; -use crate::{error, EventToken, EventType, RawDescriptor, WaitContext}; +use crate::{error, EventToken, EventType, RawDescriptor, TriggeredEvent, WaitContext}; + // MAXIMUM_WAIT_OBJECTS = 64 pub const MAXIMUM_WAIT_OBJECTS: usize = winapi::um::winnt::MAXIMUM_WAIT_OBJECTS as usize; @@ -165,11 +167,11 @@ impl EventContext { } /// Waits for one or more of the registered triggers to become signaled. - pub fn wait(&self) -> Result>> { + pub fn wait(&self) -> Result; 16]>> { self.wait_timeout(Duration::new(i64::MAX as u64, 0)) } - pub fn wait_timeout(&self, timeout: Duration) -> Result>> { + pub fn wait_timeout(&self, timeout: Duration) -> Result; 16]>> { let raw_handles_list: Vec = self .registered_handles .lock() @@ -212,7 +214,7 @@ impl EventContext { return self.wait_timeout(timeout); } - let mut events_to_return: Vec> = vec![]; + let mut events_to_return = SmallVec::<[TriggeredEvent; 16]>::new(); // Multiple events may be triggered at once, but WaitForMultipleObjects will only return one. // Once it returns, loop through the remaining triggers checking each to ensure they haven't // also been triggered. @@ -257,7 +259,7 @@ impl EventContext { Ok(events_to_return) } - WAIT_TIMEOUT => Ok(vec![]), + WAIT_TIMEOUT => Ok(Default::default()), // Invalid cases. This is most likely an WAIT_FAILED, but anything not matched by the // above is an error case. _ => errno_result(), diff --git a/base/src/wait_context.rs b/base/src/wait_context.rs index 2eeeae39ae..59d607548f 100644 --- a/base/src/wait_context.rs +++ b/base/src/wait_context.rs @@ -146,19 +146,11 @@ impl WaitContext { pub fn wait(&self) -> Result; 16]>> { self.wait_timeout(Duration::new(i64::MAX as u64, 0)) } + /// Waits for one or more of the registered triggers to become signaled, failing if no triggers /// are signaled before the designated timeout has elapsed. pub fn wait_timeout(&self, timeout: Duration) -> Result; 16]>> { - let events = self.0.wait_timeout(timeout)?; - Ok(events - .iter() - .map(|event| TriggeredEvent { - token: event.token(), - is_readable: event.readable(), - is_writable: event.writable(), - is_hungup: event.hungup(), - }) - .collect()) + self.0.wait_timeout(timeout) } }