mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Reduce typed app-server request serialization overhead (#34766)
## What changed - Use `ClientRequest::method_name()` directly when preparing typed request diagnostics for in-process and remote clients. - Allocate the owned method name only when constructing an error. - Remove the serialization-based method lookup and the redundant allocating `ClientRequest::method()` helper. GitOrigin-RevId: 93859e512a4f20204e0db4f929a2baebe4374ec5
This commit is contained in:
@@ -617,20 +617,22 @@ impl InProcessAppServerClient {
|
||||
where
|
||||
T: DeserializeOwned,
|
||||
{
|
||||
let method = request_method_name(&request);
|
||||
let method = request.method_name();
|
||||
let response =
|
||||
self.request(request)
|
||||
.await
|
||||
.map_err(|source| TypedRequestError::Transport {
|
||||
method: method.clone(),
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})?;
|
||||
let result = response.map_err(|source| TypedRequestError::Server {
|
||||
method: method.clone(),
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})?;
|
||||
serde_json::from_value(result)
|
||||
.map_err(|source| TypedRequestError::Deserialize { method, source })
|
||||
serde_json::from_value(result).map_err(|source| TypedRequestError::Deserialize {
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})
|
||||
}
|
||||
|
||||
/// Sends a typed client notification.
|
||||
@@ -790,20 +792,22 @@ impl InProcessAppServerRequestHandle {
|
||||
where
|
||||
T: DeserializeOwned,
|
||||
{
|
||||
let method = request_method_name(&request);
|
||||
let method = request.method_name();
|
||||
let response =
|
||||
self.request(request)
|
||||
.await
|
||||
.map_err(|source| TypedRequestError::Transport {
|
||||
method: method.clone(),
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})?;
|
||||
let result = response.map_err(|source| TypedRequestError::Server {
|
||||
method: method.clone(),
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})?;
|
||||
serde_json::from_value(result)
|
||||
.map_err(|source| TypedRequestError::Deserialize { method, source })
|
||||
serde_json::from_value(result).map_err(|source| TypedRequestError::Deserialize {
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -904,20 +908,6 @@ impl AppServerClient {
|
||||
}
|
||||
}
|
||||
|
||||
/// Extracts the JSON-RPC method name for diagnostics without extending the
|
||||
/// protocol crate with in-process-only helpers.
|
||||
pub(crate) fn request_method_name(request: &ClientRequest) -> String {
|
||||
serde_json::to_value(request)
|
||||
.ok()
|
||||
.and_then(|value| {
|
||||
value
|
||||
.get("method")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(ToOwned::to_owned)
|
||||
})
|
||||
.unwrap_or_else(|| "<unknown>".to_string())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
@@ -21,7 +21,6 @@ use crate::AppServerEvent;
|
||||
use crate::RequestResult;
|
||||
use crate::SHUTDOWN_TIMEOUT;
|
||||
use crate::TypedRequestError;
|
||||
use crate::request_method_name;
|
||||
use codex_app_server_protocol::ClientInfo;
|
||||
use codex_app_server_protocol::ClientNotification;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
@@ -498,20 +497,22 @@ impl RemoteAppServerClient {
|
||||
where
|
||||
T: DeserializeOwned,
|
||||
{
|
||||
let method = request_method_name(&request);
|
||||
let method = request.method_name();
|
||||
let response =
|
||||
self.request(request)
|
||||
.await
|
||||
.map_err(|source| TypedRequestError::Transport {
|
||||
method: method.clone(),
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})?;
|
||||
let result = response.map_err(|source| TypedRequestError::Server {
|
||||
method: method.clone(),
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})?;
|
||||
serde_json::from_value(result)
|
||||
.map_err(|source| TypedRequestError::Deserialize { method, source })
|
||||
serde_json::from_value(result).map_err(|source| TypedRequestError::Deserialize {
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})
|
||||
}
|
||||
|
||||
pub async fn notify(&self, notification: ClientNotification) -> IoResult<()> {
|
||||
@@ -658,20 +659,22 @@ impl RemoteAppServerRequestHandle {
|
||||
where
|
||||
T: DeserializeOwned,
|
||||
{
|
||||
let method = request_method_name(&request);
|
||||
let method = request.method_name();
|
||||
let response =
|
||||
self.request(request)
|
||||
.await
|
||||
.map_err(|source| TypedRequestError::Transport {
|
||||
method: method.clone(),
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})?;
|
||||
let result = response.map_err(|source| TypedRequestError::Server {
|
||||
method: method.clone(),
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})?;
|
||||
serde_json::from_value(result)
|
||||
.map_err(|source| TypedRequestError::Deserialize { method, source })
|
||||
serde_json::from_value(result).map_err(|source| TypedRequestError::Deserialize {
|
||||
method: method.to_string(),
|
||||
source,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -239,10 +239,6 @@ macro_rules! client_request_definitions {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn method(&self) -> String {
|
||||
self.method_name().to_string()
|
||||
}
|
||||
|
||||
pub fn serialization_scope(&self) -> Option<ClientRequestSerializationScope> {
|
||||
match self {
|
||||
$(
|
||||
@@ -2671,7 +2667,6 @@ mod tests {
|
||||
params: None,
|
||||
};
|
||||
assert_eq!(request.id(), &RequestId::Integer(1));
|
||||
assert_eq!(request.method(), "account/rateLimits/read");
|
||||
assert_eq!(
|
||||
json!({
|
||||
"method": "account/rateLimits/read",
|
||||
@@ -2689,7 +2684,6 @@ mod tests {
|
||||
params: None,
|
||||
};
|
||||
assert_eq!(request.id(), &RequestId::Integer(1));
|
||||
assert_eq!(request.method(), "account/usage/read");
|
||||
assert_eq!(
|
||||
json!({
|
||||
"method": "account/usage/read",
|
||||
@@ -2707,7 +2701,6 @@ mod tests {
|
||||
params: None,
|
||||
};
|
||||
assert_eq!(request.id(), &RequestId::Integer(1));
|
||||
assert_eq!(request.method(), "account/workspaceMessages/read");
|
||||
assert_eq!(
|
||||
json!({
|
||||
"method": "account/workspaceMessages/read",
|
||||
|
||||
Reference in New Issue
Block a user