assistant_tools: Reduce allocations (#30776)
Another batch of allocation savings. Noteworthy ones are `find_path_tool.rs` where one clone of *all* found matches was saved and `web_tool_search.rs` where the tooltip no longer clones the entire url on every hover. I'd also like to propose using `std::borrow::Cow` a lot more around the codebase instead of Strings. There are hundreds if not 1000+ clones that can be saved pretty regularly simply by switching to Cow. ´Cow´'s are likely not used because they aren't compatible with futures and because it could cause lifetime bloat. However if we use `Cow<'static, str>` (static lifetime) for when we need to pass them into futures, we could save a TON of allocations for `&'static str`. Additionally I often see structs being created using `String`'s just to be deserialized afterwards, which only requires a reference. Release Notes: - N/A
This commit is contained in:
parent
047e7eacec
commit
8bec4cbecb
4 changed files with 18 additions and 19 deletions
|
@ -1,6 +1,6 @@
|
||||||
use std::cell::RefCell;
|
|
||||||
use std::rc::Rc;
|
use std::rc::Rc;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
use std::{borrow::Cow, cell::RefCell};
|
||||||
|
|
||||||
use crate::schema::json_schema_for;
|
use crate::schema::json_schema_for;
|
||||||
use anyhow::{Context as _, Result, anyhow, bail};
|
use anyhow::{Context as _, Result, anyhow, bail};
|
||||||
|
@ -39,10 +39,11 @@ impl FetchTool {
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn build_message(http_client: Arc<HttpClientWithUrl>, url: &str) -> Result<String> {
|
async fn build_message(http_client: Arc<HttpClientWithUrl>, url: &str) -> Result<String> {
|
||||||
let mut url = url.to_owned();
|
let url = if !url.starts_with("https://") && !url.starts_with("http://") {
|
||||||
if !url.starts_with("https://") && !url.starts_with("http://") {
|
Cow::Owned(format!("https://{url}"))
|
||||||
url = format!("https://{url}");
|
} else {
|
||||||
}
|
Cow::Borrowed(url)
|
||||||
|
};
|
||||||
|
|
||||||
let mut response = http_client.get(&url, AsyncBody::default(), true).await?;
|
let mut response = http_client.get(&url, AsyncBody::default(), true).await?;
|
||||||
|
|
||||||
|
@ -156,8 +157,7 @@ impl Tool for FetchTool {
|
||||||
|
|
||||||
let text = cx.background_spawn({
|
let text = cx.background_spawn({
|
||||||
let http_client = self.http_client.clone();
|
let http_client = self.http_client.clone();
|
||||||
let url = input.url.clone();
|
async move { Self::build_message(http_client, &input.url).await }
|
||||||
async move { Self::build_message(http_client, &url).await }
|
|
||||||
});
|
});
|
||||||
|
|
||||||
cx.foreground_executor()
|
cx.foreground_executor()
|
||||||
|
|
|
@ -119,14 +119,16 @@ impl Tool for FindPathTool {
|
||||||
)
|
)
|
||||||
.unwrap();
|
.unwrap();
|
||||||
}
|
}
|
||||||
let output = FindPathToolOutput {
|
|
||||||
glob,
|
|
||||||
paths: matches.clone(),
|
|
||||||
};
|
|
||||||
|
|
||||||
for mat in matches.into_iter().skip(offset).take(RESULTS_PER_PAGE) {
|
for mat in matches.iter().skip(offset).take(RESULTS_PER_PAGE) {
|
||||||
write!(&mut message, "\n{}", mat.display()).unwrap();
|
write!(&mut message, "\n{}", mat.display()).unwrap();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let output = FindPathToolOutput {
|
||||||
|
glob,
|
||||||
|
paths: matches,
|
||||||
|
};
|
||||||
|
|
||||||
Ok(ToolResultOutput {
|
Ok(ToolResultOutput {
|
||||||
content: ToolResultContent::Text(message),
|
content: ToolResultContent::Text(message),
|
||||||
output: Some(serde_json::to_value(output)?),
|
output: Some(serde_json::to_value(output)?),
|
||||||
|
@ -235,8 +237,6 @@ impl ToolCard for FindPathToolCard {
|
||||||
format!("{} matches", self.paths.len()).into()
|
format!("{} matches", self.paths.len()).into()
|
||||||
};
|
};
|
||||||
|
|
||||||
let glob_label = self.glob.to_string();
|
|
||||||
|
|
||||||
let content = if !self.paths.is_empty() && self.expanded {
|
let content = if !self.paths.is_empty() && self.expanded {
|
||||||
Some(
|
Some(
|
||||||
v_flex()
|
v_flex()
|
||||||
|
@ -310,7 +310,7 @@ impl ToolCard for FindPathToolCard {
|
||||||
.gap_1()
|
.gap_1()
|
||||||
.child(
|
.child(
|
||||||
ToolCallCardHeader::new(IconName::SearchCode, matches_label)
|
ToolCallCardHeader::new(IconName::SearchCode, matches_label)
|
||||||
.with_code_path(glob_label)
|
.with_code_path(&self.glob)
|
||||||
.disclosure_slot(
|
.disclosure_slot(
|
||||||
Disclosure::new("path-search-disclosure", self.expanded)
|
Disclosure::new("path-search-disclosure", self.expanded)
|
||||||
.opened_icon(IconName::ChevronUp)
|
.opened_icon(IconName::ChevronUp)
|
||||||
|
|
|
@ -182,9 +182,8 @@ impl Tool for TerminalTool {
|
||||||
let mut child = pair.slave.spawn_command(cmd)?;
|
let mut child = pair.slave.spawn_command(cmd)?;
|
||||||
let mut reader = pair.master.try_clone_reader()?;
|
let mut reader = pair.master.try_clone_reader()?;
|
||||||
drop(pair);
|
drop(pair);
|
||||||
let mut content = Vec::new();
|
let mut content = String::new();
|
||||||
reader.read_to_end(&mut content)?;
|
reader.read_to_string(&mut content)?;
|
||||||
let mut content = String::from_utf8(content)?;
|
|
||||||
// Massage the pty output a bit to try to match what the terminal codepath gives us
|
// Massage the pty output a bit to try to match what the terminal codepath gives us
|
||||||
LineEnding::normalize(&mut content);
|
LineEnding::normalize(&mut content);
|
||||||
content = content
|
content = content
|
||||||
|
|
|
@ -166,7 +166,7 @@ impl ToolCard for WebSearchToolCard {
|
||||||
.gap_1()
|
.gap_1()
|
||||||
.children(response.results.iter().enumerate().map(|(index, result)| {
|
.children(response.results.iter().enumerate().map(|(index, result)| {
|
||||||
let title = result.title.clone();
|
let title = result.title.clone();
|
||||||
let url = result.url.clone();
|
let url = SharedString::from(result.url.clone());
|
||||||
|
|
||||||
Button::new(("result", index), title)
|
Button::new(("result", index), title)
|
||||||
.label_size(LabelSize::Small)
|
.label_size(LabelSize::Small)
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue