perf(vm): Improve snapshot management in batch executor #2513
Conversation
core/node/state_keeper/src/batch_executor/tests/batch_vm_factory.rs
Outdated
Show resolved
Hide resolved
| /// External callers must follow the following snapshot workflow: | ||
| /// | ||
| /// - Each new snapshot created using `make_snapshot()` must be either popped or rolled back before creating the following snapshot. | ||
| /// OTOH, it's not required to call either of these methods by the end of VM execution. |
There was a problem hiding this comment.
What does this mean? Seems to contradict the line above.
There was a problem hiding this comment.
What's the contradiction? We don't create a snapshot at the end of the VM execution, so it may have either 0 or 1 snapshot at the end.
There was a problem hiding this comment.
it's not required to call either of these methods by the end of VM execution.
This confused me (and still does) but I can understand now that it means that you can have multiple TX after your snapshot and then rollback/forget.
| /// - `pop_snapshot_no_rollback()` may be called spuriously, when no snapshot was created. It is a no-op in this case. | ||
| /// | ||
| /// These rules guarantee that at each given moment, a VM instance has <=1 snapshot (unless the VM makes snapshots internally), | ||
| /// which may allow additional VM optimizations. |
There was a problem hiding this comment.
I think that a low amount of snapshots just avoids allocating a Vec for them. Zero snapshots allow forgetting history, which is important for memory use.
slowli
force-pushed
the
aov-pla-1000-improve-snapshot-management-in-batch-executor
branch
from
bcec129 to
b51e26d
Compare
|
I've decided to remove batch VM traits from the PR. They can be added separately, after the new VM is merged into |
|
There are 2 somewhat suspicious failing CI jobs on this PR, but I don't see how they could be related to this PR:
Since the new VM isn't integrated into the API server, I don't see how the |
| self.snapshots | ||
| .pop() | ||
| .expect("Snapshot should be created before rolling it back"); | ||
| self.snapshots.pop(); |
There was a problem hiding this comment.
When is it necessary to pop a nonexistent snapshot?
There was a problem hiding this comment.
It happens here on the first transaction, or after the previous transaction was rolled back.
There was a problem hiding this comment.
I think you could just always pop the snapshot if it isn't rolled back in the same place where you may roll it back.
slowli
deleted the
aov-pla-1000-improve-snapshot-management-in-batch-executor
branch
What ❔
Why ❔
Checklist
zk fmtandzk lint.