-
Notifications
You must be signed in to change notification settings - Fork 469
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.
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
Hunting allocations 3 - introduce AccountStruct #6605
Changes from all commits
def608f
bb20f01
351d35d
5d5af51
7835b2d
0e25bda
4941979
43db963
69c1ab8
eab6aac
4cc6c14
3cac4e1
e5c8d6a
7a09616
da4909b
90addd4
243214d
09f2442
ad58087
d1b62c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ namespace Nethermind.Core | |
{ | ||
public class Account | ||
{ | ||
public static Account TotallyEmpty = new(); | ||
public static readonly Account TotallyEmpty = new(); | ||
|
||
private readonly Hash256? _codeHash; | ||
private readonly Hash256? _storageRoot; | ||
|
@@ -100,5 +100,44 @@ public Account WithChangedCodeHash(Hash256 newCodeHash) | |
{ | ||
return new(newCodeHash, this); | ||
} | ||
|
||
public AccountStruct ToStruct() => new(Nonce, Balance, StorageRoot, CodeHash); | ||
} | ||
|
||
public readonly struct AccountStruct | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a 128 bytes struct (0.125 kB); want to make sure are passing by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How this comment ended? I see that in lot of places this is returned, no |
||
{ | ||
private readonly ValueHash256 _codeHash = Keccak.OfAnEmptyString.ValueHash256; | ||
private readonly ValueHash256 _storageRoot = Keccak.EmptyTreeHash.ValueHash256; | ||
|
||
public AccountStruct(in UInt256 nonce, in UInt256 balance, in ValueHash256 storageRoot, in ValueHash256 codeHash) | ||
{ | ||
_codeHash = codeHash; | ||
_storageRoot = storageRoot; | ||
Nonce = nonce; | ||
Balance = balance; | ||
} | ||
|
||
public AccountStruct(in UInt256 nonce, in UInt256 balance) | ||
{ | ||
Nonce = nonce; | ||
Balance = balance; | ||
} | ||
|
||
public AccountStruct(in UInt256 balance) | ||
{ | ||
Balance = balance; | ||
} | ||
|
||
public bool HasCode => _codeHash != Keccak.OfAnEmptyString.ValueHash256; | ||
|
||
public bool HasStorage => _storageRoot != Keccak.EmptyTreeHash.ValueHash256; | ||
|
||
public UInt256 Nonce { get; } | ||
public UInt256 Balance { get; } | ||
public ValueHash256 StorageRoot => _storageRoot; | ||
public ValueHash256 CodeHash => _codeHash; | ||
public bool IsTotallyEmpty => _storageRoot == Keccak.EmptyTreeHash.ValueHash256 && IsEmpty; | ||
public bool IsEmpty => _codeHash == Keccak.OfAnEmptyString.ValueHash256 && Balance.IsZero && Nonce.IsZero; | ||
public bool IsContract => _codeHash != Keccak.OfAnEmptyString.ValueHash256; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// SPDX-FileCopyrightText: 2024 Demerzel Solutions Limited | ||
// SPDX-License-Identifier: LGPL-3.0-only | ||
|
||
using System; | ||
using System.Text.Json; | ||
using System.Text.Json.Serialization; | ||
using Nethermind.Core.Crypto; | ||
|
||
namespace Nethermind.Serialization.Json; | ||
|
||
public class ValueHash256Converter : JsonConverter<ValueHash256> | ||
{ | ||
public override ValueHash256 Read( | ||
ref Utf8JsonReader reader, | ||
Type typeToConvert, | ||
JsonSerializerOptions options) | ||
{ | ||
byte[]? bytes = ByteArrayConverter.Convert(ref reader); | ||
return bytes is null ? null : new ValueHash256(bytes); | ||
} | ||
|
||
public override void Write( | ||
Utf8JsonWriter writer, | ||
ValueHash256 keccak, | ||
JsonSerializerOptions options) | ||
{ | ||
ByteArrayConverter.Convert(writer, keccak.Bytes, skipLeadingZeros: false); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -162,7 +162,7 @@ public ResultWrapper<IEnumerable<Address>> eth_accounts() | |
return Task.FromResult(GetStateFailureResult<UInt256?>(header)); | ||
} | ||
|
||
Account account = _stateReader.GetAccount(header!.StateRoot!, address); | ||
AccountStruct? account = _stateReader.GetAccount(header!.StateRoot!, address); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it have to be nullable? Extra byte it adds is problematic on copy size (128 bytes -> 129 bytes) better to init with empy code hash and empty storage has and pass that back if null? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So been thinking about it, how to avoid copies to. Maybe the answer is something like:
not sure if we could write directly to stack allocated by the calling method? |
||
return Task.FromResult(ResultWrapper<UInt256?>.Success(account?.Balance ?? UInt256.Zero)); | ||
} | ||
|
||
|
@@ -200,7 +200,7 @@ public Task<ResultWrapper<UInt256>> eth_getTransactionCount(Address address, Blo | |
return Task.FromResult(GetStateFailureResult<UInt256>(header)); | ||
} | ||
|
||
Account account = _stateReader.GetAccount(header!.StateRoot!, address); | ||
AccountStruct? account = _stateReader.GetAccount(header!.StateRoot!, address); | ||
UInt256 nonce = account?.Nonce ?? 0; | ||
|
||
return Task.FromResult(ResultWrapper<UInt256>.Success(nonce)); | ||
|
@@ -257,14 +257,8 @@ public ResultWrapper<byte[]> eth_getCode(Address address, BlockParameter? blockP | |
return GetStateFailureResult<byte[]>(header); | ||
} | ||
|
||
Account account = _stateReader.GetAccount(header!.StateRoot!, address); | ||
if (account is null) | ||
{ | ||
return ResultWrapper<byte[]>.Success(Array.Empty<byte>()); | ||
} | ||
|
||
byte[]? code = _stateReader.GetCode(account.CodeHash); | ||
return ResultWrapper<byte[]>.Success(code); | ||
AccountStruct? account = _stateReader.GetAccount(header!.StateRoot!, address); | ||
return ResultWrapper<byte[]>.Success(account is null ? Array.Empty<byte>() : _stateReader.GetCode(account.Value.CodeHash)); | ||
} | ||
|
||
public ResultWrapper<byte[]> eth_sign(Address addressData, byte[] message) | ||
|
@@ -701,22 +695,22 @@ private static IEnumerable<FilterLog> GetLogs(IEnumerable<FilterLog> logs, Cance | |
} | ||
} | ||
|
||
public ResultWrapper<AccountForRpc> eth_getAccount(Address accountAddress, BlockParameter? blockParameter) | ||
public ResultWrapper<AccountForRpc?> eth_getAccount(Address accountAddress, BlockParameter? blockParameter) | ||
{ | ||
SearchResult<BlockHeader> searchResult = _blockFinder.SearchForHeader(blockParameter); | ||
if (searchResult.IsError) | ||
{ | ||
return GetFailureResult<AccountForRpc, BlockHeader>(searchResult, _ethSyncingInfo.SyncMode.HaveNotSyncedHeadersYet()); | ||
return GetFailureResult<AccountForRpc?, BlockHeader>(searchResult, _ethSyncingInfo.SyncMode.HaveNotSyncedHeadersYet()); | ||
} | ||
|
||
BlockHeader header = searchResult.Object; | ||
if (!HasStateForBlock(_blockchainBridge, header!)) | ||
{ | ||
return GetStateFailureResult<AccountForRpc>(header); | ||
return GetStateFailureResult<AccountForRpc?>(header); | ||
} | ||
|
||
Account account = _stateReader.GetAccount(header!.StateRoot!, accountAddress); | ||
return ResultWrapper<AccountForRpc>.Success(account is null ? null : new AccountForRpc(account)); | ||
AccountStruct? account = _stateReader.GetAccount(header!.StateRoot!, accountAddress); | ||
return ResultWrapper<AccountForRpc?>.Success(account is null ? null : new AccountForRpc(account.Value)); | ||
} | ||
|
||
private static ResultWrapper<TResult> GetFailureResult<TResult, TSearch>(SearchResult<TSearch> searchResult, bool isTemporary) where TSearch : class => | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah still exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are places where we need to put it on heap, not sure if worth paying for boxing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But will try to move some usages to AccountStruct in next PR.