Delete leftPad when capturing the stack before and after a frame execution

[ahamlat]

When load testing trace_block RPC method and profiling Besu, we found that leftPad method (that expands to 32 bytes) called during stack capturing before and after a frame execution takes a significant amount of time.

As padding is not necessary, deleting this call is an easy fix to improve trace namespace performance. We noticed after deleting leftPad call improvement of at least 3X, in the screenshot below we can see a performance improvement of 9X.

We can see below the profiling after deleting leftPad call in caputreStack

[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]

LGTM, local performance testing on my end validates the findings as well.

[ahamlat]

The tests on debug tracing RPC methods like debug_traceTransaction are not passing, because the stack content is part of the json output, which is not the case with tracing calls in the trace namespace. Here's an example on debug_traceTransaction :
Expected

{
      "pc" : 2,
      "op" : "CALLDATALOAD",
      "gas" : 292884,
      "gasCost" : 3,
      "depth" : 1,
      "stack" : [ "0000000000000000000000000000000000000000000000000000000000000000" ],
      "memory" : [ ],
      "storage" : null,
      "reason" : null
    }

Generated with this PR

{
      "pc" : 2,
      "op" : "CALLDATALOAD",
      "gas" : 292884,
      "gasCost" : 3,
      "depth" : 1,
      "stack" : [ "00" ],
      "memory" : [ ],
      "storage" : null,
      "reason" : null
    }

I added a fix for these use cases in the commit below.

[shemnon]

How is the performance of a41d5b4?

Have you considered changing org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.StructLog#stack to build into a StringBuilder while being attentive to packing zeros at the last possible moment instead? I think that would be more performant.

[ahamlat]

How is the performance of a41d5b4?

The performances of trace_block are the same as bebore as it is not impacted by the modication

I'm trying to get usable results on debug_traceTransaction response time (my gatling project is crashing because of the huge outputs) but at least what I can see is that org/apache/tuweni/bytes/Bytes.toFastHex seems to be the bottelneck. Also, I think there is still an optimization as we do leftPadding only once whit this PR where we do it twice before.

Have you considered changing org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.StructLog#stack to build into a StringBuilder while being attentive to packing zeros at the last possible moment instead? I think that would be more performant.

Thank you for your suggestion. However, I'm currently unable to visualize how the implementation using StringBuilder in the org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.StructLog#stack would work. Could you please provide more details or perhaps a sample code snippet to help clarify your idea?

[shemnon]

What is the wall clock time for those two traces? They are both visualized in percentage of execution time but if the top level speed is better that's a win.

Something like this

  static final String bytes32ZeroString = Bytes32.ZERO.toUnprefixedHexString();
  @JsonGetter("stack")
  public String[] stack() {
    String[] result = new String[stack.length];
    for (int i = 0; i < stack.length; i++) {
      StringBuilder sb = new StringBuilder();
      sb.append(bytes32ZeroString, 0, 62 - stack[i].length());
      sb.append(stack[i]);
      result[i] = sb.toString();
    }
    return result;
  }

However having a Byte[] in TraceFrame would likely have more speedup.

[ahamlat]

@shemnon I implemented your suggestion in this commit a5636a5, I used 64 instead of 62 in your snippet.
To have usable results in the load testing tool, I had to add debug_traceBlockByNumber to the testing tool and modify the the code to not stream the results, just process the json output and not send it back, I added some code to avoid deleting the processing code by the JIT compiler.
I compared the results between 3 versions on debug_traceBlockByNumber with a load of 1 TPS.

• Main before this PR (commit f58f6cf)
• Last commit (a5636a5) of this PR (implementation with StringBuffer)
• Commit a41d5b4 (left padding bytes)

Here're the results :

1. Main before this PR

2. Implementation with LeftPad on Bytes

**3. Implementation with StringBuffer **

From these results we can see that the last commit with StringBuilder implementation is the best implementation in terms of performance.

[macfarla]

@ahamlat is this ready to merge?

[ahamlat]

@macfarla I'm awaiting any potential feedback from Danno before proceeding with the merge. Let's wait another one or two days.

[shemnon]

Sorry, missed this one. No feedback. I should have been more attentive so you could have merged it days ago.