Re: [PATCH 1/1] upload-pack.c: make output buffer size configurable

[Jacob Vosmaer <jacob@gitlab.com>]

On Tue, Dec 14, 2021 at 4:37 PM Jeff King <peff@peff.net> wrote:
>
> It gets about 2.3GB/s with the tip of 'master' and 3.2GB/s with the
> equivalent of your patch (using LARGE_PACKET_DATA_MAX). So definitely an
> improvement.
>
> Without the cached case (so actually running pack-objects, albeit a
> pretty quick one because of bitmaps and pack-reuse), the timings are
> about the same (171MB/s versus 174MB/s, but really it's just pegging a
> CPU running pack-objects). So it would be fine to just do this
> unconditionally, I think.

Thank you for validating this!

> Looking at strace, the other thing I notice is that we write() the
> packet header separately in send_sideband(), which doubles the number of
> syscalls. I hackily re-wrote this to use writev() instead (patch below),
> but it doesn't seem to actually help much (maybe a curiosity to explore
> further, but definitely not something to hold up your patch).

Those double writes bug me too. Getting rid of them was one of the
things I liked about using stdio here. Curious now if writev will make
an impact in my benchmarks. As your experiments indicate, the double
writes may not be a problem in real life. Either way, I also feel it
is not something to rope into this patch set.