On Sat, Feb 12, 2022 at 01:05:29AM +0200, Nir Soffer wrote:
On Fri, Feb 11, 2022 at 9:08 PM Nir Soffer <nsoffer@redhat.com> wrote:
On Fri, Feb 11, 2022 at 1:22 PM Richard W.M. Jones <rjones@redhat.com> wrote:
On Fri, Feb 11, 2022 at 03:21:21AM +0200, Nir Soffer wrote:
Add unit tests and benchmarks for AioBuffer. The tests are trivial but they server as running documentation, and they point out important details about the type.
The benchmarks show the efficiency of allocating a new buffer, zeroing it, and interfacing with Go code.
These tests will also ensure that we don't break anything by the next changes.
To run the benchmarks use:
$ go test -run=xxx -bench=. [...] +# Run the benchmarks with 10 milliseconds timeout to make sure they do +# not break by mistake, without overloading the CI. For performance +# testing run "go test" directly. +$GOLANG test -run=XXX -bench=. -benchtime=10ms
-run param is a regexp matching the names of the tests to run. It might be best to use something like this instead:
go test -run=<regexp> -bench=.
because elsewhere we use "XXX" to mark code that needs to be fixed.
The intent of this command is to run only the benchmark, using -run=XXX to match no test. I agree this is a poor choice for this project since we use XXX for other purposes.
Apart from this the whole series seems fine to me, ACK.
Thanks, I'll push this with a better regex.
Updated using this change:
$ git diff -U3 aio-buffer-v2.. diff --git a/golang/run-tests.sh b/golang/run-tests.sh index 3a07b23a..82dcca56 100755 --- a/golang/run-tests.sh +++ b/golang/run-tests.sh @@ -28,7 +28,9 @@ requires nbdkit --version # The -v option enables verbose output. $GOLANG test -count=1 -v
-# Run the benchmarks with 10 milliseconds timeout to make sure they do -# not break by mistake, without overloading the CI. For performance +# Run the only benchmarks with 10 milliseconds timeout to make sure they +# do not break by mistake, without overloading the CI. For performance # testing run "go test" directly. -$GOLANG test -run=XXX -bench=. -benchtime=10ms +# The -run=- parameter is the way to match no test, assuming that no test or +# sub test contains "-" in the name. +$GOLANG test -run=- -bench=. -benchtime=10ms
I'm waiting a few days in case Eric wants to comment.
I admit that I misread the original patch and thought this was a comment (with "XXX" as a placeholder), rather than a command that runs. I think your updated version is better. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top