Improve flat trace generation performance

[ahamlat]

PR description

When testing one of the blocks that has a lot of subcalls (838 internal transactions), we noticed that most of the time was spent on hasRevertInSubCall method.

This PR reduced the time to trace a 29 mgas block from more than 5 seconds to 1 second.

Fixed Issue(s)

[github-actions]

• I thought about documentation and added the doc-change-required label to this PR if updates are required.
• I thought about the changelog and included a changelog update if required.
• If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

[shemnon]

OPCODE request is optional.

[shemnon]

Consider adding an OPCODE constant to RevertOperation and ReturnOperation so we can reduce magic constants in the code.

[ahamlat]

Thanks for the suggestion, I was thinking actually of an Enum OpCode to store all the opcodes and use only the Enum in the EVM and outside, like Opcode.REVERT.getOpcodeNumber() for 0xFD
public enum Opcode {
STOP(0x00, "STOP"),
ADD(0x01, "ADD"),
MUL(0x02, "MUL"),
...
}
In terms of performance, I guess JIT will replace by the opCodeNumber or the opcode name at runtime

[ahamlat]

@shemnon I've added the OpCode enum within the EVM and would like to use it across all EVM opcode operations. An implementation example has been provided for ADDMOD. Does this approach work for you ?

[shemnon]

This is not what I meant. Such an OpCode enum breaks extensibility to downstream users use. Please revert.

[shemnon]

What I intended was something like this, where a constant is added to the Operation classes, not an entirely different enum.

besu/evm/src/main/java/org/hyperledger/besu/evm/operation/BlobHashOperation.java

Line 37 in c89e4db

public static final int OPCODE = 0x49;

[ahamlat]

I actually understood your suggestion but wanted to suggest an implementation that would work for similar use cases in the future, but it looks like it wasn't a good idea. I'm going to revert the last changes.

Changes are now problematic.

[ahamlat]

@shemnon Could you review the last changes ?

[jflo]

Might be worth making AbstractOperation.getOpcode() static and doing the comparison to that to avoid creating another range of enums.

[ahamlat]

This commit is outdated. Making AbstractOperation.getOpcode() static means that we need to make opcode static in AbstractOperation which is not the case, opcode depends on the implementation.
What suggested by Danno is very similar in some way to what you suggest, add a static constants directly in the implementation, that is stored in the opcode field.

[kinsi55]

Kind of a random question (Side note: I dont really have Java exp) but what is the point of this If condition? Wouldn't starting the loop at 1 and running it until size() - 1 have the same result? (Or if iterating in reverse has no side-effects, even starting at size() - 1 and running while its > 0)