Re: [Libguestfs] [PATCH libnbd 1/9] golang: tests: Add test for AioBuffer

On Sun, Jan 30, 2022 at 01:33:29AM +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 how efficient is allocation a new buffer, zeroing it, and > interfacing with Go code. Wording suggestion: The benchmarks show the efficiency of allocating a new buffer, zeroing it, and interfacing with Go code.

> > This tests will also ensure that we don't break anything by the next Either "These tests" or "This test"

> 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-12 17551 69552 ns/op > BenchmarkFromBytes-12 9632 139112 ns/op > BenchmarkAioBufferBytes-12 69375 16410 ns/op > PASS > ok libguestfs.org/libnbd 5.843s > > Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt; > --- > golang/Makefile.am | 1 + > golang/libnbd_620_aio_buffer_test.go | 104 +++++++++++++++++++++++++++ > 2 files changed, 105 insertions(+) > create mode 100644 golang/libnbd_620_aio_buffer_test.go > > diff --git a/golang/Makefile.am b/golang/Makefile.am > index 10fb8934..ae0486dd 100644 > --- a/golang/Makefile.am > +++ b/golang/Makefile.am > @@ -37,20 +37,21 @@ source_files = \ > libnbd_300_get_size_test.go \ > libnbd_400_pread_test.go \ > libnbd_405_pread_structured_test.go \ > libnbd_410_pwrite_test.go \ > libnbd_460_block_status_test.go \ > libnbd_500_aio_pread_test.go \ > libnbd_510_aio_pwrite_test.go \ > libnbd_590_aio_copy_test.go \ > libnbd_600_debug_callback_test.go \ > libnbd_610_error_test.go \ > + libnbd_620_aio_buffer_test.go \ As discussed in a different thread, the numbering here groups somewhat-related functionality, and helps us keep cross-language tests correlated over testing the same features. Since you aren't adding counterpart tests to python or ocaml, I don't know what number would be best. But our existing numbering is more along the lines of 0xx for language-level loading, 1xx for NBD handle tests, 2xx for connection tests, 3xx for initial APIs after connecting, 4xx for synchronous APIs, 5xx for asynch APIs, and 6xx for high-level usage patterns. This feels like it might fit better in the 0xx series, since the benchmark does not use any NBD handle.

> $(NULL) > > generator_built = \ > bindings.go \ > closures.go \ > wrappers.go \ > wrappers.h \ > $(NULL) > > EXTRA_DIST = \ > diff --git a/golang/libnbd_620_aio_buffer_test.go b/golang/libnbd_620_aio_buffer_test.go > new file mode 100644 > index 00000000..2632f87f > --- /dev/null > +++ b/golang/libnbd_620_aio_buffer_test.go > @@ -0,0 +1,104 @@ > +/* libnbd golang tests > + * Copyright (C) 2013-2021 Red Hat Inc. You may want to add 2022. Take the rest of my review with a grain of salt; I'm not (yet?) a Go expert. > + > +package libnbd > + > +import ( > + "bytes" > + "testing" > +) > + > +func TestAioBuffer(t *testing.T) { > + /* Create a buffer with unitinialized backing array. */ uninitialized > + 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 arrary 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 a nother buffer from Go slice. */ another > + 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 uninitilized buffer. an uninitialized > +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 coping a Go slice. > +func BenchmarkFromBytes(b *testing.B) { > + for i := 0; i < b.N; i++ { > + zeroes := make([]byte, bufferSize) > + buf := FromBytes(zeroes) > + buf.Free() > + } > +} > + > +// Benchmark creating a slice by copying the contents. > +func BenchmarkAioBufferBytes(b *testing.B) { > + buf := MakeAioBuffer(bufferSize) > + defer buf.Free() > + var r int > + > + b.ResetTimer() > + for i := 0; i < b.N; i++ { > + r += len(buf.Bytes()) > + } > +} > -- > 2.34.1 > How long do these benchmark tests take to run? The longer you run a benchmark, the more accurate of a number we can give for a per-operation average, but it also eats up a lot of CPU.

Is this something we want running on every 'make check' to ensure no regressions (since it doesn't even involve an NBD handle)?

Is there a way to tune things so that 'make check' runs a bare-bones version to avoid bit rot, but another 'make benchmark' (or some such name) runs the full length of a benchmark?