Re: [Libguestfs] [PATCH libnbd v2 1/9] golang: tests: Add test for AioBuffer
On Tue, Feb 15, 2022 at 12:18 AM Nir Soffer wrote: > On Mon, Feb 14, 2022 at 3:22 PM Eric Blake 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 >> >> serve >> > > Fixed > > >> >> > 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=. >> > goos: linux >> > goarch: amd64 >> > pkg: libguestfs.org/libnbd >> > cpu: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz >> > BenchmarkMakeAioBuffer-12 6871759 157.2 ns/op >> > BenchmarkAioBufferZero-1217551 69552 ns/op >> > BenchmarkFromBytes-12 9632139112 ns/op >> > BenchmarkAioBufferBytes-12 69375 16410 ns/op >> > PASS >> > oklibguestfs.org/libnbd 5.843s >> > >> > To make sure the benchmarks will not break, we run them in "make check" >> > with a very short timeout. For actual performance testing run "go test" >> > directly. >> >> Sounds good to me. >> > > Thanks, I'll push tomorrow if you don't have more comments. > Pushed as 28137eea9b78ca32fce97f8c68483f447b861bc9..9099657ef541a635eae0d6d79080ad5bb0bc3281 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd v2 1/9] golang: tests: Add test for AioBuffer
On Mon, Feb 14, 2022 at 3:22 PM Eric Blake 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 > > serve > Fixed > > > 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=. > > goos: linux > > goarch: amd64 > > pkg: libguestfs.org/libnbd > > cpu: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz > > BenchmarkMakeAioBuffer-12 6871759 157.2 ns/op > > BenchmarkAioBufferZero-1217551 69552 ns/op > > BenchmarkFromBytes-12 9632139112 ns/op > > BenchmarkAioBufferBytes-12 69375 16410 ns/op > > PASS > > oklibguestfs.org/libnbd 5.843s > > > > To make sure the benchmarks will not break, we run them in "make check" > > with a very short timeout. For actual performance testing run "go test" > > directly. > > Sounds good to me. > Thanks, I'll push tomorrow if you don't have more comments. Nir ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd v2 1/9] golang: tests: Add test for AioBuffer
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 serve > 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=. > goos: linux > goarch: amd64 > pkg: libguestfs.org/libnbd > cpu: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz > BenchmarkMakeAioBuffer-12 6871759 157.2 ns/op > BenchmarkAioBufferZero-1217551 69552 ns/op > BenchmarkFromBytes-12 9632139112 ns/op > BenchmarkAioBufferBytes-12 69375 16410 ns/op > PASS > oklibguestfs.org/libnbd 5.843s > > To make sure the benchmarks will not break, we run them in "make check" > with a very short timeout. For actual performance testing run "go test" > directly. Sounds good to me. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd v2 1/9] golang: tests: Add test for AioBuffer
On Sat, Feb 12, 2022 at 01:05:29AM +0200, Nir Soffer wrote: > On Fri, Feb 11, 2022 at 9:08 PM Nir Soffer wrote: > > > > On Fri, Feb 11, 2022 at 1:22 PM Richard W.M. Jones > > 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= -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 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd v2 1/9] golang: tests: Add test for AioBuffer
On Fri, Feb 11, 2022 at 9:08 PM Nir Soffer wrote: > > On Fri, Feb 11, 2022 at 1:22 PM Richard W.M. Jones 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= -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. ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd v2 1/9] golang: tests: Add test for AioBuffer
On Fri, Feb 11, 2022 at 1:22 PM Richard W.M. Jones 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= -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. Nir ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH libnbd v2 1/9] golang: tests: Add test for AioBuffer
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= -bench=. because elsewhere we use "XXX" to mark code that needs to be fixed. Apart from this the whole series seems fine to me, ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
