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

Tue Feb 8 19:33:53 UTC 2022

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 at redhat.com>
> ---
>  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?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org