Re: [PATCH v4 03/10] commit-graph: compute generation numbers

Derrick Stolee [off-list ref] writes:

On 4/29/2018 5:08 AM, Jakub Narebski wrote:

quoted

Derrick Stolee [off-list ref] writes:

[...]

quoted

It is a bit strange to me that the code uses get_be32 for reading, but
htonl for writing.  Is Git tested on non little-endian machines, like
big-endian ppc64 or s390x, or on mixed-endian machines (or
selectable-endian machines with data endianness set to non
little-endian, like ia64)?  If not, could we use for example openSUSE
Build Service (https://build.opensuse.org/) for this?

Since we are packing two values into 64 bits, I am using htonl() here
to arrange the 30-bit generation number alongside the 34-bit commit
date value, then writing with hashwrite(). The other 32-bit integers
are written with hashwrite_be32() to avoid translating this data
in-memory.

O.K., so you are using what is more effective and easier to use.
Nice to know, thanks for the information.

[...]

quoted

quoted

  +static void compute_generation_numbers(struct commit** commits,
+				       int nr_commits)
+{

[...]

quoted

quoted

+	for (i = 0; i < nr_commits; i++) {
+		if (commits[i]->generation != GENERATION_NUMBER_INFINITY &&
+		    commits[i]->generation != GENERATION_NUMBER_ZERO)
+			continue;

[...]

quoted

quoted

  +	compute_generation_numbers(commits.list, commits.nr);
+

Nice and simple.  All right.

I guess that we do not pass "struct packed_commit_list commits" as
argument to compute_generation_numbers instead of "struct commit**
commits.list" and "int commits.nr" to compute_generation_numbers() to
keep the latter nice and generic?

Good catch. There is no reason to not use packed_commit_list here.

Actually, now that v5 shows how using packed_commit_list looks like, in
my opinion it looks uglier.  And it might be easier to make mistake.

Also, depending on how compiler is able to optimize it, the version
passing packed_commit_list as an argument has one more indirection
(following two pointers) in the loop.

Best,
-- 
Jakub Narębski