perf: optimize jump table initialization and reuse - #6943
Conversation
| if (VMConfig.allowTvmSelfdestructRestriction()) { | ||
| adjustSelfdestruct(table); | ||
| } | ||
| public static JumpTable prepareAndGetTable(boolean isConstantCall) { |
There was a problem hiding this comment.
Nice change — just to confirm, are these config-based adjustments expected to stay stable once this table has been prepared?
|
Just to confirm the intended lifecycle: prepareAndGetTable() is called for the top-level execution, while nested executions reuse the table that has already been prepared for that execution context. Is that correct? |
| return enabled.getAsBoolean(); | ||
| } | ||
|
|
||
| public Operation adjustCost(Function<Program, Long> newCost) { |
There was a problem hiding this comment.
These helper methods preserve the opcode, stack metadata, and enabled supplier while replacing only the cost or action. Is retaining the same enabled supplier important for keeping the activation behavior identical across the cached variants?
There was a problem hiding this comment.
Yes. Each variant should differ only in the cost or action being adjusted. Reusing the same enabled supplier ensures that opcode activation behavior and the remaining operation metadata stay unchanged.
| tableMap.put(Version.TRON_V1_3, newTronV13OperationSet()); | ||
| tableMap.put(Version.TRON_V1_4, newTronV14OperationSet()); | ||
| tableMap.put(Version.TRON_V1_5, newTronV15OperationSet()); | ||
| tableMap.put(LATEST_VERSION, newLatestOperationSet()); |
There was a problem hiding this comment.
The historical operation-set builders remain available, while only the latest set is registered in tableMap. Is this intended to preserve construction and test compatibility without initializing tables that are no longer selected at runtime?
There was a problem hiding this comment.
Exactly. The builders are retained for compatibility and targeted testing, but only the currently selected version needs a shared runtime table. This avoids initializing unused table instances.
| } | ||
| VM.play(program, OperationRegistry.getTable()); | ||
| // Reuse the table prepared by the top-level execution. | ||
| VM.play(program, OperationRegistry.getTable(isConstantCall())); |
There was a problem hiding this comment.
For nested contract creation, getTable(isConstantCall()) assumes that the corresponding table was already prepared by the top-level execution. Is the execution mode inherited consistently by all nested frames?
There was a problem hiding this comment.
Yes. Nested frames inherit the execution mode of the parent context, so they should reuse the table prepared for that mode instead of applying the same adjustments again.
|
@TonyStank911 Yes. The top-level execution prepares the configuration-dependent operations, and nested executions retrieve the already prepared table. This avoids repeating the same adjustment work during internal calls. |
|
|
||
| public class OperationRegistry { | ||
|
|
||
| private static final Operation DEFAULT_MLOAD = new Operation( |
There was a problem hiding this comment.
Nice work, the table isolation and cached operation variants make this much cleaner. Do you have rough numbers for the startup and per-execution gains after this change? It would be nice to capture them in the PR description.
6 participants
What does this PR do?
LATEST_VERSIONandnewLatestOperationSet().Why is this needed?
The previous implementation initialized jump tables for multiple historical versions even though only the latest version was selected for execution.
Transactions and constant calls also shared the same table, which allowed adjustments made for constant calls to affect transaction execution. In addition, configuration-dependent adjustments could be applied repeatedly during nested calls and contract creation.
This change centralizes the selection of the latest operation set and separates transaction and constant-call tables. It reduces startup initialization and repeated adjustment overhead while keeping the two execution paths isolated.
Tests