On Sun, Nov 14, 2021 at 8:09 PM Richard W.M. Jones <rjones@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