Skip to content

Improve binary search performance time for diff creation #3

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Improve binary search performance time for diff creation #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 12 additions & 2 deletions src/diff.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,20 @@ static int64_t search(int64_t *I, u_char *old, int64_t oldsize,
}

x = st + (en - st) / 2;
if (memcmp(old + I[x], new, MIN(oldsize - I[x], newsize)) < 0) {

int64_t length = MIN(oldsize - I[x], newsize);
int result = memcmp(old + I[x], new, length);

if (result < 0) {
return search(I, old, oldsize, new, newsize, x, en, pos);
} else {
} else if (result > 0) {
return search(I, old, oldsize, new, newsize, st, x, pos);
} else {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this else is not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, yes, but this is a style preference. I think having the else block keeps the code in a more maintainable state, since it is linked (conceptually) to the memcmp result.

/* As a special case, short circuit for the first exact match
* between old_data and new_data, since future exact matches
* will have shorter length. */
*pos = I[en];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before assign a value to pos , you should validate that it is not NULL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function that populates the I array will never add NULL values because the values are offsets into old_data (so, greater than or equal to 0).

return length;
}
}

Expand Down