On 11/12/2018 10:46 AM, Jeff King wrote:
On Mon, Nov 12, 2018 at 10:38:28AM -0500, Derrick Stolee wrote:
We could go either direction here, but this patch moves the alternates
struct over to the main directory style (rather than vice-versa).
Technically the alternates style is more efficient, as it avoids
rewriting the object directory name on each call. But this is unlikely
to matter in practice, as we avoid reallocations either way (and nobody
has ever noticed or complained that the main object directory is copying
a few extra bytes before making a much more expensive system call).
Hm. I've complained in the past [1] about a simple method like strbuf_addf()
over loose objects, but that was during abbreviation checks so we were
adding the string for every loose object but not actually reading the
objects.
[1]
https://public-inbox.org/git/20171201174956.143245-1-dstolee@xxxxxxxxxxxxx/
I suspect that had more to do with the cost of snprintf() than the extra
bytes being copied. And here we'd still be using addstr and addch
exclusively. I'm open to numeric arguments to the contrary, though. :)
I agree. I don't think it is worth investigating now, as the performance
difference should be moot. I am making a mental note to take a look here
if I notice a performance regression later. ;)
There's actually a lot of low-hanging fruit there for pre-sizing, too.
E.g., fill_sha1_path() calls strbuf_addch() in a loop, but it could
quite easily grow the 41 bytes it needs ahead of time. I wouldn't want
to change that without finding a measurable improvement, though. It
might not be a big deal due to fec501dae8 (strbuf_addch: avoid calling
strbuf_grow, 2015-04-16).
-Peff
