Commit 6725fa0e12 changed copy_uint32_array() to utilize a Go hack for
accessing a C array as a Go slice in order to potentially benefit from
any optimizations in Go's copy() for bulk transfer of memory over
naive one-at-a-time iteration. But that commit also acknowledged that
no benchmark timings were performed, which would have been useful to
demonstrat an actual benefit for using hack in the first place. And
since we are copying data anyways (rather than using the slice to
avoid a copy), and network transmission costs have a higher impact to
performance than in-memory copying speed, it's hard to justify keeping
the hack without hard data.
What's more, while using Go's copy() on an array of C uint32_t makes
sense for 32-bit extents, our corresponding 64-bit code uses a struct
which does not map as nicely to Go's copy(). Using a common style
between both list copying helpers is beneficial to mainenance.
Additionally, at face value, converting C.size_t to int may truncate;
we could avoid that risk if we were to uniformly use uint64 instead of
int. But we can equally just panic if the count is oversized: our
state machine guarantees that the server's response fits within 64M
bytes (count will be smaller than that, since it is multiple bytes per
extent entry).
Suggested-by: Laszlo Ersek <ler...@redhat.com>
CC: Nir Soffer <nsof...@redhat.com>
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v4: new patch to the series, but previously posted as part of the
golang cleanups. Since then: rework the commit message as it is no
longer a true revert, and add a panic() if count exceeds expected
bounds.
---
generator/GoLang.ml | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 73df5254..cc7d78b6 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -516,11 +516,16 @@ let
/* Closures. */
func copy_uint32_array(entries *C.uint32_t, count C.size_t) []uint32 {
+ if (uint64(count) > 64*1024*1024) {
+ panic(\"violation of state machine guarantee\")
+ }
ret := make([]uint32, int(count))
- // See
https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
- // TODO: Use unsafe.Slice() when we require Go 1.17.
- s := (*[1 << 30]uint32)(unsafe.Pointer(entries))[:count:count]
- copy(ret, s)
+ addr := uintptr(unsafe.Pointer(entries))
+ for i := 0; i < int(count); i++ {
+ ptr := (*C.uint32_t)(unsafe.Pointer(addr))
+ ret[i] = uint32(*ptr)
+ addr += unsafe.Sizeof(*ptr)
+ }
return ret
}
";
--
2.41.0
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
