Linus Torvalds [off-list ref] writes:
I think the hash function with its comment is self-explanatory:
/*
* This effectively just creates a sortable number from the
* last sixteen non-whitespace characters. Last characters
* count "most", so things that end in ".c" sort together.
*/
while ((c = *name++) != 0) {
if (isspace(c))
continue;
hash = (hash >> 2) + (c << 24);
}
return hash;
ie we just create a 32-bit hash, where we "age" previous characters by two
bits, so the last characters in a filename count most. So when we then
compare the hashes in the sort routine, filenames that end the same way
sort the same way.
IIRC, sometimes this function is called with path and name split
and sometimes with full path in name, depending on who calls you
(the latter happens for rev-list --object generated names, and
the former is for objects we extract ourselves from the --thin
base tree, or something like that). I suspect your patch may
break paths whose filename after the last slash is shorter than
16 bytes.
On Mon, 5 Jun 2006, Junio C Hamano wrote:
IIRC, sometimes this function is called with path and name split
and sometimes with full path in name
Yeah, I was pretty confused by the whole hashing thing. Are you sure that
complexity is needed, it seems a bit overkill.
Linus
Linus Torvalds [off-list ref] writes:
On Mon, 5 Jun 2006, Junio C Hamano wrote:
quoted
IIRC, sometimes this function is called with path and name split
and sometimes with full path in name
Yeah, I was pretty confused by the whole hashing thing. Are you sure that
complexity is needed, it seems a bit overkill.
Two issues in the code confuses any reader of that function.
- The code wants to hash Makefile from different revisions
together, and Makefile and t/Makefile close to each other.
The current code did it by treating '/' specially, used
basename hash as the upper bits of the resulting hash and
dirname hash as the lower bits. It's my tendency to treat
slashes specially too much, which is one of your favorite
things to pick me on.
This is not needed by your change anymore -- by only using
the tail of the filename, and making sure tail part weighs
more in the resulting group number, the new code gives the
desired grouping characteristics in a much simpler way.
- The output from rev-list --objects is fed to the function as
its name parameter while path is set to NULL. When we allow
a thin pack to be generated, rev-list --objects output also
contains "-<commit-object-name>" lines. We read trees for
these commits that are not going to be sent but can be used
as base objects, and pass the pathname discovered from the
tree using path and name pair (path is set to the linked list
of struct name_path that describes the dirname, and name is
set to the basename). This was done to reduce the need for
allocating and copying the pathname in preparation for
calling name_hash() function.
If you use only the "name" variable in your group number
computation, and suppose we are doing send-pack to send
updates between rev A..B, contrib/git-svn/Makefile from rev B
will use git-svn/Makefile (tail 16 characters) to compute the
number, but the blob from rev A (which we are not going to
send but would want to use as a potential delta base) will
have contrib/git-svn part in "path" (the element points at
string "git-svn", and its uplink points at another element
that points at "contrib" with an uplink that says it is at
the root level), and Makefile in "name". They will be hashed
slightly differently.