From ed3c202c20483863ad61ea35003082f7c08e90ea Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Tue, 1 Jul 2025 17:06:26 +0200 Subject: [PATCH] libstore: be more economical about fcntl on RemoteStore download progress reports send a STDERR_RESULT frame. many concurrent downloads send many STDERR_RESULT frames. each of these frames has us run the report loop once. since many frames can happen in very little time we may receive many frames in a single read from the socket, and that in turn means we don't have to fcntl that socket on every round. we must still ensure that the socket is in the correct state for each part of the loop, and this does mean we may run two unnecessary fcntl sequences per processStderr call. that's a small price to pay though. Change-Id: I7af607d8c759b76aff0f6016435955e2f9456923 --- lix/libstore/remote-store.cc | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lix/libstore/remote-store.cc b/lix/libstore/remote-store.cc index db4b9d7cf..aba00c790 100644 --- a/lix/libstore/remote-store.cc +++ b/lix/libstore/remote-store.cc @@ -775,19 +775,23 @@ static Logger::Fields readFields(Source & from) kj::Promise> RemoteStore::Connection::processStderr(AsyncFdIoStream & stream) try { + // SAFETY NOTE: while we're running we own the executor, and thus the stream. + // setting these flags is unsafe if the stream is shared with another thread. + const auto oldState = makeBlocking(getFD()); + KJ_DEFER(resetBlockingState(getFD(), oldState)); + while (true) { // fill the read buffer asynchronously until we have at least a message type. // once we have this we'll continue synchronously as in the sendCommand case. - while (fromBuf->used() < sizeof(uint64_t)) { - const auto available = fromBuf->getWriteBuffer(); - fromBuf->added(TRY_AWAIT(stream.read(available.data(), available.size()))); + if (fromBuf->used() < sizeof(uint64_t)) { + const auto oldState = makeNonBlocking(getFD()); + KJ_DEFER(resetBlockingState(getFD(), oldState)); + while (fromBuf->used() < sizeof(uint64_t)) { + const auto available = fromBuf->getWriteBuffer(); + fromBuf->added(TRY_AWAIT(stream.read(available.data(), available.size()))); + } } - // SAFETY NOTE: while we're running we own the executor, and thus the stream. - // setting these flags is unsafe if the stream is shared with another thread. - const auto oldState = makeBlocking(getFD()); - KJ_DEFER(resetBlockingState(getFD(), oldState)); - FdSource from{getFD(), fromBuf}; from.specialEndOfFileError = "Nix daemon disconnected while waiting for a response";