Skip to content

Commit

Permalink
Merge pull request #1483 from microsoft/ab-profiler-reduce-timers
Browse files Browse the repository at this point in the history
[profiler] Enhancment: panic on wrong timer usage
  • Loading branch information
anandbonde authored Jan 9, 2025
2 parents a8ea334 + 9ed2881 commit 87db761
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 28 deletions.
21 changes: 4 additions & 17 deletions src/rust/catnap/linux/transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use crate::{
},
poll_yield, DemiRuntime, SharedDemiRuntime, SharedObject,
},
timer,
};
use ::futures::FutureExt;
use ::slab::Slab;
Expand Down Expand Up @@ -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<Self::SocketDescriptor, Fail> {
timer!("catnap::linux::transport::socket");
// Select protocol.
let protocol: Protocol = match typ {
Type::STREAM => Protocol::TCP,
Type::DGRAM => Protocol::UDP,
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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)?;

Expand All @@ -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.
Expand Down Expand Up @@ -577,13 +568,10 @@ impl NetworkTransport for SharedCatnapTransport {
buf: &mut DemiBuffer,
addr: Option<SocketAddr>,
) -> 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
Expand All @@ -594,7 +582,6 @@ impl NetworkTransport for SharedCatnapTransport {
sd: &mut Self::SocketDescriptor,
size: usize,
) -> Result<(Option<SocketAddr>, DemiBuffer), Fail> {
timer!("catnap::linux::transport::pop");
self.data_from_sd(sd).pop(size).await
}

Expand Down
8 changes: 0 additions & 8 deletions src/rust/demikernel/libos/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ impl LibOS {
#[allow(unused_variables)]
pub fn accept(&mut self, sockqd: QDesc) -> Result<QToken, Fail> {
let result: Result<QToken, Fail> = {
timer!("demikernel::accept");
match self {
LibOS::NetworkLibOS(libos) => libos.accept(sockqd),
}
Expand All @@ -220,7 +219,6 @@ impl LibOS {
#[allow(unused_variables)]
pub fn connect(&mut self, sockqd: QDesc, remote: SocketAddr) -> Result<QToken, Fail> {
let result: Result<QToken, Fail> = {
timer!("demikernel::connect");
match self {
LibOS::NetworkLibOS(libos) => libos.connect(sockqd, remote),
}
Expand All @@ -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) {
Expand All @@ -253,7 +250,6 @@ impl LibOS {

pub fn async_close(&mut self, qd: QDesc) -> Result<QToken, Fail> {
let result: Result<QToken, Fail> = {
timer!("demikernel::async_close");
match self {
LibOS::NetworkLibOS(libos) => libos.async_close(qd),
}
Expand All @@ -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<QToken, Fail> {
let result: Result<QToken, Fail> = {
timer!("demikernel::push");
match self {
LibOS::NetworkLibOS(libos) => libos.push(qd, sga),
}
Expand All @@ -282,7 +277,6 @@ impl LibOS {
#[allow(unused_variables)]
pub fn pushto(&mut self, qd: QDesc, sga: &demi_sgarray_t, to: SocketAddr) -> Result<QToken, Fail> {
let result: Result<QToken, Fail> = {
timer!("demikernel::pushto");
match self {
LibOS::NetworkLibOS(libos) => libos.pushto(qd, sga, to),
}
Expand All @@ -296,8 +290,6 @@ impl LibOS {
/// Pops data from a an I/O queue.
pub fn pop(&mut self, qd: QDesc, size: Option<usize>) -> Result<QToken, Fail> {
let result: Result<QToken, Fail> = {
timer!("demikernel::pop");

// Check if this is a fixed-size pop.
if let Some(size) = size {
// Check if size is valid.
Expand Down
4 changes: 1 addition & 3 deletions src/rust/perftools/profiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
};
}

Expand Down

0 comments on commit 87db761

Please sign in to comment.