radareorg / radare2-bindings

Bindings of the r2 api for Valabind and friends
GNU Lesser General Public License v3.0
130 stars 92 forks source link

Improve Python performance by using MemoryView #213

Closed aronsky closed 5 years ago

aronsky commented 5 years ago

Python bindings for analysis and disassembly plugins passed buffers by using Py_BuildValue and passing the buffer as a string for Python consumption. This required Python to copy the whole buffer - a costly operation, especially when the buffer was large enough.

For a vast majority of architectures, copying the whole buffer is unnecessary, as the opcode can't be longer than several bytes (e.g., on x86 an opcode's size is limited to 15 bytes).

One option was passing R_MIN (len, MAX_INSTRUCTION_SIZE) instead of len, when creating the Python buffer. However, this has 2 problems: MAX_INSTRUCTION_SIZE is architecture-specific. We could retrieve it from the Python plugin on instantiation, but RAnalPlugin does not contain a proper field for storing it (instead, relying on archinfo to be used for retrieving this information). Calling archinfo every time to retrieve the maximum instruction size doesn't look right to me, and neither does storing the value statically in a global variable.

Instead, I rewrote the buffer passing code to use Python's buffer protocol and memory views - the recommended way of passing binary data to Python from C. By using the buffer protocol, we describe the buffer to Python (via a pointer to the buffer and a length), and Python looks directly at the memory provided, without copying it anywhere.

Caveat: the analysis and disassembly Python functions now receive a memoryview object instead of a bytes object. While it can be sliced as a regular array, it has some limitations. A bytes object based on it can be obtained by calling its tobytes method (which will initiate the copy - but can be called on a small slice, to save time and memory).


Additionally, and unrelated to the subject of this PR, I added a couple of missing refcount decrements to the analysis code.

aronsky commented 5 years ago

One more thing. I tried calling Py_DECREF on the memview variable after using it, but got segfaults. I am aware that I'm passing it with N instead of O, which doesn't increment the refcount - when passing with O, callling Py_DECREF afterwards works, but I don't see the difference between that and simply passing it with the N format string.

XVilka commented 5 years ago

Thank you, this looks good. Please update also the radare2book https://github.com/radare/radare2book/tree/master/plugins

XVilka commented 5 years ago

If I might ask, what Python version do you use? Python 2 or 3?

aronsky commented 5 years ago

Hi, I totally forgot to test it on Python 2 (I'm using 3 usually) - though I did make sure to use functions available in both 2 and 3 (Memory views have been added to Python in version 2.7).

However, checking now, it fails to build Python 2 anyway, unrelated to my changes (due to the use of PyUnicode_AsUTF8AndSize in py_io_read).

XVilka commented 5 years ago

Yes, and we plan to drop Python 2 support totally.

XVilka commented 5 years ago

See https://github.com/radare/radare2-bindings/issues/206 , so if you want you can remove Python 2 quirks in another PR.