dusk-network / rusk-vm

The Dusk Rust WASM VM implementation
Mozilla Public License 2.0
54 stars 12 forks source link

VM2: dynamic scratch buffer implementation #321

Closed miloszm closed 2 years ago

miloszm commented 2 years ago

Describe what you want implemented Decide if dynamic scratch buffer implementation in VM2 is to be used, taking into consideration its feasibility and performance.

Describe "Why" this is needed We need to decide if we keep the dynamic scratch buffer or we opt for the static buffer solution.

Describe alternatives you've considered An alternative to a dynamically allocated buffer would be a static buffer - yet the downside of static buffer is that its size needs to be predicted by a smart contract creator, and if predicted wrong, it will not protect the user from buffer overrun errors.

Additional context We need to take into consideration contracts with simple a scalar state, as well as contracts with a state in the form of a complex, recursive data structure.

Result: Dynamically allocated scratch buffer has to be used. The reason for that is that the size needed for scratch memory may grow theoretically arbitrarily high, depending on what the contract is doing. Static buffer is only possible if we let the user be responsible for predicting its size, but such prediction may be hard, and the benefits from doing that, like better performance, are not that significant (in a range of 10%). Hence - I recommend using dynamically allocated buffer with growth steps set to a relatively high value, like 8192. This reduces the reallocation occurrences while still providing small and well adjusted buffer for less demanding cases.

One example scenario that leads to high scratch memory size is as follows:

Pushes won't cause scratch memory growth, as new elements are inserted directly into host memory. On the other hand, the pops cause abi_gets being executed from the host memory into a local wasm contract memory (i.e., the scratch buffer) so if we do the number of pops sufficient number of times, we can achieve arbitrary size of the scratch buffer. The following tests can be used to reproduce this scenario:

Similar scenario could be devised for Hamt as well or any other non-trivial data structure which does not serialise into a simple contiguous chunk of memory and uses internal references (and thus requires a storage-backed serialiser). Conclusion - we should use the dynamically allocated scratch buffer across the board, unless we want to finely tune some contracts and allow for a 2-tier approach. Dynamic buffer seems to be a fine and necessary solution for the initial deployment of VM2.

krl commented 2 years ago

I would suggest we actually do the re-allocation on a wasm page basis, as is the case in microkelvin itself. This would also make unneccesary the extended api in microkelvin.