[PATCH v4 00/10] upload-pack: reduce lock contention when writing packfile data
Hi,
this small patch series fixes some heavy lock contention when writing data from git-upload-pack(1) into pipes. This lock contention can be observed when having hundreds of git-upload-pack(1) processes active at the same time that write data into pipes at dozens of gigabits per second.
I have uploaded the flame graph that clearly shows the lock contention at [1].
Changes in v4: - Drop a stale half-sentence that I wanted to remove. - Link to v3: https://lore.kernel.org/r/20260310-pks-upload-pack-write-contention-v3-0-8bc97aa3e267@pks.im
Changes in v3:
- Fix handling of `iov_len` overflows in writev(3p) wrapper.
- Add another patch that causes us to flush out data instead of
sending a 0005 keepalive packet.
- Link to v2: https://lore.kernel.org/r/20260303-pks-upload-pack-write-contention-v2-0-7321830f08fe@pks.imChanges in v2:
- Change the buffer size in git-pack-objects(1) to also reduce the
number of write syscalls over there.
- Introduce writev to half the number of syscalls when writing
pktlines.
- Use `sizeof(os->buffer)` instead of open-coding its size.
- Improve keepalive logic in git-upload-pack(1) to account for
buffering.
- Link to v1: https://lore.kernel.org/r/20260227-pks-upload-pack-write-contention-v1-0-7166fe255704@pks.imThanks!
Patrick
[1]: https://gitlab.com/gitlab-org/git/-/work_items/675
---
Patrick Steinhardt (10):
upload-pack: fix debug statement when flushing packfile data
upload-pack: adapt keepalives based on buffering
upload-pack: prefer flushing data over sending keepalive
upload-pack: reduce lock contention when writing packfile data
compat/posix: introduce writev(3p) wrapper
wrapper: introduce writev(3p) wrappers
sideband: use writev(3p) to send pktlines
csum-file: introduce `hashfd_ext()`
csum-file: drop `hashfd_throughput()`
builtin/pack-objects: reduce lock contention when writing packfile dataMakefile | 4 +++ builtin/pack-objects.c | 23 +++++++++++--- compat/posix.h | 14 +++++++++ compat/writev.c | 44 +++++++++++++++++++++++++++ config.mak.uname | 2 ++ csum-file.c | 28 +++++------------ csum-file.h | 16 ++++++++-- meson.build | 1 + sideband.c | 14 +++++++-- upload-pack.c | 81 +++++++++++++++++++++++++++++++++++++++----------- wrapper.c | 41 +++++++++++++++++++++++++ wrapper.h | 9 ++++++ write-or-die.c | 8 +++++ write-or-die.h | 1 + 14 files changed, 239 insertions(+), 47 deletions(-)
Range-diff versus v3:
1: 65471f969b = 1: f9896a8451 upload-pack: fix debug statement when flushing packfile data
2: c5705c1cb1 = 2: b1bc6f5749 upload-pack: adapt keepalives based on buffering
3: f34fa584f4 ! 3: fcf5f06375 upload-pack: prefer flushing data over sending keepalive
@@ Commit message
the early bit waiting for packfile URIs. But the optimization is easy
enough to realize.
- Do so and flush out data instead of sending an empty pktline. While at
- it, drop the useless
+ Do so and flush out data instead of sending an empty pktline.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
4: 8d08e93929 = 4: f2bb7a38aa upload-pack: reduce lock contention when writing packfile data
5: aa42217e43 = 5: 78a1bbb810 compat/posix: introduce writev(3p) wrapper
6: c5d194ca2b = 6: c199dd398a wrapper: introduce writev(3p) wrappers
7: e4df805840 = 7: b7fe2b818f sideband: use writev(3p) to send pktlines
8: 16f9968bcd = 8: b9d33b93cd csum-file: introduce `hashfd_ext()`
9: 47ae58440b = 9: b2af62c4ff csum-file: drop `hashfd_throughput()`
10: c92ccf9df2 = 10: 47f5090b66 builtin/pack-objects: reduce lock contention when writing packfile data--- base-commit: fb1b83bcddff60463f6e86bb021784c88d0b748c change-id: 20260227-pks-upload-pack-write-contention-435ce01f5fe9