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
[Libguestfs] [PATCH libnbd v2 1/9] golang: tests: Add test for AioBuffer
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=.
goos: linux
goarch: amd64
pkg: libguestfs.org/libnbd
cpu: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
BenchmarkMakeAioBuffer-126871759 157.2 ns/op
BenchmarkAioBufferZero-12 17551 69552 ns/op
BenchmarkFromBytes-12 9632139112 ns/op
BenchmarkAioBufferBytes-12 69375 16410 ns/op
PASS
ok libguestfs.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.
Signed-off-by: Nir Soffer
---
golang/Makefile.am | 1 +
golang/libnbd_020_aio_buffer_test.go | 104 +++
golang/run-tests.sh | 5 ++
3 files changed, 110 insertions(+)
create mode 100644 golang/libnbd_020_aio_buffer_test.go
diff --git a/golang/Makefile.am b/golang/Makefile.am
index 10fb8934..f170cbc4 100644
--- a/golang/Makefile.am
+++ b/golang/Makefile.am
@@ -19,20 +19,21 @@ include $(top_srcdir)/subdir-rules.mk
source_files = \
aio_buffer.go \
bindings.go \
callbacks.go \
closures.go \
handle.go \
wrappers.go \
wrappers.h \
libnbd_010_load_test.go \
+ libnbd_020_aio_buffer_test.go \
libnbd_100_handle_test.go \
libnbd_110_defaults_test.go \
libnbd_120_set_non_defaults_test.go \
libnbd_200_connect_command_test.go \
libnbd_210_opt_abort_test.go \
libnbd_220_opt_list_test.go \
libnbd_230_opt_info_test.go \
libnbd_240_opt_list_meta_test.go \
libnbd_300_get_size_test.go \
libnbd_400_pread_test.go \
diff --git a/golang/libnbd_020_aio_buffer_test.go
b/golang/libnbd_020_aio_buffer_test.go
new file mode 100644
index ..5898746b
--- /dev/null
+++ b/golang/libnbd_020_aio_buffer_test.go
@@ -0,0 +1,104 @@
+/* libnbd golang tests
+ * Copyright (C) 2013-2022 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+package libnbd
+
+import (
+ "bytes"
+ "testing"
+)
+
+func TestAioBuffer(t *testing.T) {
+ /* Create a buffer with uninitialized backing array. */
+ buf := MakeAioBuffer(uint(32))
+ defer buf.Free()
+
+ /* Initialize backing array contents. */
+ for i := uint(0); i < buf.Size; i++ {
+ *buf.Get(i) = 0
+ }
+
+ /* Create a slice by copying the backing array contents into Go memory.
*/
+ b := buf.Bytes()
+
+ zeroes := make([]byte, 32)
+ if !bytes.Equal(b, zeroes) {
+ t.Fatalf("Expected %v, got %v", zeroes, buf.Bytes())
+ }
+
+ /* Modifying returned slice does not modify the buffer. */
+ for i := 0; i < len(b); i++ {
+ b[i] = 42
+ }
+
+ /* Bytes() still returns zeroes. */
+ if !bytes.Equal(buf.Bytes(), zeroes) {
+ t.Fatalf("Expected %v, got %v", zeroes, buf.Bytes())
+ }
+
+ /* Create another buffer from Go slice. */
+ buf2 := FromBytes(zeroes)
+ defer buf2.Free()
+
+ if !bytes.Equal(buf2.Bytes(), zeroes) {
+ t.Fatalf("Expected %v, got %v", zeroes, buf2.Bytes())
+ }
+}
+
+// Typical buffer size.
+const bufferSize uint = 256 * 1024
+
+// Benchmark creating an uninitialized buffer.
+func BenchmarkMakeAioBuffer(b *testing.B) {
+ for i := 0; i < b.N; i++ {
+ buf := MakeAioBuffer(bufferSize)
+ buf.Free()
+ }
+}
+
+// Benchmark zeroing a buffer.
+func BenchmarkAioBufferZero(b *testing.B) {
+ for i := 0; i < b.N; i++ {
+ buf := MakeAioBuffer(bufferSize)
+ for i := uint(0); i < bufferSize; i++ {
+ *buf.Get(i) = 0
+ }
+ buf.Free()
+ }
+}
+
+// Benchmark creating a buffer by copying a Go slice.
+func BenchmarkFr
