On Sun, Nov 14, 2021 at 8:09 PM Richard W.M. Jones <rjones(a)redhat.com> wrote:
I'm lukewarm of this series, but I've got a few comments on it as well
as some questions below.
- In nbdkit we have a function tvdiff_usec (see
common/include/tvdiff.h). Maybe we should use that instead of
microtime?
It is much easier to use a function returning one value compared with
the pain of working with struct timeval/spec.
- Rather than having a single test that does multiple runs (and so
runs for a very long time), it's better to have multiple tests
because they can run in parallel. This is how it could be done (but
see also my comment about benchmarks below).
* Copy original tests/synch-parallel.sh to
tests/synch-parallel-conn-1-request-4096.sh
tests/synch-parallel-conn-1-request-262144.sh
etc. (Or choose better names)
Possible, but I think the benchmark is more useful when you run different
combinations at the same time. This way when you make a change in libnbd
you can tell how the change affects all combinations in one run.
* Each test does:
CONNECTIONS=1 REQUEST_SIZE=4096 ./synch-parallel.sh (etc)
* Original synch-parallel.sh is unchanged, but remove it from TESTS
This will be easy to fix
* Add the new scripts to TESTS
On Sun, Nov 14, 2021 at 09:21:39AM +0200, Nir Soffer wrote:
> I'm working on an application using the sync API. In my tests I see best read
> throughput with 2 nbd connections which is unexpected.
I'm interested in why you're using the synchronous API. I think it'll
inevitably be slower than using the asynch API because you can never
have multiple requests on a single TCP connection.
The synchronous API is easier to use and matches better what I want to do.
I have 2 use cases:
- creating checksums - I need to stream image data in order into the checksum
function.
- copying images between nbd and http endpoints. At the nbd side I can
have async
API, but for http the sync API is better and makes it easier to
minimize the number
of request. For example if I have 128 MiB of data, it is more
efficient to do one request
and stream 128 MiB than do 128 1MiB unordered requests.
In both cases I can create a layer reordering requests so I can
pull/push data fast as
possible from the nbd server, and use ordered APIs on the other side,
but this is hard
work and it makes sense only if it gives significant performance advantage.
...
> - The test scripts is much slower now (120 seconds instead of
> 10). We need to to separate the benchmark, running many
> combinations (synch-parallel-bench.sh) and the test, running one
> combination (sync-parallel.sh).
This is too long for tests. I think there are really two conflicting
requirements - you want to benchmark the synchronous API, which is a
worthwhile but separate goal from testing that things are working.
Exactly what I think, we like the vector benchmarks, we need to separate
the tests from the benchmarks, and make the tests much faster.
Should we consider having benchmarks in a separate directory
We can keep the benchmarks in a separate directory, but I don't think it
makes sense to duplicate sync-parallel.c and aio-parallel.c just to make
them configurable and compatible with different nbd servers. The same
program can be used for testing and benchmarks.
For example for testing we can run:
RUN_TIME=0.1 INTEGRETY=1 nbdkit ... program socket
For benchmarks we can run:
start some nbd server...
RUN_TIME=60 CONNECTIONS=4 REQUEST_SIZE=4k IO=read program socket
stop the server
or even a separate git repo? (Separate repo is what I did for
libguestfs).
I think it will be more useful to users of the library if the
benchmarks are included
in the project.
As first step I think I should remove the changes in the tests
scritps, so CI will
not be affected by these changes.
Nir