From 9ed2881e7653b4817a26bb389bb97756335b689f Mon Sep 17 00:00:00 2001 From: Anand Bonde Date: Tue, 7 Jan 2025 10:05:24 -0800 Subject: [PATCH] [profiler] Enhancement: panic in incorrect timer usage Nesting async_timer!() within a timer!() scope causes reparenting issues in the profiler. To avoid such invalid uses in future, changing to panic if this happens. Also, removed all timers that violated this invariant. --- src/rust/catnap/linux/transport.rs | 21 ++++----------------- src/rust/demikernel/libos/mod.rs | 8 -------- src/rust/perftools/profiler/mod.rs | 4 +--- 3 files changed, 5 insertions(+), 28 deletions(-) diff --git a/src/rust/catnap/linux/transport.rs b/src/rust/catnap/linux/transport.rs index 1ca2977ab..a1fe6d8e1 100644 --- a/src/rust/catnap/linux/transport.rs +++ b/src/rust/catnap/linux/transport.rs @@ -26,7 +26,6 @@ use crate::{ }, poll_yield, DemiRuntime, SharedDemiRuntime, SharedObject, }, - timer, }; use ::futures::FutureExt; use ::slab::Slab; @@ -254,8 +253,6 @@ impl NetworkTransport for SharedCatnapTransport { /// Creates a new socket on the underlying network transport. We only support IPv4 and UDP and TCP sockets for now. fn socket(&mut self, domain: Domain, typ: Type) -> Result { - timer!("catnap::linux::transport::socket"); - // Select protocol. let protocol: Protocol = match typ { Type::STREAM => Protocol::TCP, Type::DGRAM => Protocol::UDP, @@ -430,7 +427,6 @@ impl NetworkTransport for SharedCatnapTransport { /// Binds a socket to [local] on the underlying network transport. fn bind(&mut self, sd: &mut Self::SocketDescriptor, local: SocketAddr) -> Result<(), Fail> { - timer!("catnap::linux::transport::bind"); trace!("Bind to {:?}", local); let socket: &mut Socket = self.socket_from_sd(sd); @@ -465,8 +461,6 @@ impl NetworkTransport for SharedCatnapTransport { /// Sets a socket to passive listening on the underlying transport and registers it to accept incoming connections /// with epoll. fn listen(&mut self, sd: &mut Self::SocketDescriptor, backlog: usize) -> Result<(), Fail> { - timer!("catnap::linux::transport::listen"); - trace!("Listen to"); if let Err(e) = self.socket_from_sd(sd).listen(backlog as i32) { let cause: String = format!("failed to listen on socket: {:?}", e); error!("listen(): {}", cause); @@ -483,7 +477,6 @@ impl NetworkTransport for SharedCatnapTransport { /// Accept the next incoming connection. This function blocks until a new connection arrives from the underlying /// transport. async fn accept(&mut self, sd: &mut Self::SocketDescriptor) -> Result<(Self::SocketDescriptor, SocketAddr), Fail> { - timer!("catnap::linux::transport::accept"); let (new_socket, addr) = self.data_from_sd(sd).accept().await?; // Set socket options. if let Err(e) = new_socket.set_reuse_address(true) { @@ -514,7 +507,6 @@ impl NetworkTransport for SharedCatnapTransport { /// Connect to [remote] through the underlying transport. This function blocks until the connect succeeds or fails /// with an error. async fn connect(&mut self, sd: &mut Self::SocketDescriptor, remote: SocketAddr) -> Result<(), Fail> { - timer!("catnap::linux::transport::connect"); self.data_from_sd(sd).move_socket_to_active(); self.register_epoll(&sd, (libc::EPOLLIN | libc::EPOLLOUT) as u32)?; @@ -538,7 +530,6 @@ impl NetworkTransport for SharedCatnapTransport { /// Close the socket and block until close completes. async fn close(&mut self, sd: &mut Self::SocketDescriptor) -> Result<(), Fail> { - timer!("catnap::linux::transport::close"); let data: &mut SharedSocketData = self.data_from_sd(sd); loop { // Close the socket. @@ -577,13 +568,10 @@ impl NetworkTransport for SharedCatnapTransport { buf: &mut DemiBuffer, addr: Option, ) -> Result<(), Fail> { - timer!("catnap::linux::transport::push"); - { - self.data_from_sd(sd).push(addr, buf.clone()).await?; - // Clear out the original buffer. - expect_ok!(buf.trim(buf.len()), "Should be able to empty the buffer"); - Ok(()) - } + self.data_from_sd(sd).push(addr, buf.clone()).await?; + // Clear out the original buffer. + expect_ok!(buf.trim(buf.len()), "Should be able to empty the buffer"); + Ok(()) } /// Pop a [buf] of at most [size] from the underlying transport. This function blocks until the socket has data to @@ -594,7 +582,6 @@ impl NetworkTransport for SharedCatnapTransport { sd: &mut Self::SocketDescriptor, size: usize, ) -> Result<(Option, DemiBuffer), Fail> { - timer!("catnap::linux::transport::pop"); self.data_from_sd(sd).pop(size).await } diff --git a/src/rust/demikernel/libos/mod.rs b/src/rust/demikernel/libos/mod.rs index a3ea91bdd..897399577 100644 --- a/src/rust/demikernel/libos/mod.rs +++ b/src/rust/demikernel/libos/mod.rs @@ -206,7 +206,6 @@ impl LibOS { #[allow(unused_variables)] pub fn accept(&mut self, sockqd: QDesc) -> Result { let result: Result = { - timer!("demikernel::accept"); match self { LibOS::NetworkLibOS(libos) => libos.accept(sockqd), } @@ -220,7 +219,6 @@ impl LibOS { #[allow(unused_variables)] pub fn connect(&mut self, sockqd: QDesc, remote: SocketAddr) -> Result { let result: Result = { - timer!("demikernel::connect"); match self { LibOS::NetworkLibOS(libos) => libos.connect(sockqd, remote), } @@ -234,7 +232,6 @@ impl LibOS { /// Closes an I/O queue. async_close() + wait() achieves the same effect as this synchronous function. pub fn close(&mut self, qd: QDesc) -> Result<(), Fail> { let result: Result<(), Fail> = { - timer!("demikernel::close"); match self { LibOS::NetworkLibOS(libos) => match libos.async_close(qd) { Ok(qt) => match self.wait(qt, None) { @@ -253,7 +250,6 @@ impl LibOS { pub fn async_close(&mut self, qd: QDesc) -> Result { let result: Result = { - timer!("demikernel::async_close"); match self { LibOS::NetworkLibOS(libos) => libos.async_close(qd), } @@ -267,7 +263,6 @@ impl LibOS { /// Pushes a scatter-gather array to an I/O queue. pub fn push(&mut self, qd: QDesc, sga: &demi_sgarray_t) -> Result { let result: Result = { - timer!("demikernel::push"); match self { LibOS::NetworkLibOS(libos) => libos.push(qd, sga), } @@ -282,7 +277,6 @@ impl LibOS { #[allow(unused_variables)] pub fn pushto(&mut self, qd: QDesc, sga: &demi_sgarray_t, to: SocketAddr) -> Result { let result: Result = { - timer!("demikernel::pushto"); match self { LibOS::NetworkLibOS(libos) => libos.pushto(qd, sga, to), } @@ -296,8 +290,6 @@ impl LibOS { /// Pops data from a an I/O queue. pub fn pop(&mut self, qd: QDesc, size: Option) -> Result { let result: Result = { - timer!("demikernel::pop"); - // Check if this is a fixed-size pop. if let Some(size) = size { // Check if size is valid. diff --git a/src/rust/perftools/profiler/mod.rs b/src/rust/perftools/profiler/mod.rs index fce07c8e1..fb46b6720 100644 --- a/src/rust/perftools/profiler/mod.rs +++ b/src/rust/perftools/profiler/mod.rs @@ -155,9 +155,7 @@ impl Profiler { current_scope.parent_scope.as_ref().cloned() } else { // This should not happen with proper usage. - log::error!("Called perftools::profiler::leave() while not in any scope"); - - None + unreachable!("Called perftools::profiler::leave() while not in any scope"); }; }