mirror of
https://github.com/zed-industries/zed.git
synced 2026-10-19 11:04:06 +00:00
git_ui: Batch merge conflict anchor resolution (#63394)
Improves merge-conflict loading by resolving ordered conflict anchors with a single excerpt sweep instead of restarting the excerpt scan four times per conflict. The pull request also adds `script/generate-merge-conflict-benchmark`, which creates validated small, medium, or severe repositories with real unresolved Git merges. On the severe workload (128 files × 256 conflicts), anchor conversion decreased from 483.5 ms to 5.9 ms, saving about 478 ms, or roughly 5% of the total refresh. Closes https://linear.app/zed-industries/issue/FR-174/improve-git-panel-performance-for-large-merge-conflicts ## Testing - `cargo test -p multi_buffer test_ordered_buffer_anchor_ranges_to_anchor_ranges` - `cargo test -p git_ui conflict` - `./script/clippy -p multi_buffer -p git_ui` - Generated and validated the small synthetic merge-conflict workload Release Notes: - Improved performance when opening large merge conflicts
This commit is contained in:
parent
e8fbacb6d7
commit
ff020dd053
4 changed files with 404 additions and 24 deletions
|
|
@ -237,25 +237,39 @@ fn conflicts_updated(
|
|||
// Add new highlights and blocks
|
||||
let editor_handle = cx.weak_entity();
|
||||
let new_conflicts = &conflict_set.conflicts[event.new_range.clone()];
|
||||
let mut converted_ranges = snapshot
|
||||
.ordered_buffer_anchor_ranges_to_anchor_ranges(new_conflicts.iter().flat_map(|conflict| {
|
||||
[
|
||||
conflict.range.start..conflict.range.start,
|
||||
conflict.range.clone(),
|
||||
conflict.ours.clone(),
|
||||
conflict.theirs.clone(),
|
||||
]
|
||||
}))
|
||||
.into_iter();
|
||||
let mut blocks = Vec::new();
|
||||
for conflict in new_conflicts {
|
||||
update_conflict_highlighting(editor, conflict, &snapshot, cx);
|
||||
let block_anchor = converted_ranges.next().flatten().map(|range| range.start);
|
||||
let outer = converted_ranges.next().flatten();
|
||||
let ours = converted_ranges.next().flatten();
|
||||
let theirs = converted_ranges.next().flatten();
|
||||
if let (Some(outer), Some(ours), Some(theirs)) = (outer, ours, theirs) {
|
||||
update_conflict_highlighting(editor, conflict, outer, ours, theirs, cx);
|
||||
}
|
||||
|
||||
let Some(anchor) = snapshot.anchor_in_excerpt(conflict.range.start) else {
|
||||
continue;
|
||||
};
|
||||
|
||||
let editor_handle = editor_handle.clone();
|
||||
blocks.push(BlockProperties {
|
||||
placement: BlockPlacement::Above(anchor),
|
||||
height: Some(1),
|
||||
style: BlockStyle::Sticky,
|
||||
render: Arc::new({
|
||||
let conflict = conflict.clone();
|
||||
move |cx| render_conflict_buttons(&conflict, editor_handle.clone(), cx)
|
||||
}),
|
||||
priority: 0,
|
||||
})
|
||||
if let Some(anchor) = block_anchor {
|
||||
let editor_handle = editor_handle.clone();
|
||||
blocks.push(BlockProperties {
|
||||
placement: BlockPlacement::Above(anchor),
|
||||
height: Some(1),
|
||||
style: BlockStyle::Sticky,
|
||||
render: Arc::new({
|
||||
let conflict = conflict.clone();
|
||||
move |cx| render_conflict_buttons(&conflict, editor_handle.clone(), cx)
|
||||
}),
|
||||
priority: 0,
|
||||
})
|
||||
}
|
||||
}
|
||||
let new_block_ids = editor.insert_blocks(blocks, None, cx);
|
||||
|
||||
|
|
@ -279,15 +293,13 @@ fn conflicts_updated(
|
|||
fn update_conflict_highlighting(
|
||||
editor: &mut Editor,
|
||||
conflict: &ConflictRegion,
|
||||
buffer: &editor::MultiBufferSnapshot,
|
||||
outer: Range<editor::Anchor>,
|
||||
ours: Range<editor::Anchor>,
|
||||
theirs: Range<editor::Anchor>,
|
||||
cx: &mut Context<Editor>,
|
||||
) -> Option<()> {
|
||||
) {
|
||||
log::debug!("update conflict highlighting for {conflict:?}");
|
||||
|
||||
let outer = buffer.buffer_anchor_range_to_anchor_range(conflict.range.clone())?;
|
||||
let ours = buffer.buffer_anchor_range_to_anchor_range(conflict.ours.clone())?;
|
||||
let theirs = buffer.buffer_anchor_range_to_anchor_range(conflict.theirs.clone())?;
|
||||
|
||||
let ours_background = |cx: &App| cx.theme().colors().version_control_conflict_marker_ours;
|
||||
let theirs_background = |cx: &App| cx.theme().colors().version_control_conflict_marker_theirs;
|
||||
|
||||
|
|
@ -318,8 +330,6 @@ fn update_conflict_highlighting(
|
|||
options,
|
||||
cx,
|
||||
);
|
||||
|
||||
Some(())
|
||||
}
|
||||
|
||||
fn render_conflict_buttons(
|
||||
|
|
|
|||
|
|
@ -5417,6 +5417,97 @@ impl MultiBufferSnapshot {
|
|||
None
|
||||
}
|
||||
|
||||
/// Converts ranges from one buffer in start-anchor order without restarting the excerpt scan
|
||||
/// for every range. Unordered or mixed-buffer input falls back to independent conversions.
|
||||
pub fn ordered_buffer_anchor_ranges_to_anchor_ranges(
|
||||
&self,
|
||||
text_anchor_ranges: impl IntoIterator<Item = Range<text::Anchor>>,
|
||||
) -> Vec<Option<Range<Anchor>>> {
|
||||
let text_anchor_ranges = text_anchor_ranges.into_iter().collect::<Vec<_>>();
|
||||
let Some(first_range) = text_anchor_ranges.first() else {
|
||||
return Vec::new();
|
||||
};
|
||||
let buffer_id = first_range.start.buffer_id;
|
||||
let ranges_share_buffer = text_anchor_ranges
|
||||
.iter()
|
||||
.all(|range| range.start.buffer_id == buffer_id && range.end.buffer_id == buffer_id);
|
||||
if !ranges_share_buffer {
|
||||
return text_anchor_ranges
|
||||
.into_iter()
|
||||
.map(|range| self.buffer_anchor_range_to_anchor_range(range))
|
||||
.collect();
|
||||
}
|
||||
|
||||
let Some(buffer_state) = self.buffers.get(&buffer_id) else {
|
||||
return vec![None; text_anchor_ranges.len()];
|
||||
};
|
||||
let buffer_snapshot = &buffer_state.buffer_snapshot;
|
||||
let ranges_are_ordered = text_anchor_ranges.windows(2).all(|ranges| {
|
||||
ranges[0]
|
||||
.start
|
||||
.cmp(&ranges[1].start, buffer_snapshot)
|
||||
.is_le()
|
||||
});
|
||||
if !ranges_are_ordered {
|
||||
return text_anchor_ranges
|
||||
.into_iter()
|
||||
.map(|range| self.buffer_anchor_range_to_anchor_range(range))
|
||||
.collect();
|
||||
}
|
||||
|
||||
let path_key = buffer_state.path_key.clone();
|
||||
let mut cursor = self.excerpts.cursor::<PathKey>(());
|
||||
cursor.seek_forward(&path_key, Bias::Left);
|
||||
let excerpts = iter::from_fn(move || {
|
||||
let excerpt = cursor.item()?;
|
||||
if excerpt.path_key != path_key {
|
||||
return None;
|
||||
}
|
||||
cursor.next();
|
||||
Some(excerpt)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let mut first_candidate_index = 0;
|
||||
text_anchor_ranges
|
||||
.into_iter()
|
||||
.map(|range| {
|
||||
while let Some(excerpt) = excerpts.get(first_candidate_index) {
|
||||
let buffer_snapshot = excerpt.buffer_snapshot(self);
|
||||
if excerpt
|
||||
.range
|
||||
.context
|
||||
.end
|
||||
.cmp(&range.start, buffer_snapshot)
|
||||
.is_lt()
|
||||
{
|
||||
first_candidate_index += 1;
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
for excerpt in &excerpts[first_candidate_index..] {
|
||||
let buffer_snapshot = excerpt.buffer_snapshot(self);
|
||||
if excerpt
|
||||
.range
|
||||
.context
|
||||
.start
|
||||
.cmp(&range.start, buffer_snapshot)
|
||||
.is_gt()
|
||||
{
|
||||
break;
|
||||
}
|
||||
if excerpt.range.contains(&range.start, buffer_snapshot)
|
||||
&& excerpt.range.contains(&range.end, buffer_snapshot)
|
||||
{
|
||||
return Some(Anchor::range_in_buffer(excerpt.path_key_index, range));
|
||||
}
|
||||
}
|
||||
None
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Returns a buffer anchor and its buffer snapshot for the given anchor, if it is in the multibuffer.
|
||||
pub fn anchor_to_buffer_anchor(
|
||||
&self,
|
||||
|
|
|
|||
|
|
@ -1170,6 +1170,76 @@ fn test_remove_excerpts_for_paths(cx: &mut App) {
|
|||
}
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_ordered_buffer_anchor_ranges_to_anchor_ranges(cx: &mut App) {
|
||||
let buffer = cx.new(|cx| Buffer::local(sample_text(12, 3, 'a'), cx));
|
||||
let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite));
|
||||
multibuffer.update(cx, |multibuffer, cx| {
|
||||
multibuffer.set_excerpts_for_path(
|
||||
PathKey::for_buffer(&buffer, cx),
|
||||
buffer.clone(),
|
||||
[
|
||||
Point::new(1, 0)..Point::new(1, 3),
|
||||
Point::new(5, 0)..Point::new(5, 3),
|
||||
Point::new(9, 0)..Point::new(9, 3),
|
||||
],
|
||||
0,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
let buffer_snapshot = buffer.read(cx).snapshot();
|
||||
let ranges = [
|
||||
buffer_snapshot.anchor_before(Point::new(1, 0))
|
||||
..buffer_snapshot.anchor_before(Point::new(1, 0)),
|
||||
buffer_snapshot.anchor_before(Point::new(1, 1))
|
||||
..buffer_snapshot.anchor_after(Point::new(1, 2)),
|
||||
buffer_snapshot.anchor_before(Point::new(3, 0))
|
||||
..buffer_snapshot.anchor_after(Point::new(3, 2)),
|
||||
buffer_snapshot.anchor_before(Point::new(5, 0))
|
||||
..buffer_snapshot.anchor_after(Point::new(5, 3)),
|
||||
buffer_snapshot.anchor_before(Point::new(9, 1))
|
||||
..buffer_snapshot.anchor_after(Point::new(9, 2)),
|
||||
];
|
||||
let snapshot = multibuffer.read(cx).snapshot(cx);
|
||||
let expected = ranges
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(|range| snapshot.buffer_anchor_range_to_anchor_range(range))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(
|
||||
snapshot.ordered_buffer_anchor_ranges_to_anchor_ranges(ranges.clone()),
|
||||
expected
|
||||
);
|
||||
|
||||
let unordered_ranges = ranges.clone().into_iter().rev().collect::<Vec<_>>();
|
||||
let expected = unordered_ranges
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(|range| snapshot.buffer_anchor_range_to_anchor_range(range))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(
|
||||
snapshot.ordered_buffer_anchor_ranges_to_anchor_ranges(unordered_ranges),
|
||||
expected
|
||||
);
|
||||
|
||||
let other_buffer = cx.new(|cx| Buffer::local("other", cx));
|
||||
let other_snapshot = other_buffer.read(cx).snapshot();
|
||||
let mixed_buffer_ranges = vec![
|
||||
other_snapshot.anchor_before(Point::zero())..other_snapshot.anchor_after(Point::new(0, 5)),
|
||||
ranges[0].clone(),
|
||||
];
|
||||
let expected = mixed_buffer_ranges
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(|range| snapshot.buffer_anchor_range_to_anchor_range(range))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(
|
||||
snapshot.ordered_buffer_anchor_ranges_to_anchor_ranges(mixed_buffer_ranges),
|
||||
expected
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_expand_excerpts(cx: &mut App) {
|
||||
let buffer = cx.new(|cx| Buffer::local(sample_text(20, 3, 'a'), cx));
|
||||
|
|
|
|||
209
script/generate-merge-conflict-benchmark
Executable file
209
script/generate-merge-conflict-benchmark
Executable file
|
|
@ -0,0 +1,209 @@
|
|||
#!/usr/bin/env python3
|
||||
|
||||
import argparse
|
||||
import os
|
||||
from pathlib import Path
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
|
||||
PRESETS = {
|
||||
"small": (2, 8, 16 * 1024),
|
||||
"medium": (32, 64, 128 * 1024),
|
||||
"severe": (128, 256, 512 * 1024),
|
||||
}
|
||||
|
||||
|
||||
def run_git(
|
||||
repository: Path, *arguments: str, expect_failure: bool = False
|
||||
) -> subprocess.CompletedProcess:
|
||||
environment = os.environ.copy()
|
||||
environment.update(
|
||||
{
|
||||
"GIT_CONFIG_GLOBAL": os.devnull,
|
||||
"GIT_CONFIG_SYSTEM": os.devnull,
|
||||
"GIT_AUTHOR_NAME": "Zed Benchmark",
|
||||
"GIT_AUTHOR_EMAIL": "benchmark@zed.dev",
|
||||
"GIT_COMMITTER_NAME": "Zed Benchmark",
|
||||
"GIT_COMMITTER_EMAIL": "benchmark@zed.dev",
|
||||
}
|
||||
)
|
||||
result = subprocess.run(
|
||||
["git", "-C", repository, *arguments],
|
||||
env=environment,
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
check=False,
|
||||
)
|
||||
if expect_failure:
|
||||
if result.returncode == 0:
|
||||
raise RuntimeError(f"git {' '.join(arguments)} unexpectedly succeeded")
|
||||
elif result.returncode != 0:
|
||||
stderr = result.stderr.decode(errors="replace").strip()
|
||||
raise RuntimeError(f"git {' '.join(arguments)} failed: {stderr}")
|
||||
return result
|
||||
|
||||
|
||||
def unchanged_separator(region_index: int) -> str:
|
||||
return "".join(
|
||||
f"// unchanged separator {region_index:04d} {line_index:02d}\n"
|
||||
for line_index in range(10)
|
||||
)
|
||||
|
||||
|
||||
def conflict_file(
|
||||
file_index: int, region_count: int, variant: str, padding: int
|
||||
) -> str:
|
||||
chunks = [
|
||||
f"// Synthetic merge-conflict workload: file {file_index:04d}, variant {variant}\n"
|
||||
]
|
||||
for region_index in range(region_count):
|
||||
value = f"{variant}-file-{file_index:04d}-region-{region_index:04d}-"
|
||||
chunks.append(
|
||||
f'pub const VALUE_{region_index:04d}: &str = "{value}{"x" * padding}";\n'
|
||||
)
|
||||
chunks.append(unchanged_separator(region_index))
|
||||
return "".join(chunks)
|
||||
|
||||
|
||||
def padding_for_target_size(file_index: int, region_count: int, target_size: int) -> int:
|
||||
unpadded_ours = conflict_file(file_index, region_count, "ours", 0)
|
||||
unpadded_theirs = conflict_file(file_index, region_count, "theirs", 0)
|
||||
marker_overhead = region_count * len(
|
||||
"<<<<<<< HEAD\n=======\n>>>>>>> conflict-side\n"
|
||||
)
|
||||
shared_separator_size = sum(
|
||||
len(unchanged_separator(region_index))
|
||||
for region_index in range(region_count)
|
||||
)
|
||||
minimum_size = (
|
||||
len(unpadded_ours)
|
||||
+ len(unpadded_theirs)
|
||||
- shared_separator_size
|
||||
+ marker_overhead
|
||||
)
|
||||
if target_size < minimum_size:
|
||||
raise ValueError(
|
||||
f"target size {target_size} is too small for {region_count} regions; "
|
||||
f"minimum is approximately {minimum_size} bytes"
|
||||
)
|
||||
return (target_size - minimum_size) // (2 * region_count)
|
||||
|
||||
|
||||
def write_variant(
|
||||
repository: Path,
|
||||
file_count: int,
|
||||
region_count: int,
|
||||
target_size: int,
|
||||
variant: str,
|
||||
) -> None:
|
||||
for file_index in range(file_count):
|
||||
path = (
|
||||
repository
|
||||
/ "conflicts"
|
||||
/ f"module_{file_index:04d}"
|
||||
/ f"generated_conflict_{file_index:04d}.rs"
|
||||
)
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
padding = padding_for_target_size(file_index, region_count, target_size)
|
||||
path.write_text(
|
||||
conflict_file(file_index, region_count, variant, padding),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
def validate(repository: Path, file_count: int, region_count: int) -> tuple[int, int]:
|
||||
merge_head = run_git(repository, "rev-parse", "-q", "--verify", "MERGE_HEAD")
|
||||
if not merge_head.stdout.strip():
|
||||
raise RuntimeError("MERGE_HEAD is missing")
|
||||
|
||||
unresolved_output = run_git(
|
||||
repository, "diff", "--name-only", "--diff-filter=U", "-z"
|
||||
).stdout
|
||||
unresolved_paths = [
|
||||
Path(path.decode()) for path in unresolved_output.split(b"\0") if path
|
||||
]
|
||||
if len(unresolved_paths) != file_count:
|
||||
raise RuntimeError(
|
||||
f"expected {file_count} unresolved paths, found {len(unresolved_paths)}"
|
||||
)
|
||||
|
||||
total_bytes = 0
|
||||
for relative_path in unresolved_paths:
|
||||
contents = (repository / relative_path).read_bytes()
|
||||
total_bytes += len(contents)
|
||||
marker_counts = (
|
||||
contents.count(b"<<<<<<< "),
|
||||
contents.count(b"======="),
|
||||
contents.count(b">>>>>>> "),
|
||||
)
|
||||
if marker_counts != (region_count, region_count, region_count):
|
||||
raise RuntimeError(
|
||||
f"{relative_path} has marker counts {marker_counts}, "
|
||||
f"expected {(region_count, region_count, region_count)}"
|
||||
)
|
||||
|
||||
status_entries = [
|
||||
entry
|
||||
for entry in run_git(
|
||||
repository, "status", "--porcelain=v1", "-z"
|
||||
).stdout.split(b"\0")
|
||||
if entry
|
||||
]
|
||||
if len(status_entries) != file_count or any(
|
||||
not entry.startswith(b"UU ") for entry in status_entries
|
||||
):
|
||||
raise RuntimeError("the repository contains unexpected status entries")
|
||||
|
||||
return len(unresolved_paths), total_bytes
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(
|
||||
description="Create a deterministic unresolved Git merge for Zed performance testing."
|
||||
)
|
||||
parser.add_argument("destination", type=Path)
|
||||
parser.add_argument(
|
||||
"--preset", choices=PRESETS, default="medium", help="workload size"
|
||||
)
|
||||
arguments = parser.parse_args()
|
||||
|
||||
file_count, region_count, target_size = PRESETS[arguments.preset]
|
||||
destination = arguments.destination.expanduser().resolve()
|
||||
try:
|
||||
destination.mkdir(parents=True, exist_ok=False)
|
||||
run_git(destination, "init", "-b", "main")
|
||||
|
||||
write_variant(destination, file_count, region_count, target_size, "base")
|
||||
run_git(destination, "add", "conflicts")
|
||||
run_git(destination, "commit", "-m", "base")
|
||||
|
||||
run_git(destination, "switch", "-c", "conflict-side")
|
||||
write_variant(destination, file_count, region_count, target_size, "theirs")
|
||||
run_git(destination, "add", "conflicts")
|
||||
run_git(destination, "commit", "-m", "theirs")
|
||||
|
||||
run_git(destination, "switch", "main")
|
||||
write_variant(destination, file_count, region_count, target_size, "ours")
|
||||
run_git(destination, "add", "conflicts")
|
||||
run_git(destination, "commit", "-m", "ours")
|
||||
|
||||
run_git(destination, "merge", "conflict-side", expect_failure=True)
|
||||
unresolved_count, total_bytes = validate(
|
||||
destination, file_count, region_count
|
||||
)
|
||||
except (OSError, RuntimeError, ValueError) as error:
|
||||
print(f"error: {error}", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
print(f"repository: {destination}")
|
||||
print(f"preset: {arguments.preset}")
|
||||
print(f"unresolved files: {unresolved_count}")
|
||||
print(f"conflict regions per file: {region_count}")
|
||||
print(f"total conflict regions: {unresolved_count * region_count}")
|
||||
print(f"working tree bytes: {total_bytes}")
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Loading…
Add table
Add a link
Reference in a new issue