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 <nsof...@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
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
