Improve Reference Test Performance (#5801)

* Improve Reference Test Performance

Improve reference test performance by caching protocol specs and stacked
world state values.

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>

* spotless

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>

---------

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>

ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/StateTestSubCommand.java

@@ -56,12 +56,14 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JavaType;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.base.Stopwatch;
+import com.google.common.base.Suppliers;
 import org.apache.tuweni.bytes.Bytes;
 import picocli.CommandLine.Command;
 import picocli.CommandLine.Option;
@@ -76,7 +78,10 @@
 public class StateTestSubCommand implements Runnable {
   public static final String COMMAND_NAME = "state-test";
 
-  @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"})
+  static final Supplier<ReferenceTestProtocolSchedules> referenceTestProtocolSchedules =
+      Suppliers.memoize(ReferenceTestProtocolSchedules::create);
+
+  @SuppressWarnings({"FieldCanBeFinal"})
   @Option(
       names = {"--fork"},
       description = "Force the state tests to run on a specific fork.")
@@ -99,9 +104,8 @@ public class StateTestSubCommand implements Runnable {
 
   @ParentCommand private final EvmToolCommand parentCommand;
 
-  @SuppressWarnings("MismatchedQueryAndUpdateOfCollection") // picocli does it magically
-  @Parameters
-  private final List<Path> stateTestFiles = new ArrayList<>();
+  // picocli does it magically
+  @Parameters private final List<Path> stateTestFiles = new ArrayList<>();
 
   @SuppressWarnings("unused")
   public StateTestSubCommand() {
@@ -174,8 +178,6 @@ private void executeStateTest(final Map<String, GeneralStateTestCaseSpec> genera
   }
 
   private void traceTestSpecs(final String test, final List<GeneralStateTestCaseEipSpec> specs) {
-    final var referenceTestProtocolSchedules = ReferenceTestProtocolSchedules.create();
-
     final OperationTracer tracer = // You should have picked Mercy.
         parentCommand.showJsonResults
             ? new StandardJsonTracer(
@@ -226,7 +228,7 @@ private void traceTestSpecs(final String test, final List<GeneralStateTestCaseEi
 
         final String forkName = fork == null ? spec.getFork() : fork;
         final ProtocolSchedule protocolSchedule =
-            referenceTestProtocolSchedules.getByName(forkName);
+            referenceTestProtocolSchedules.get().getByName(forkName);
         if (protocolSchedule == null) {
           throw new UnsupportedForkException(forkName);
         }

evm/src/main/java/org/hyperledger/besu/evm/worldstate/UpdateTrackingAccount.java

@@ -37,7 +37,7 @@
 import org.apache.tuweni.units.bigints.UInt256;
 
 /**
- * A implementation of {@link MutableAccount} that tracks updates made to the account since the
+ * An implementation of {@link MutableAccount} that tracks updates made to the account since the
  * creation of the updater this is linked to.
  *
  * <p>Note that in practice this only track the modified value of the nonce and balance, but doesn't
@@ -57,9 +57,11 @@ public class UpdateTrackingAccount<A extends Account> implements MutableAccount,
   private Wei balance;
 
   @Nullable private Bytes updatedCode; // Null if the underlying code has not been updated.
+  private final Bytes oldCode;
   @Nullable private Hash updatedCodeHash;
+  private final Hash oldCodeHash;
 
-  // Only contains updated storage entries, but may contains entry with a value of 0 to signify
+  // Only contains updated storage entries, but may contain entry with a value of 0 to signify
   // deletion.
   private final NavigableMap<UInt256, UInt256> updatedStorage;
   private boolean storageWasCleared = false;
@@ -80,6 +82,8 @@ public class UpdateTrackingAccount<A extends Account> implements MutableAccount,
     this.balance = Wei.ZERO;
 
     this.updatedCode = Bytes.EMPTY;
+    this.oldCode = Bytes.EMPTY;
+    this.oldCodeHash = Hash.EMPTY;
     this.updatedStorage = new TreeMap<>();
   }
 
@@ -101,6 +105,9 @@ public UpdateTrackingAccount(final A account) {
     this.nonce = account.getNonce();
     this.balance = account.getBalance();
 
+    this.oldCode = account.getCode();
+    this.oldCodeHash = account.getCodeHash();
+
     this.updatedStorage = new TreeMap<>();
   }
 
@@ -181,14 +188,14 @@ public void setBalance(final Wei value) {
   @Override
   public Bytes getCode() {
     // Note that we set code for new account, so it's only null if account isn't.
-    return updatedCode == null ? account.getCode() : updatedCode;
+    return updatedCode == null ? oldCode : updatedCode;
   }
 
   @Override
   public Hash getCodeHash() {
     if (updatedCode == null) {
       // Note that we set code for new account, so it's only null if account isn't.
-      return account.getCodeHash();
+      return oldCodeHash;
     } else {
       // Cache the hash of updated code to avoid DOS attacks which repeatedly request hash
       // of updated code and cause us to regenerate it.
@@ -202,7 +209,7 @@ public Hash getCodeHash() {
   @Override
   public boolean hasCode() {
     // Note that we set code for new account, so it's only null if account isn't.
-    return updatedCode == null ? account.hasCode() : !updatedCode.isEmpty();
+    return updatedCode == null ? !oldCode.isEmpty() : !updatedCode.isEmpty();
   }
 
   @Override
@@ -226,7 +233,7 @@ public UInt256 getStorageValue(final UInt256 key) {
       return UInt256.ZERO;
     }
 
-    // We haven't updated the key-value yet, so either it's a new account and it doesn't have the
+    // We haven't updated the key-value yet, so either it's a new account, and it doesn't have the
     // key, or we should query the underlying storage for its existing value (which might be 0).
     return account == null ? UInt256.ZERO : account.getStorageValue(key);
   }