On Tue, Jun 14, 2022 at 07:58:08AM -0500, Eric Blake wrote:
Add new methods nbd.Buffer.{to,from}_buffer that avoid the copying
present in the existing nbd.Buffer.{to,from}_bytearray. The reduction
in copies possible by this approach is no longer quite as necessary,
now that aio_p{read,write} have been converted to take buffers
directly. However, there is still one (marginal) benefit: if you use
h.set_pread_initialize(False), and create a new buffer for every I/O
(rather than reusing a buffer pool), nbd.Buffer(n) handed to
h.aio_pread then buf.to_buffer() is slightly faster than handing a
bytearray(n) directly to h.aio_pread, because we are able to skip the
step of pre-zeroing the buffer.
---
Instead of adding a copy=True/False parameter to the existing API as I
had previously suggested, I decided that a new API made more sense.
generator/Python.ml | 39 ++++++++++++++++++++++----
python/t/585-aio-buffer-share.py | 47 ++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+), 6 deletions(-)
create mode 100644 python/t/585-aio-buffer-share.py
diff --git a/generator/Python.ml b/generator/Python.ml
index 03a7e6b..cb89ccd 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -735,6 +735,21 @@ let
'''Allocate an uninitialized AIO buffer used for
nbd.aio_pread.'''
self._o = libnbdmod.alloc_aio_buffer(len)
+ @classmethod
+ def from_buffer(cls, buf):
+ '''Create an AIO buffer that shares an existing buffer-like object.
+
+ Because the buffer is shared, changes to the original are visible
+ to nbd.aio_pwrite, and changes in nbd.aio_pread are visible to the
+ original.
+ '''
+ self = cls(0)
+ # Ensure that buf is already buffer-like
+ with memoryview(buf):
+ self._o = buf
+ self._init = True
+ return self
+
@classmethod
def from_bytearray(cls, ba):
'''Create an AIO buffer from a bytearray or other buffer-like
object.
@@ -743,16 +758,28 @@ let
bytearray constructor. Otherwise, ba is copied. Either way, the
resulting AIO buffer is independent from the original.
'''
- self = cls(0)
- self._o = bytearray(ba)
- self._init = True
- return self
+ return cls.from_buffer(bytearray(ba))
- def to_bytearray(self):
- '''Copy an AIO buffer into a bytearray.'''
+ def to_buffer(self):
+ '''Return a shared view of the AIO buffer contents.
+
+ This exposes the underlying buffer; changes to the buffer are
+ visible to nbd.aio_pwrite, and changes from nbd.aio_pread are
+ visible in the buffer.
+ '''
if not hasattr(self, '_init'):
self._o = bytearray(len(self._o))
self._init = True
+ return self._o
+
+ def to_bytearray(self):
+ '''Copy an AIO buffer into a bytearray.
+
+ This copies the contents of an AIO buffer to a new bytearray, which
+ remains independent from the original.
+ '''
+ if not hasattr(self, '_init'):
+ return bytearray(len(self._o))
return bytearray(self._o)
def size(self):
diff --git a/python/t/585-aio-buffer-share.py b/python/t/585-aio-buffer-share.py
new file mode 100644
index 0000000..21752b7
--- /dev/null
+++ b/python/t/585-aio-buffer-share.py
@@ -0,0 +1,47 @@
+# libnbd Python bindings
+# Copyright (C) 2010-2022 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+# This tests the nbd.Buffer is_zero method. We can do this entirely
+# without using the NBD protocol.
+
+import nbd
+
+# Use of to/from_bytearray always creates copies
+ba = bytearray(512)
+buf = nbd.Buffer.from_bytearray(ba)
+ba.append(1)
+assert len(ba) == 513
+assert len(buf) == 512
+assert buf.is_zero()
+assert buf.to_bytearray() is not ba
+
+# Use of to/from_buffer shares the same buffer
+buf = nbd.Buffer.from_buffer(ba)
+assert not buf.is_zero()
+assert len(buf) == 513
+ba.pop()
+assert buf.is_zero()
+assert len(buf) == 512
+assert buf.to_buffer() is ba
+
+# Even though nbd.Buffer(n) start uninitialized, we sanitize before exporting.
+# This test cheats and examines the private member ._init
+buf = nbd.Buffer(512)
+assert buf.is_zero()
+assert not hasattr(buf, '_init')
+assert buf.to_buffer() == bytearray(512)
+assert hasattr(buf, '_init')
--
2.36.1
Acked-by: Richard W.M. Jones <rjones(a)redhat.com>
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