Re: [PATCH v2 6/8] refs: add update_refs for multiple simultaneous updates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/02/2013 07:20 PM, Brad King wrote:
> On 09/01/2013 02:08 AM, Junio C Hamano wrote:
>>> Though the refs themeselves cannot be modified together in a single
>>
>> "themselves".
> 
> Fixed.
> 
>> I notice that we are using an array of structures and letting qsort
>> swap 50~64 bytes of data
> 
> Michael suggested this too, so fixed.

Hmmm, I see that you changed the signature of update_refs() to take an
array of pointers.  My suggestion was unclear, but I didn't mean that
the function signature had to be changed.  Rather, I meant that *within*
the function, you could have created an array of pointers to the
structures in the input array and thereafter accessed it via the pointers:

int update_refs(const char *action, const struct ref_update *updates_orig,
		int n, enum action_on_err onerr)
{
	[...]
	struct ref_update **updates;

	[...]
	updates = xcalloc(n, sizeof(*updates));
	for (i = 0; i < n; i++)
		updates[i] = &updates_orig[i];
	[...]
}

However, your approach is also fine.  It will typically involve more
malloc()s but smaller memcpy()s (i.e., via ALLOC_GROW()) at the caller,
and since usually the number of ref_updates being done at one time will
be limited anyway, I don't see a reason to prefer one version over the
other.

Thanks for making the change.

Michael

-- 
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]