Re: [PATCH] index-pack, unpack-objects: increase input buffer from 4 KiB to 128 KiB

[Junio C Hamano <gitster@pobox.com>]

"Scott Bauersfeld via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Scott Bauersfeld <sbauersfeld@g.ucla.edu>
>
> Both index-pack and unpack-objects read pack data from stdin through
> a 4 KiB static buffer (input_buffer[4096]). On each fill(), consumed
> bytes are flushed to the output pack file via write_or_die(), so
> every write(2) moves at most 4 KiB.

Micronit.  Output of unpack-objects obviously does not get flushed
to "the output pack file".

> On FUSE-backed filesystems every write(2) is a synchronous round
> trip through the FUSE protocol (userspace -> kernel -> userspace ->
> back), so the 4 KiB buffer turns a clone into many unnecessary tiny
> writes with noticeable latency overhead.
>
> Increase the buffer from 4 KiB to 128 KiB, matching the default
> already used by the hashfile layer in csum-file.c.

Quite sensible reasoning presented very nicely.

It may probably be a #leftoverbit but these three instances of (128
* 1024) may want to have a common symbolic constant, like

    #define DEFAULT_IOBUFFER_SIZE_IN_BYTES (128 * 1024)

in a bit more central header file.  Especially for the one in
csum-file.c where there is no symbolic constant used for that
purpose.

> Testing with strace on HTTPS clones of git/git (~296 MB pack, 5 runs
> per variant, isolated builds from the same v2.54.0 source) shows:
>
>   index-pack pack file writes: 72,465 -> 24,943 avg (66% reduction)
>   total write() syscalls:     310,192 -> 259,530 avg (17% reduction)
>   writes of exactly 4096 bytes: ~40,077 -> 0 (eliminated)

Hmph, I would have expected more like (1 - 4/128) ~ 97% reduction.
The difference between that and 66% is coming from where?  There are
inherently short writes that do not utilize the new larger buffer
beyond 4kB?  If so, another number of interest might be the number
of writes smaller than 4096 bytes, perhaps?