Remove wasted clones of loop-invariant values (#62763)
# Objective Remove avoidable clones of loop-invariant values that pay a copy on every loop iteration. These were found by running a prototype `deep_clone_in_loop` dylint (modeled on the accidentally quadratic project-search fix in #62658) across the workspace. None of these sites are quadratic in practice — the loops are bounded — but each one does strictly wasted work that a one-line change removes. ## Solution - `collab/src/api/extensions.rs`: use `next_marker.take()` instead of `next_marker.clone()` for the S3 pagination marker. The marker is either overwritten via `clone_from` at the end of the iteration or the loop breaks, so it is never read after the `take`. - `project/src/manifest_tree/path_trie.rs`: build the trie key with `Arc::from(path_so_far.as_rel_path())` instead of `path_so_far.clone().into()`. The `From<RelPathBuf> for Arc<RelPath>` impl copies the bytes into the `Arc` anyway, so the intermediate `RelPathBuf` clone was a second, redundant copy. - `context_server/src/client.rs`: in `notify`, split off the last handler and move the `serde_json::Value` payload into it, cloning only for the preceding handlers. Previously every handler received a clone and the final one was always wasted. - `editor/src/rewrap.rs`: use `std::mem::replace` to move the comment-delimiter and rewrap-prefix accumulators into the pushed range instead of cloning them and immediately reassigning. ## Testing - `cargo check -p collab -p project -p context_server -p editor` — clean. - `cargo test -p editor rewrap` — 7/7 pass, including the boundary-sensitive `test_rewrap*` editor tests. - `cargo test -p context_server` — 97/97 pass. - `cargo test -p project path_trie` — 2/2 pass. - Re-ran the `deep_clone_in_loop` lint on the four crates: the `extensions.rs`, `path_trie.rs`, and `rewrap.rs` hits are gone; `client.rs` still (correctly) reports the remaining required per-handler clone. - Not tested platform-specifically; the changes are platform-independent. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - N/A
This commit is contained in:
1 parent
1a42a2b714
commit
22a582a86a
4 files changed
+17
-11
No files matched your search
@@ -217,7 +217,7 @@ async fn fetch_extensions_from_blob_store(
|
||||
.list_objects()
|
||||
.bucket(blob_store_bucket)
|
||||
.prefix("extensions/")
|
||||
.set_marker(next_marker.clone())
|
||||
.set_marker(next_marker.take())
|
||||
.send()
|
||||
.await?;
|
||||
let objects = list.contents.unwrap_or_default();
|
||||
|
||||
@@ -592,9 +592,14 @@ impl NotificationSubscriptionSet {
|
||||
return;
|
||||
};
|
||||
|
||||
for handler_id in handler_ids {
|
||||
if let Some(handler) = self.handlers.get_mut(*handler_id) {
|
||||
handler(payload.clone(), cx.clone());
|
||||
if let Some((last_handler_id, handler_ids)) = handler_ids.split_last() {
|
||||
for handler_id in handler_ids {
|
||||
if let Some(handler) = self.handlers.get_mut(*handler_id) {
|
||||
handler(payload.clone(), cx.clone());
|
||||
}
|
||||
}
|
||||
if let Some(handler) = self.handlers.get_mut(*last_handler_id) {
|
||||
handler(payload, cx.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -191,13 +191,14 @@ impl Editor {
|
||||
Point::new(current_range_start, 0)
|
||||
..Point::new(prev_row, buffer.line_len(MultiBufferRow(prev_row))),
|
||||
current_range_indent,
|
||||
current_range_comment_delimiters.clone(),
|
||||
current_range_rewrap_prefix.clone(),
|
||||
std::mem::replace(
|
||||
&mut current_range_comment_delimiters,
|
||||
row_comment_delimiters,
|
||||
),
|
||||
std::mem::replace(&mut current_range_rewrap_prefix, row_rewrap_prefix),
|
||||
));
|
||||
current_range_start = row;
|
||||
current_range_indent = row_indent;
|
||||
current_range_comment_delimiters = row_comment_delimiters;
|
||||
current_range_rewrap_prefix = row_rewrap_prefix;
|
||||
}
|
||||
prev_row = row;
|
||||
}
|
||||
|
||||
@@ -64,9 +64,9 @@ impl<Label: Ord + Clone> RootPathTrie<Label> {
|
||||
for key in path.0.iter() {
|
||||
path_so_far = path_so_far.join(RelPath::from_unix_str(key.as_ref()).unwrap());
|
||||
current = match current.children.entry(key.clone()) {
|
||||
Entry::Vacant(vacant_entry) => {
|
||||
vacant_entry.insert(RootPathTrie::new_with_key(path_so_far.clone().into()))
|
||||
}
|
||||
Entry::Vacant(vacant_entry) => vacant_entry.insert(RootPathTrie::new_with_key(
|
||||
Arc::from(path_so_far.as_rel_path()),
|
||||
)),
|
||||
Entry::Occupied(occupied_entry) => occupied_entry.into_mut(),
|
||||
};
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user