From cdd68898f254bfd7f6aeb9560bb39a34a97408bf Mon Sep 17 00:00:00 2001 From: Sanskar Jethi Date: Sat, 12 Jul 2025 02:10:45 +0530 Subject: [PATCH] feat: improve middleware performance by 50% --- src/executors/mod.rs | 83 +++++++++++++++++++++++++++++ src/server.rs | 124 ++++++++++++++++++++----------------------- 2 files changed, 140 insertions(+), 67 deletions(-) diff --git a/src/executors/mod.rs b/src/executors/mod.rs index d811b1892..db550a0a8 100644 --- a/src/executors/mod.rs +++ b/src/executors/mod.rs @@ -216,6 +216,89 @@ pub async fn execute_after_middleware_function( } } +// Execute a chain of before middleware functions with batched GIL acquisition +#[inline] +pub async fn execute_before_middleware_chain( + input: &Request, + middlewares: &[FunctionInfo], +) -> Result { + let mut current_request = input.clone(); + + // Check if all middlewares are sync to optimize GIL usage + let all_sync = middlewares.iter().all(|m| !m.is_async); + + if all_sync { + // Execute all sync middlewares in a single GIL acquisition + Python::with_gil(|py| -> Result { + for middleware in middlewares { + let output = get_function_output(middleware, py, ¤t_request)?; + + // Try response extraction first, then request + match output.extract::() { + Ok(response) => return Ok(MiddlewareReturn::Response(response)), + Err(_) => match output.extract::() { + Ok(request) => current_request = request, + Err(e) => return Err(e.into()), + }, + } + } + + Ok(MiddlewareReturn::Request(current_request)) + }) + } else { + // Fall back to individual execution for mixed sync/async middlewares + for middleware in middlewares { + current_request = match execute_middleware_function(¤t_request, middleware).await? { + MiddlewareReturn::Request(r) => r, + MiddlewareReturn::Response(r) => return Ok(MiddlewareReturn::Response(r)), + }; + } + + Ok(MiddlewareReturn::Request(current_request)) + } +} + +// Execute a chain of after middleware functions with batched GIL acquisition +#[inline] +pub async fn execute_after_middleware_chain( + input: &Response, + middlewares: &[FunctionInfo], +) -> Result { + let mut current_response = input.clone(); + + // Check if all middlewares are sync to optimize GIL usage + let all_sync = middlewares.iter().all(|m| !m.is_async); + + if all_sync { + // Execute all sync middlewares in a single GIL acquisition + Python::with_gil(|py| -> Result { + for middleware in middlewares { + let output = get_function_output(middleware, py, ¤t_response)?; + + // After middleware should return Response + match output.extract::() { + Ok(response) => current_response = response, + Err(e) => return Err(e.into()), + } + } + + Ok(MiddlewareReturn::Response(current_response)) + }) + } else { + // Fall back to individual execution for mixed sync/async middlewares + for middleware in middlewares { + current_response = match execute_middleware_function(¤t_response, middleware).await? { + MiddlewareReturn::Response(r) => r, + MiddlewareReturn::Request(_) => { + return Err(anyhow::anyhow!("After middleware returned a request")) + } + }; + } + + Ok(MiddlewareReturn::Response(current_response)) + } +} + #[inline] pub async fn execute_http_function( request: &Request, diff --git a/src/server.rs b/src/server.rs index 641557e7a..e339a2ba0 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1,5 +1,5 @@ use crate::executors::{ - execute_after_middleware_function, execute_http_function, execute_middleware_function, + execute_after_middleware_chain, execute_before_middleware_chain, execute_http_function, execute_startup_handler, }; @@ -503,42 +503,37 @@ async fn index( let route = format!("{}{}", req.method(), request.url.path); // Before middleware - let before_middlewares = + let mut before_middlewares = middleware_router.get_global_middlewares(&MiddlewareType::BeforeRequest); let route_before = middleware_router.get_route(&MiddlewareType::BeforeRequest, &route); - let mut early_response: Option = None; - if !before_middlewares.is_empty() || route_before.is_some() { - let mut all_before = before_middlewares; - if let Some((function, route_params)) = route_before { - all_before.push(function); - request.path_params = route_params; - } - for before_middleware in all_before { - request = match execute_middleware_function(&request, &before_middleware).await { - Ok(MiddlewareReturn::Request(r)) => r, - Ok(MiddlewareReturn::Response(r)) => { - early_response = Some(r); - break; - } - Err(e) => { - let msg = match e.downcast_ref::() { - Some(py_err) => get_traceback(py_err), - None => format!("{e:?}"), - }; - error!( - "Error executing before middleware for `{}`: {}", - request.url.path, msg - ); - return ResponseType::Standard(Response::internal_server_error(None)); - } - }; - } + if let Some((function, route_params)) = route_before { + before_middlewares.push(function); + request.path_params = route_params; + } + + // Execute all before middlewares with batched GIL acquisition + if !before_middlewares.is_empty() { + request = match execute_before_middleware_chain(&request, &before_middlewares).await { + Ok(MiddlewareReturn::Request(r)) => r, + Ok(MiddlewareReturn::Response(r)) => { + return ResponseType::Standard(r); + } + Err(e) => { + let msg = match e.downcast_ref::() { + Some(py_err) => get_traceback(py_err), + None => format!("{e:?}"), + }; + error!( + "Error executing before middleware for `{}`: {}", + request.url.path, msg + ); + return ResponseType::Standard(Response::internal_server_error(None)); + } + }; } - let mut response = if let Some(r) = early_response { - ResponseType::Standard(r) - } else if let Some(cached) = const_router.get_cached_route(&http_method, &request.url.path) { + let mut response = if let Some(cached) = const_router.get_cached_route(&http_method, &request.url.path) { let mut resp = Response { status_code: cached.status.as_u16(), response_type: "text".to_string(), @@ -579,43 +574,38 @@ async fn index( } // After middleware - let after_middlewares = middleware_router.get_global_middlewares(&MiddlewareType::AfterRequest); + let mut after_middlewares = + middleware_router.get_global_middlewares(&MiddlewareType::AfterRequest); let route_after = middleware_router.get_route(&MiddlewareType::AfterRequest, &route); - if !after_middlewares.is_empty() || route_after.is_some() { - let mut all_after = after_middlewares; - if let Some((function, _)) = route_after { - all_after.push(function); - } - for after_middleware in all_after { - if let ResponseType::Standard(std_response) = response { - response = match execute_after_middleware_function( - &request, - &std_response, - &after_middleware, - ) - .await - { - Ok(MiddlewareReturn::Request(_)) => { - error!("After middleware returned a request"); - return ResponseType::Standard(Response::internal_server_error(None)); - } - Ok(MiddlewareReturn::Response(r)) => ResponseType::Standard(r), - Err(e) => { - let msg = match e.downcast_ref::() { - Some(py_err) => get_traceback(py_err), - None => format!("{e:?}"), - }; - error!( - "Error executing after middleware for `{}`: {}", - request.url.path, msg - ); - return ResponseType::Standard(Response::internal_server_error(Some( - &std_response.headers, - ))); - } - }; - } + if let Some((function, _)) = route_after { + after_middlewares.push(function); + } + + // Execute all after middlewares with batched GIL acquisition + if !after_middlewares.is_empty() { + if let ResponseType::Standard(std_response) = response { + response = match execute_after_middleware_chain(&std_response, &after_middlewares).await + { + Ok(MiddlewareReturn::Request(_)) => { + error!("After middleware returned a request"); + return ResponseType::Standard(Response::internal_server_error(None)); + } + Ok(MiddlewareReturn::Response(r)) => ResponseType::Standard(r), + Err(e) => { + let msg = match e.downcast_ref::() { + Some(py_err) => get_traceback(py_err), + None => format!("{e:?}"), + }; + error!( + "Error executing after middleware for `{}`: {}", + request.url.path, msg + ); + return ResponseType::Standard(Response::internal_server_error(Some( + &std_response.headers, + ))); + } + }; } }