Skip to content

Optimise precompile and create allocations #12105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flcl42 wants to merge 1 commit into
base: master
Choose a base branch
from
+54 −7
Draft

Optimise precompile and create allocations #12105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/Nethermind/Nethermind.Blockchain.Test/EthereumPrecompileProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only

using System.Collections.Frozen;
using Nethermind.Core;
using Nethermind.Evm.CodeAnalysis;
using NUnit.Framework;

namespace Nethermind.Blockchain.Test;

[TestFixture]
[Parallelizable(ParallelScope.All)]
public class EthereumPrecompileProviderTests
{
[Test]
public void GetPrecompiles_ReturnsCachedDictionary()
{
EthereumPrecompileProvider provider = new();
FrozenDictionary<AddressAsKey, CodeInfo> precompiles = provider.GetPrecompiles();
FrozenDictionary<AddressAsKey, CodeInfo> precompilesAgain = provider.GetPrecompiles();

Assert.That(precompilesAgain, Is.SameAs(precompiles));
}

@claude claude Bot Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider also asserting that two different EthereumPrecompileProvider instances return the same reference — that would demonstrate the static part of the fix (not just repeated calls on the same instance):

Suggested change
}
[Test]
public void GetPrecompiles_ReturnsCachedDictionary()
{
EthereumPrecompileProvider provider = new();
FrozenDictionary<AddressAsKey, CodeInfo> precompiles = provider.GetPrecompiles();
FrozenDictionary<AddressAsKey, CodeInfo> precompilesAgain = provider.GetPrecompiles();
Assert.That(precompilesAgain, Is.SameAs(precompiles));
}
[Test]
public void GetPrecompiles_ReturnsSameReferenceAcrossInstances()
{
EthereumPrecompileProvider first = new();
EthereumPrecompileProvider second = new();
Assert.That(second.GetPrecompiles(), Is.SameAs(first.GetPrecompiles()));
}

}
6 changes: 2 additions & 4 deletions src/Nethermind/Nethermind.Blockchain/EthereumPrecompileProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ namespace Nethermind.Blockchain;

