Skip to content

gix-blame: Replace BTreeMap by SmallVec #1945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions gix-blame/src/file/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub fn file(
break;
}

let is_still_suspect = hunks_to_blame.iter().any(|hunk| hunk.suspects.contains_key(&suspect));
let is_still_suspect = hunks_to_blame.iter().any(|hunk| hunk.has_suspect(&suspect));
if !is_still_suspect {
// There are no `UnblamedHunk`s associated with this `suspect`, so we can continue with
// the next one.
Expand Down Expand Up @@ -189,7 +189,7 @@ pub fn file(
.collect();

for hunk in hunks_to_blame.iter() {
if let Some(range_in_suspect) = hunk.suspects.get(&suspect) {
if let Some(range_in_suspect) = hunk.get_range(&suspect) {
let range_in_blamed_file = hunk.range_in_blamed_file.clone();

for (blamed_line_number, source_line_number) in range_in_blamed_file.zip(range_in_suspect.clone()) {
Expand Down
34 changes: 21 additions & 13 deletions gix-blame/src/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fn process_change(
// 3. `change` *must* be returned if it is not fully included in `hunk`.
match (hunk, change) {
(Some(hunk), Some(Change::Unchanged(unchanged))) => {
let Some(range_in_suspect) = hunk.suspects.get(&suspect) else {
let Some(range_in_suspect) = hunk.get_range(&suspect) else {
// We don’t clone blame to `parent` as `suspect` has nothing to do with this
// `hunk`.
new_hunks_to_blame.push(hunk);
Expand Down Expand Up @@ -102,7 +102,7 @@ fn process_change(
}
}
(Some(hunk), Some(Change::AddedOrReplaced(added, number_of_lines_deleted))) => {
let Some(range_in_suspect) = hunk.suspects.get(&suspect).cloned() else {
let Some(range_in_suspect) = hunk.get_range(&suspect).cloned() else {
new_hunks_to_blame.push(hunk);
return (None, Some(Change::AddedOrReplaced(added, number_of_lines_deleted)));
};
Expand Down Expand Up @@ -247,7 +247,7 @@ fn process_change(
}
}
(Some(hunk), Some(Change::Deleted(line_number_in_destination, number_of_lines_deleted))) => {
let Some(range_in_suspect) = hunk.suspects.get(&suspect) else {
let Some(range_in_suspect) = hunk.get_range(&suspect) else {
new_hunks_to_blame.push(hunk);
return (
None,
Expand Down Expand Up @@ -359,12 +359,16 @@ fn process_changes(

impl UnblamedHunk {
fn shift_by(mut self, suspect: ObjectId, offset: Offset) -> Self {
self.suspects.entry(suspect).and_modify(|e| *e = e.shift_by(offset));
if let Some(position) = self.suspects.iter().position(|entry| entry.0 == suspect) {
if let Some((_, ref mut range_in_suspect)) = self.suspects.get_mut(position) {
*range_in_suspect = range_in_suspect.shift_by(offset);
}
}
self
}

fn split_at(self, suspect: ObjectId, line_number_in_destination: u32) -> Either<Self, (Self, Self)> {
match self.suspects.get(&suspect) {
match self.get_range(&suspect) {
None => Either::Left(self),
Some(range_in_suspect) => {
if !range_in_suspect.contains(&line_number_in_destination) {
Expand Down Expand Up @@ -405,34 +409,38 @@ impl UnblamedHunk {
/// This is like [`Self::pass_blame()`], but easier to use in places where the 'passing' is
/// done 'inline'.
fn passed_blame(mut self, from: ObjectId, to: ObjectId) -> Self {
if let Some(range_in_suspect) = self.suspects.remove(&from) {
self.suspects.insert(to, range_in_suspect);
if let Some(position) = self.suspects.iter().position(|entry| entry.0 == from) {
if let Some((ref mut commit_id, _)) = self.suspects.get_mut(position) {
*commit_id = to;
}
}
self
}

/// Transfer all ranges from the commit at `from` to the commit at `to`.
fn pass_blame(&mut self, from: ObjectId, to: ObjectId) {
if let Some(range_in_suspect) = self.suspects.remove(&from) {
self.suspects.insert(to, range_in_suspect);
if let Some(position) = self.suspects.iter().position(|entry| entry.0 == from) {
if let Some((ref mut commit_id, _)) = self.suspects.get_mut(position) {
*commit_id = to;
}
}
}

fn clone_blame(&mut self, from: ObjectId, to: ObjectId) {
if let Some(range_in_suspect) = self.suspects.get(&from) {
self.suspects.insert(to, range_in_suspect.clone());
if let Some(range_in_suspect) = self.get_range(&from) {
self.suspects.push((to, range_in_suspect.clone()));
}
}

fn remove_blame(&mut self, suspect: ObjectId) {
self.suspects.remove(&suspect);
self.suspects.retain(|entry| entry.0 != suspect);
}
}

impl BlameEntry {
/// Create an offset from a portion of the *Blamed File*.
fn from_unblamed_hunk(unblamed_hunk: &UnblamedHunk, commit_id: ObjectId) -> Option<Self> {
let range_in_source_file = unblamed_hunk.suspects.get(&commit_id)?;
let range_in_source_file = unblamed_hunk.get_range(&commit_id)?;

Some(Self {
start_in_blamed_file: unblamed_hunk.range_in_blamed_file.start,
Expand Down
25 changes: 19 additions & 6 deletions gix-blame/src/types.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
use crate::file::function::tokens_for_diffing;
use gix_hash::ObjectId;
use gix_object::bstr::BString;
use smallvec::SmallVec;
use std::num::NonZeroU32;
use std::{
collections::BTreeMap,
ops::{AddAssign, Range, SubAssign},
};
use std::ops::{AddAssign, Range, SubAssign};

/// Options to be passed to [`file()`](crate::file()).
#[derive(Default, Debug, Clone)]
Expand Down Expand Up @@ -198,8 +196,23 @@ impl LineRange for Range<u32> {
pub struct UnblamedHunk {
/// The range in the file that is being blamed that this hunk represents.
pub range_in_blamed_file: Range<u32>,
/// Maps a commit to the range in a source file (i.e. *Blamed File* at a revision) that is equal to `range_in_blamed_file`.
pub suspects: BTreeMap<ObjectId, Range<u32>>,
/// Maps a commit to the range in a source file (i.e. *Blamed File* at a revision) that is
/// equal to `range_in_blamed_file`. Since `suspects` rarely contains more than 1 item, it can
/// efficiently be stored as a `SmallVec`.
pub suspects: SmallVec<[(ObjectId, Range<u32>); 1]>,
}

impl UnblamedHunk {
pub(crate) fn has_suspect(&self, suspect: &ObjectId) -> bool {
self.suspects.iter().any(|entry| entry.0 == *suspect)
}

pub(crate) fn get_range(&self, suspect: &ObjectId) -> Option<&Range<u32>> {
self.suspects
.iter()
.find(|entry| entry.0 == *suspect)
.map(|entry| &entry.1)
}
}

#[derive(Debug)]
Expand Down
Loading