[bug#41720] [PATCH] store: Use buffered I/O for all protocol writes

[Ludovic Courtès]

Hi,

Lars-Dominik Braun <ldb@leibniz-psychology.org> skribis:

> I did some digging to investigate why SSH-based guix operations are currently
> so slow. One of the reasons seems to be that some operations write single 
> words
> to the socket and due to NOWAIT these result in a lot of small packets.

Good catch!

> The attached patch modifies store.scm to buffer all writes, so that won’t 
> happen
> any more. I’m seeing about ~40% speedup (6.194s vs 10.075s for the best out of
> five runs) for `GUIX_DAEMON_SOCKET=ssh://localhost guix environment guix --
> true`, but it also seems to have a negative impact on UNIX domain socket
> communication by ~10% (1.561s vs 1.385s). For the notorious r-learnr package
> it’s still ~20% better (ssh, 16.620/21.418) and only ~3% worse (unix,
> 11.489/11.199). Keep in mind localhost has a much larger MTU and much lower
> latency than usual networks though. It might benefit from having an even 
> bigger
> write buffer (64k vs. currently 8k).

Nice, though the slowdown on Unix-domain sockets is annoying given that
it’s the primary transport.  Are the 10% significant and stable over
multiple runs?

It may be that writing through the buffered port, with the extra
allocations, indirection through the custom port, etc., outweighs the
gains.

OTOH all the other RPCs are buffered, so it makes sense to just do the
same.

(It may be wiser to make the ‘operation’ macro smarter and emit code
that directly allocates a bytevector of the right size and sends it.
Future work!)

> From eb38dc246e5dd33f8a48a99aded8b8dc0c378376 Mon Sep 17 00:00:00 2001
> From: Lars-Dominik Braun <ldb@leibniz-psychology.org>
> Date: Fri, 5 Jun 2020 10:38:32 +0200
> Subject: [PATCH] store: Use buffered I/O for all protocol writes
>
> * guix/store.scm (run-gc) Use buffered output port.
> (export-path) Same.
> (add-file-tree-to-store) Same.
> (set-build-options): Same. Add explicit flush.

Look:

--8<---------------cut here---------------start------------->8---
$ GUIX_PROFILING=rpc guix environment guix -- true
Remote procedure call summary: 1927 RPCs
  built-in-builders              ...     1
  build-things                   ...     1
  query-substitutable-path-infos ...     1
  valid-path?                    ...     2
  add-to-store/tree              ...    19
  query-references               ...   108
  add-to-store                   ...   133
  add-text-to-store              ...  1662
--8<---------------cut here---------------end--------------->8---

So probably the gain you’re seeing comes from ‘add-file-tree-to-store’.

Otherwise the patch LGTM as long as it doesn’t introduce any test suite
regressions.

Thanks for investigating!

Ludo’.