From 63ff99795c1294dfdb71e5f64fffee3d08744b9d Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 26 May 2026 12:00:34 +0200 Subject: [PATCH] agent: Make messages vec cheap to clone (#57712) For long threads we will spend more and more time cloning the messages just to save them to the database, as we need a copy of everything to do so asynchronously. Messages are really expensive to clone though and we accumulate a lot of them really fast, so even for smaller threads we start seeing pauses in the millisecond range. The fix to this is fairly simple though, we never mutate the messages once pushed to the vec, so just Arc them. This PR also slightly changes `UserMessage` to be a bit faster to clone as well. Release Notes: - Fixed a cause of stutters when interacting with the agent --- crates/acp_thread/src/connection.rs | 4 +- crates/agent/src/agent.rs | 4 +- crates/agent/src/db.rs | 8 +- crates/agent/src/tests/mod.rs | 4 +- crates/agent/src/thread.rs | 79 ++++++++++--------- crates/language_model_core/src/request.rs | 2 +- .../src/provider/copilot_chat.rs | 6 +- .../src/provider/open_router.rs | 2 +- crates/open_ai/src/completion.rs | 24 +++--- crates/open_router/src/open_router.rs | 2 +- 10 files changed, 69 insertions(+), 66 deletions(-) diff --git a/crates/acp_thread/src/connection.rs b/crates/acp_thread/src/connection.rs index bbda24e377..7085d45746 100644 --- a/crates/acp_thread/src/connection.rs +++ b/crates/acp_thread/src/connection.rs @@ -7,14 +7,14 @@ use gpui::{Entity, SharedString, Task}; use language_model::LanguageModelProviderId; use project::{AgentId, Project}; use serde::{Deserialize, Serialize}; -use std::{any::Any, error::Error, fmt, path::PathBuf, rc::Rc, sync::Arc}; +use std::{any::Any, error::Error, fmt, path::PathBuf, rc::Rc}; use task::{HideStrategy, SpawnInTerminal, TaskId}; use ui::{App, IconName}; use util::path_list::PathList; use uuid::Uuid; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Hash)] -pub struct UserMessageId(Arc); +pub struct UserMessageId(SharedString); impl UserMessageId { pub fn new() -> Self { diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index 8ef06de164..df3ae91682 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -1844,10 +1844,10 @@ impl NativeAgentConnection { match event { ThreadEvent::UserMessage(message) => { acp_thread.update(cx, |thread, cx| { - for content in message.content { + for content in &*message.content { thread.push_user_content_block( Some(message.id.clone()), - content.into(), + content.clone().into(), cx, ); } diff --git a/crates/agent/src/db.rs b/crates/agent/src/db.rs index a34290742a..1b2f7639cd 100644 --- a/crates/agent/src/db.rs +++ b/crates/agent/src/db.rs @@ -53,7 +53,7 @@ impl From<&DbThreadMetadata> for acp_thread::AgentSessionInfo { #[derive(Debug, Serialize, Deserialize)] pub struct DbThread { pub title: SharedString, - pub messages: Vec, + pub messages: Vec>, pub updated_at: DateTime, #[serde(default)] pub detailed_summary: Option, @@ -92,7 +92,7 @@ pub struct SerializedScrollPosition { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SharedThread { pub title: SharedString, - pub messages: Vec, + pub messages: Vec>, pub updated_at: DateTime, #[serde(default)] pub model: Option, @@ -206,7 +206,7 @@ impl DbThread { crate::Message::User(UserMessage { // MessageId from old format can't be meaningfully converted, so generate a new one id, - content, + content: Arc::from(content), }) } language_model::Role::Assistant => { @@ -285,7 +285,7 @@ impl DbThread { } }; - messages.push(message); + messages.push(Arc::new(message)); } Ok(Self { diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index eab1c031f0..84e0db98ea 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -4133,8 +4133,8 @@ async fn test_send_retry_finishes_tool_calls_on_error(cx: &mut TestAppContext) { events.collect::>().await; thread.read_with(cx, |thread, _cx| { assert_eq!( - thread.last_received_or_pending_message(), - Some(Message::Agent(AgentMessage { + thread.last_received_or_pending_message().as_deref(), + Some(&Message::Agent(AgentMessage { content: vec![AgentMessageContent::Text("Done".into())], tool_results: IndexMap::default(), reasoning_details: None, diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 147266b678..458f3c1d2a 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -123,7 +123,7 @@ enum RetryStrategy { }, } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] pub enum Message { User(UserMessage), Agent(AgentMessage), @@ -176,13 +176,16 @@ impl Message { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct UserMessage { pub id: UserMessageId, - pub content: Vec, + pub content: Arc<[UserMessageContent]>, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum UserMessageContent { Text(String), - Mention { uri: MentionUri, content: String }, + Mention { + uri: MentionUri, + content: SharedString, + }, Image(LanguageModelImage), } @@ -190,7 +193,7 @@ impl UserMessage { pub fn to_markdown(&self) -> String { let mut markdown = String::new(); - for content in &self.content { + for content in &*self.content { match content { UserMessageContent::Text(text) => { markdown.push_str(text); @@ -250,7 +253,7 @@ impl UserMessage { let mut merge_conflict_context = MERGE_CONFLICT_TAG.to_string(); let mut skills_context = OPEN_SKILLS_TAG.to_string(); - for chunk in &self.content { + for chunk in &*self.content { let chunk = match chunk { UserMessageContent::Text(text) => { language_model::MessageContent::Text(text.clone()) @@ -266,7 +269,7 @@ impl UserMessage { "\n{}", MarkdownCodeBlock { tag: &codeblock_tag(abs_path, None), - text: &content.to_string(), + text: content, } ) .ok(); @@ -627,9 +630,9 @@ impl AgentMessage { #[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct AgentMessage { - pub content: Vec, - pub tool_results: IndexMap, - pub reasoning_details: Option, + pub(crate) content: Vec, + pub(crate) tool_results: IndexMap, + pub(crate) reasoning_details: Option>, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -963,7 +966,7 @@ pub struct Thread { title_generation_failed: bool, pending_summary_generation: Option>>>, summary: Option, - messages: Vec, + messages: Vec>, user_store: Entity, /// Holds the task that handles agent interaction until the end of the turn. /// Survives across multiple requests as the model performs tool calls and @@ -1183,7 +1186,7 @@ impl Thread { let (tx, rx) = mpsc::unbounded(); let stream = ThreadEventStream(tx); for message in &self.messages { - match message { + match &**message { Message::User(user_message) => stream.send_user_message(user_message), Message::Agent(assistant_message) => { for content in &assistant_message.content { @@ -1639,13 +1642,13 @@ impl Thread { } pub fn last_message(&self) -> Option<&Message> { - self.messages.last() + self.messages.last().map(std::ops::Deref::deref) } #[cfg(any(test, feature = "test-support"))] - pub fn last_received_or_pending_message(&self) -> Option { + pub fn last_received_or_pending_message(&self) -> Option> { if let Some(message) = self.pending_message.clone() { - Some(Message::Agent(message)) + Some(Arc::new(Message::Agent(message))) } else { self.messages.last().cloned() } @@ -1802,13 +1805,13 @@ impl Thread { // and we don't want that content to be added after we truncate self.pending_message.take(); let Some(position) = self.messages.iter().position( - |msg| matches!(msg, Message::User(UserMessage { id, .. }) if id == &message_id), + |msg| matches!(&**msg, Message::User(UserMessage { id, .. }) if id == &message_id), ) else { return Err(anyhow!("Message not found")); }; for message in self.messages.drain(position..) { - match message { + match &*message { Message::User(message) => { self.request_token_usage.remove(&message.id); } @@ -1850,7 +1853,7 @@ impl Thread { let mut previous_user_message_id: Option<&UserMessageId> = None; for message in &self.messages { - if let Message::User(user_msg) = message { + if let Message::User(user_msg) = &**message { if &user_msg.id == target_id { let prev_id = previous_user_message_id?; let usage = self.request_token_usage.get(prev_id)?; @@ -1895,7 +1898,7 @@ impl Thread { &mut self, cx: &mut Context, ) -> Result>> { - self.messages.push(Message::Resume); + self.messages.push(Arc::new(Message::Resume)); cx.notify(); log::debug!("Total messages in thread: {}", self.messages.len()); @@ -1914,11 +1917,11 @@ impl Thread { where T: Into, { - let content = content.into_iter().map(Into::into).collect::>(); + let content = content.into_iter().map(Into::into).collect::>(); log::debug!("Thread::send content: {:?}", content); self.messages - .push(Message::User(UserMessage { id, content })); + .push(Arc::new(Message::User(UserMessage { id, content }))); cx.notify(); self.send_existing(cx) @@ -1949,9 +1952,9 @@ impl Thread { let content = blocks .into_iter() .map(|block| UserMessageContent::from_content_block(block, path_style)) - .collect::>(); + .collect::>(); self.messages - .push(Message::User(UserMessage { id, content })); + .push(Arc::new(Message::User(UserMessage { id, content }))); cx.notify(); } @@ -1969,10 +1972,10 @@ impl Thread { _ => "[unknown]".to_string(), }; - self.messages.push(Message::Agent(AgentMessage { + self.messages.push(Arc::new(Message::Agent(AgentMessage { content: vec![AgentMessageContent::Text(text)], ..Default::default() - })); + }))); cx.notify(); } @@ -2230,10 +2233,10 @@ impl Thread { } } this.update(cx, |this, _cx| { - if let Some(Message::Agent(message)) = this.messages.last() { + if let Some(Message::Agent(message)) = this.last_message() { if message.tool_results.is_empty() { intent = CompletionIntent::UserPrompt; - this.messages.push(Message::Resume); + this.messages.push(Arc::new(Message::Resume)); } } })?; @@ -2356,12 +2359,12 @@ impl Thread { let last_message = self.pending_message(); // Store the last non-empty reasoning_details (overwrites earlier ones) // This ensures we keep the encrypted reasoning with signatures, not the early text reasoning - if let serde_json::Value::Array(ref arr) = details { + if let serde_json::Value::Array(arr) = &details { if !arr.is_empty() { - last_message.reasoning_details = Some(details); + last_message.reasoning_details = Some(Arc::new(details)); } } else { - last_message.reasoning_details = Some(details); + last_message.reasoning_details = Some(Arc::new(details)); } } ToolUse(tool_use) => { @@ -2895,7 +2898,7 @@ impl Thread { self.messages .iter() .rev() - .find_map(|message| match message { + .find_map(|message| match &**message { Message::User(user_message) => Some(user_message), Message::Agent(_) => None, Message::Resume => None, @@ -2936,7 +2939,7 @@ impl Thread { } } - self.messages.push(Message::Agent(message)); + self.messages.push(Arc::new(Message::Agent(message))); self.updated_at = Utc::now(); self.clear_summary(); cx.notify() @@ -3199,7 +3202,7 @@ impl Thread { if ix > 0 { markdown.push('\n'); } - match message { + match &**message { Message::User(_) => markdown.push_str("## User\n\n"), Message::Agent(_) => markdown.push_str("## Assistant\n\n"), Message::Resume => {} @@ -4411,7 +4414,7 @@ impl UserMessageContent { match MentionUri::parse(&resource_link.uri, path_style) { Ok(uri) => Self::Mention { uri, - content: String::new(), + content: SharedString::default(), }, Err(err) => { log::error!("Failed to parse mention link: {}", err); @@ -4424,7 +4427,7 @@ impl UserMessageContent { match MentionUri::parse(&resource.uri, path_style) { Ok(uri) => Self::Mention { uri, - content: resource.text, + content: resource.text.into(), }, Err(err) => { log::error!("Failed to parse mention link: {}", err); @@ -4626,14 +4629,14 @@ mod tests { }, ); - thread.messages.push(Message::Agent(AgentMessage { + thread.messages.push(Arc::new(Message::Agent(AgentMessage { content: vec![ AgentMessageContent::ToolUse(registered_tool_use), AgentMessageContent::ToolUse(missing_tool_use), ], tool_results, reasoning_details: None, - })); + }))); thread.replay(cx) }) @@ -4784,11 +4787,11 @@ mod tests { }, ); - thread.messages.push(Message::Agent(AgentMessage { + thread.messages.push(Arc::new(Message::Agent(AgentMessage { content: vec![AgentMessageContent::ToolUse(tool_use)], tool_results, reasoning_details: None, - })); + }))); } #[gpui::test] diff --git a/crates/language_model_core/src/request.rs b/crates/language_model_core/src/request.rs index 19a642ba01..867d087ab3 100644 --- a/crates/language_model_core/src/request.rs +++ b/crates/language_model_core/src/request.rs @@ -293,7 +293,7 @@ pub struct LanguageModelRequestMessage { pub content: Vec, pub cache: bool, #[serde(default, skip_serializing_if = "Option::is_none")] - pub reasoning_details: Option, + pub reasoning_details: Option>, } impl LanguageModelRequestMessage { diff --git a/crates/language_models/src/provider/copilot_chat.rs b/crates/language_models/src/provider/copilot_chat.rs index d9db887e12..bfac215b6c 100644 --- a/crates/language_models/src/provider/copilot_chat.rs +++ b/crates/language_models/src/provider/copilot_chat.rs @@ -1299,7 +1299,7 @@ fn into_copilot_responses( Role::Assistant => { append_reasoning_details_to_response_items( - message.reasoning_details.as_ref(), + message.reasoning_details.as_deref(), &mut replayed_reasoning_item_indexes, &mut input_items, ); @@ -1608,7 +1608,7 @@ mod tests { MessageContent::Text("Done".into()), ], cache: false, - reasoning_details: Some(json!({ + reasoning_details: Some(Arc::new(json!({ "reasoning_items": [ { "id": "r1", @@ -1621,7 +1621,7 @@ mod tests { "encrypted_content": "ENC" } ] - })), + }))), }], ..Default::default() }; diff --git a/crates/language_models/src/provider/open_router.rs b/crates/language_models/src/provider/open_router.rs index c0b0e33062..d9632c402f 100644 --- a/crates/language_models/src/provider/open_router.rs +++ b/crates/language_models/src/provider/open_router.rs @@ -540,7 +540,7 @@ fn add_message_content_part( new_part: open_router::MessagePart, role: Role, messages: &mut Vec, - reasoning_details: Option, + reasoning_details: Option>, ) { match (role, messages.last_mut()) { (Role::User, Some(open_router::RequestMessage::User { content })) diff --git a/crates/open_ai/src/completion.rs b/crates/open_ai/src/completion.rs index 6e16f0961f..4f1a671aa8 100644 --- a/crates/open_ai/src/completion.rs +++ b/crates/open_ai/src/completion.rs @@ -302,14 +302,14 @@ fn append_message_to_response_items( .. } = message; let phase = if role == Role::Assistant { - response_message_phase_from_details(reasoning_details.as_ref()) + response_message_phase_from_details(reasoning_details.as_deref()) } else { None }; if role == Role::Assistant { append_reasoning_details_to_response_items( - reasoning_details.as_ref(), + reasoning_details.as_deref(), replayed_reasoning_item_indexes, input_items, ); @@ -1483,7 +1483,7 @@ mod tests { role: Role::Assistant, content: vec![MessageContent::ToolUse(tool_use)], cache: false, - reasoning_details: Some(json!({ + reasoning_details: Some(Arc::new(json!({ "reasoning_items": [ { "id": "rs_123", @@ -1503,7 +1503,7 @@ mod tests { "status": "completed", } ] - })), + }))), }], tools: Vec::new(), tool_choice: None, @@ -1571,7 +1571,7 @@ mod tests { role: Role::Assistant, content: vec![MessageContent::Text("Done.".into())], cache: false, - reasoning_details: Some(json!({ + reasoning_details: Some(Arc::new(json!({ "reasoning_items": [ { "id": "rs_123", @@ -1585,7 +1585,7 @@ mod tests { "status": "completed" } ] - })), + }))), }], tools: Vec::new(), tool_choice: None, @@ -1754,7 +1754,7 @@ mod tests { role: Role::Assistant, content: vec![MessageContent::Text("Done.".into())], cache: false, - reasoning_details: Some(json!({ + reasoning_details: Some(Arc::new(json!({ "phase": "final_answer", "reasoning_items": [ { @@ -1764,7 +1764,7 @@ mod tests { "status": "completed" } ] - })), + }))), }], tools: Vec::new(), tool_choice: None, @@ -1846,13 +1846,13 @@ mod tests { role: Role::Assistant, content: vec![MessageContent::Text("First.".into())], cache: false, - reasoning_details: Some(first_reasoning_details), + reasoning_details: Some(Arc::new(first_reasoning_details)), }, LanguageModelRequestMessage { role: Role::Assistant, content: vec![MessageContent::Text("Second.".into())], cache: false, - reasoning_details: Some(second_reasoning_details), + reasoning_details: Some(Arc::new(second_reasoning_details)), }, ], tools: Vec::new(), @@ -1926,7 +1926,7 @@ mod tests { MessageContent::Text("This is visible assistant output.".into()), ], cache: false, - reasoning_details: Some(json!({ + reasoning_details: Some(Arc::new(json!({ "reasoning_items": [ { "id": "rs_123", @@ -1940,7 +1940,7 @@ mod tests { "status": "completed" } ] - })), + }))), }], tools: Vec::new(), tool_choice: None, diff --git a/crates/open_router/src/open_router.rs b/crates/open_router/src/open_router.rs index b94631f9a0..306f2c22cb 100644 --- a/crates/open_router/src/open_router.rs +++ b/crates/open_router/src/open_router.rs @@ -212,7 +212,7 @@ pub enum RequestMessage { #[serde(default, skip_serializing_if = "Vec::is_empty")] tool_calls: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] - reasoning_details: Option, + reasoning_details: Option>, }, User { content: MessageContent,