public class EthereumPrecompileProvider() : IPrecompileProvider
{
private static FrozenDictionary<AddressAsKey, CodeInfo> Precompiles
{
get => new Dictionary<AddressAsKey, CodeInfo>
private static readonly FrozenDictionary<AddressAsKey, CodeInfo> Precompiles =

@claude claude Bot Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a genuine bug fix, not just an optimization. The old get-only property was calling ToFrozenDictionary() on a freshly-constructed Dictionary on every call to GetPrecompiles(), allocating ~20 CodeInfo wrappers and building a new frozen dictionary each time. The static readonly field initializer runs exactly once at class initialization — correct and thread-safe under CLR static-init guarantees.

new Dictionary<AddressAsKey, CodeInfo>
{
[ECRecoverPrecompile.Address] = new(ECRecoverPrecompile.Instance),
[Sha256Precompile.Address] = new(Sha256Precompile.Instance),
Expand All @@ -40,7 +39,6 @@ private static FrozenDictionary<AddressAsKey, CodeInfo> Precompiles

[SecP256r1Precompile.Address] = new(SecP256r1Precompile.Instance),
}.ToFrozenDictionary();
}

public FrozenDictionary<AddressAsKey, CodeInfo> GetPrecompiles() => Precompiles;
}
31 changes: 28 additions & 3 deletions src/Nethermind/Nethermind.Evm/VirtualMachine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public unsafe partial class VirtualMachine<TGasPolicy>(
private ReadOnlyMemory<byte> _returnDataBuffer = Array.Empty<byte>();
protected VmState<TGasPolicy> _currentState;
protected ReadOnlyMemory<byte>? _previousCallResult;
protected Address? _previousCallAddressResult;

@claude claude Bot Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The invariant is: exactly one of _previousCallAddressResult and _previousCallResult is non-null when a value needs to be pushed, never both. This is correctly maintained across all code paths:

  • PrepareCreateData (success): sets address non-null, result null
  • TryChargeAndDepositCode failure: sets result=BytesZero, address=null ✓
  • HandleRevert: sets result=FailureBytes, address=null ✓
  • HandleFailure: sets result=FailureBytes, address=null ✓
  • PrepareNextCallFrame: clears both to null ✓
  • HandleRegularReturn: sets result=status, address=null ✓

The optimization is safe: Address.Bytes returns a ReadOnlySpan<byte> over an immutable private readonly ValueAddress, so no defensive copy is needed. The pushed 20-byte span is identical to what previousState.Env.ExecutingAccount.Bytes.ToArray() produced before.

protected UInt256 _previousCallOutputDestination;

public ILogger Logger => _logger;
Expand Down Expand Up @@ -158,6 +159,7 @@ public virtual TransactionSubstate ExecuteTransaction<TTracingInst>(
_codeInfoRepository = TxExecutionContext.CodeInfoRepository;
_currentState = vmState;
_previousCallResult = null;
_previousCallAddressResult = null;
_previousCallOutputDestination = UInt256.Zero;
ZeroPaddedSpan previousCallOutput = ZeroPaddedSpan.Empty;

Expand Down Expand Up @@ -204,6 +206,7 @@ public virtual TransactionSubstate ExecuteTransaction<TTracingInst>(
{
callResult = ExecuteCall<TTracingInst>(
_previousCallResult,
_previousCallAddressResult,
previousCallOutput,
_previousCallOutputDestination);
}
Expand Down Expand Up @@ -335,7 +338,8 @@ public TransactionSubstate ExecuteTransaction(VmState<TGasPolicy> vmState, IWorl

protected void PrepareCreateData(VmState<TGasPolicy> previousState, ref ZeroPaddedSpan previousCallOutput)
{
_previousCallResult = previousState.Env.ExecutingAccount.Bytes.ToArray();
_previousCallResult = null;
_previousCallAddressResult = previousState.Env.ExecutingAccount;
_previousCallOutputDestination = UInt256.Zero;
ReturnDataBuffer = Array.Empty<byte>();
previousCallOutput = ZeroPaddedSpan.Empty;
Expand All @@ -346,6 +350,7 @@ protected ZeroPaddedSpan HandleRegularReturn<TTracingInst>(scoped in CallResult
{
ZeroPaddedSpan previousCallOutput;
ReturnDataBuffer = callResult.Output;
_previousCallAddressResult = null;
_previousCallResult = callResult.PrecompileSuccess.HasValue
? (callResult.PrecompileSuccess.Value ? StatusCode.SuccessBytes : StatusCode.FailureBytes)
: StatusCode.SuccessBytes;
Expand Down Expand Up @@ -470,6 +475,7 @@ private void TryChargeAndDepositCode(
}

_previousCallResult = BytesZero;
_previousCallAddressResult = null;
previousStateSucceeded = false;

if (_txTracer.IsTracingActions)
Expand Down Expand Up @@ -512,6 +518,7 @@ protected void HandleRevert(VmState<TGasPolicy> previousState, in CallResult cal
ReturnDataBuffer = outputBytes;

_previousCallResult = StatusCode.FailureBytes;
_previousCallAddressResult = null;

// Slice the output bytes, zero-padding if necessary, to match the expected output length.
// This ensures that the returned data conforms to the caller's output length expectations.
Expand Down Expand Up @@ -590,6 +597,7 @@ protected TransactionSubstate HandleFailure<TTracingInst>(Exception failure, str
}

_previousCallResult = StatusCode.FailureBytes;
_previousCallAddressResult = null;
bool failedCreate = _currentState.ExecutionType.IsAnyCreate();

// Reset output destination and return data.
Expand Down Expand Up @@ -722,6 +730,7 @@ protected void PrepareNextCallFrame(in CallResult callResult, ref ZeroPaddedSpan

// Clear the previous call result as the execution context is moving to a new frame.
_previousCallResult = null;
_previousCallAddressResult = null;

// Reset the return data buffer to ensure no residual data persists across call frames.
ReturnDataBuffer = Array.Empty<byte>();
Expand Down Expand Up @@ -772,6 +781,7 @@ protected TransactionSubstate HandleException(scoped in CallResult callResult, s
}

_previousCallResult = StatusCode.FailureBytes;
_previousCallAddressResult = null;
bool failedCreate = _currentState.ExecutionType.IsAnyCreate();

// Reset output destination and clear return data.
Expand Down Expand Up @@ -1091,6 +1101,9 @@ protected static string GetErrorString(IPrecompile precompile, string? error)
/// An optional read-only memory buffer containing the output of a previous call; if provided, its bytes
/// will be pushed onto the stack for further processing.
/// </param>
/// <param name="previousCallAddressResult">
/// An optional contract creation result address to push onto the stack.
/// </param>
/// <param name="previousCallOutput">
/// A zero-padded span containing output from the previous call used for updating the memory state.
/// </param>
Expand All @@ -1108,6 +1121,7 @@ protected static string GetErrorString(IPrecompile precompile, string? error)
[SkipLocalsInit]
protected CallResult ExecuteCall<TTracingInst>(
ReadOnlyMemory<byte>? previousCallResult,
Address? previousCallAddressResult,
ZeroPaddedSpan previousCallOutput,
scoped in UInt256 previousCallOutputDestination)
where TTracingInst : struct, IFlag
Expand Down Expand Up @@ -1157,8 +1171,19 @@ protected CallResult ExecuteCall<TTracingInst>(
// gas/state-gas accounting without needing interpreter-wide exception handling.
ref TGasPolicy gas = ref vmState.Gas;

// If a previous call result exists, push its bytes onto the stack.
if (previousCallResult is not null)
// If a previous call or create result exists, push its bytes onto the stack.
if (previousCallAddressResult is not null)
{
EvmExceptionType pushResult = stack.PushBytes<TTracingInst>(previousCallAddressResult.Bytes);
if (pushResult != EvmExceptionType.None) return new(pushResult);

// Report the remaining gas if tracing instructions are enabled.
if (TTracingInst.IsActive)
{
_txTracer.ReportOperationRemainingGas(TGasPolicy.GetRemainingGas(vmState.Gas));
}
}
else if (previousCallResult is not null)
{
EvmExceptionType pushResult = stack.PushBytes<TTracingInst>(previousCallResult.Value.Span);
if (pushResult != EvmExceptionType.None) return new(pushResult);
Expand Down
Loading