Instead of malloc'ing a C buffer, nbd_pread()ing into it, then copying it into an immutable Python bytes object, we can instead pre-create a correctly-sized Python bytearray object, then nbd_pread() directly into that object's underlying bytes. While the data copying might not be the bottleneck compared to the networking costs, it does have noticeable results; on my machine: $ export script=' m=1024*1024 size=h.get_size() for i in range(size // m): buf = h.pread(m, m*i) ' $ time ./run nbdkit -U - pattern 10G --run 'nbdsh -u $uri -c "$script"' sped up from about 7.8s pre-patch to about 7.1s post-patch, approximately a 10% speedup. Note that this slightly changes the python API: h.pread[_structured] now returns a mutable 'bytearray' object, rather than an immutable 'bytes' object. This makes it possible to modify the just-read string in place, rather than having to create yet another memory buffer for any modifications, which offers even more speedups when writing a read-modify-write paradigm in python. But the change is backwards-compatible - python already states that a read-write buffer may be returned instead of readonly for any client that already operated only on a buffer in a readonly manner. --- Note that h.pread is more like Python read() semantics in creating a buffer, while h.aio_pread is more like Python readinto() semantics in modifying a passed-in buffer. But now that both code paths have a python object prior to calling into the C API, my next task is to improve the h.*pread_structured callback function to pass its buffer as a slice of the Python input buffer, rather than doing yet another round of pointless memcpy from C into python objects. generator/Python.ml | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/generator/Python.ml b/generator/Python.ml index 4ab18f6..1c4446e 100644 --- a/generator/Python.ml +++ b/generator/Python.ml @@ -1,6 +1,6 @@ (* hey emacs, this is OCaml code: -*- tuareg -*- *) (* nbd client library in userspace: Python bindings - * Copyright (C) 2013-2021 Red Hat Inc. + * Copyright (C) 2013-2022 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -293,7 +293,7 @@ let | BytesIn (n, _) -> pr " Py_buffer %s = { .obj = NULL };\n" n | BytesOut (n, count) -> - pr " char *%s = NULL;\n" n; + pr " PyObject *%s = NULL;\n" n; pr " Py_ssize_t %s;\n" count | BytesPersistIn (n, _) | BytesPersistOut (n, _) -> @@ -432,8 +432,8 @@ let | Bool _ -> () | BytesIn _ -> () | BytesOut (n, count) -> - pr " %s = malloc (%s);\n" n count; - pr " if (%s == NULL) { PyErr_NoMemory (); goto out; }\n" n + pr " %s = PyByteArray_FromStringAndSize (NULL, %s);\n" n count; + pr " if (%s == NULL) goto out;\n" n | BytesPersistIn (n, _) | BytesPersistOut (n, _) -> pr " %s_buf = nbd_internal_py_get_aio_buffer (%s);\n" n n; pr " if (!%s_buf) goto out;\n" n; @@ -479,7 +479,7 @@ let function | Bool n -> pr ", %s" n | BytesIn (n, _) -> pr ", %s.buf, %s.len" n n - | BytesOut (n, count) -> pr ", %s, %s" n count + | BytesOut (n, count) -> pr ", PyByteArray_AS_STRING (%s), %s" n count | BytesPersistIn (n, _) | BytesPersistOut (n, _) -> pr ", %s_buf->data, %s_buf->len" n n | Closure { cbname } -> pr ", %s" cbname @@ -524,8 +524,9 @@ let let use_ret = ref true in List.iter ( function - | BytesOut (n, count) -> - pr " py_ret = PyBytes_FromStringAndSize (%s, %s);\n" n count; + | BytesOut (n, _) -> + pr " py_ret = %s;\n" n; + pr " %s = NULL;\n" n; use_ret := false | Bool _ | BytesIn _ @@ -572,7 +573,7 @@ let | BytesIn (n, _) -> pr " if (%s.obj)\n" n; pr " PyBuffer_Release (&%s);\n" n - | BytesOut (n, _) -> pr " free (%s);\n" n + | BytesOut (n, _) -> pr " Py_XDECREF (%s);\n" n | BytesPersistIn _ | BytesPersistOut _ -> () | Closure { cbname } -> pr " free_user_data (%s_user_data);\n" cbname -- 2.36.1