Skip to content

Commit bf0206d

Browse files
benaadamsbenaadams
authored
Remove some closure allocations (#7953)
1 parent 372e6e7 commit bf0206d

File tree

15 files changed

+164
-131
lines changed

15 files changed

+164
-131
lines changed

‎src/Nethermind/Nethermind.Core/Crypto/PublicKey.cs

+1-13
Original file line numberDiff line numberDiff line change
@@ -70,19 +70,7 @@ public Hash256 Hash
7070

7171
public byte[] Bytes { get; }
7272

73-
public byte[] PrefixedBytes
74-
{
75-
get
76-
{
77-
if (_prefixedBytes is null)
78-
{
79-
return LazyInitializer.EnsureInitialized(ref _prefixedBytes,
80-
() => Core.Extensions.Bytes.Concat(0x04, Bytes));
81-
}
82-
83-
return _prefixedBytes;
84-
}
85-
}
73+
public byte[] PrefixedBytes => _prefixedBytes ??= Core.Extensions.Bytes.Concat(0x04, Bytes);
8674

8775
public bool Equals(PublicKey? other) => other is not null && Core.Extensions.Bytes.AreEqual(Bytes, other.Bytes);
8876

‎src/Nethermind/Nethermind.Db/SimpleFilePublicKeyDb.cs

+22-5
Original file line numberDiff line numberDiff line change
@@ -65,20 +65,37 @@ public void Set(ReadOnlySpan<byte> key, byte[]? value, WriteFlags flags = WriteF
6565
{
6666
if (value is null)
6767
{
68-
_cacheSpan.TryRemove(key, out _);
68+
if (_cacheSpan.TryRemove(key, out _))
69+
{
70+
_hasPendingChanges = true;
71+
}
72+
return;
73+
}
74+
75+
bool setValue = true;
76+
if (_cacheSpan.TryGetValue(key, out var existingValue))
77+
{
78+
if (!Bytes.AreEqual(existingValue, value))
79+
{
80+
setValue = false;
81+
}
6982
}
70-
else
83+
84+
if (setValue)
7185
{
72-
_cache.AddOrUpdate(key.ToArray(), newValue => Add(value), (x, oldValue) => Update(oldValue, value));
86+
_cacheSpan[key] = value;
87+
_hasPendingChanges = true;
7388
}
7489
}
7590

7691
public KeyValuePair<byte[], byte[]>[] this[byte[][] keys] => keys.Select(k => new KeyValuePair<byte[], byte[]>(k, _cache.TryGetValue(k, out var value) ? value : null)).ToArray();
7792

7893
public void Remove(ReadOnlySpan<byte> key)
7994
{
80-
_hasPendingChanges = true;
81-
_cacheSpan.TryRemove(key, out _);
95+
if (_cacheSpan.TryRemove(key, out _))
96+
{
97+
_hasPendingChanges = true;
98+
}
8299
}
83100

84101
public bool KeyExists(ReadOnlySpan<byte> key)

‎src/Nethermind/Nethermind.Network.Discovery/DiscoveryManager.cs

+24-13
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Collections.Concurrent;
55
using System.Diagnostics.CodeAnalysis;
6+
using System.Runtime.CompilerServices;
67
using Nethermind.Config;
78
using Nethermind.Core;
89
using Nethermind.Core.Crypto;
@@ -25,6 +26,8 @@ public class DiscoveryManager : IDiscoveryManager
2526
private readonly INetworkStorage _discoveryStorage;
2627

2728
private readonly ConcurrentDictionary<MessageTypeKey, TaskCompletionSource<DiscoveryMsg>> _waitingEvents = new();
29+
private readonly Func<Hash256, Node, INodeLifecycleManager> _createNodeLifecycleManager;
30+
private readonly Func<Hash256, Node, INodeLifecycleManager> _createNodeLifecycleManagerPersisted;
2831
private IMsgSender? _msgSender;
2932

3033
public DiscoveryManager(
@@ -41,6 +44,24 @@ public DiscoveryManager(
4144
_discoveryStorage = discoveryStorage ?? throw new ArgumentNullException(nameof(discoveryStorage));
4245
_nodeLifecycleManagerFactory.DiscoveryManager = this;
4346
_outgoingMessageRateLimiter = new RateLimiter(discoveryConfig.MaxOutgoingMessagePerSecond);
47+
_createNodeLifecycleManager = GetLifecycleManagerFunc(isPersisted: false);
48+
_createNodeLifecycleManagerPersisted = GetLifecycleManagerFunc(isPersisted: true);
49+
}
50+
51+
private Func<Hash256, Node, INodeLifecycleManager> GetLifecycleManagerFunc(bool isPersisted)
52+
{
53+
return (_, node) =>
54+
{
55+
Interlocked.Increment(ref _managersCreated);
56+
INodeLifecycleManager manager = _nodeLifecycleManagerFactory.CreateNodeLifecycleManager(node);
57+
manager.OnStateChanged += ManagerOnOnStateChanged;
58+
if (!isPersisted)
59+
{
60+
_discoveryStorage.UpdateNodes(new[] { new NetworkNode(manager.ManagedNode.Id, manager.ManagedNode.Host, manager.ManagedNode.Port, manager.NodeStats.NewPersistedNodeReputation(DateTime.UtcNow)) });
61+
}
62+
63+
return manager;
64+
};
4465
}
4566

4667
public IMsgSender MsgSender
@@ -120,18 +141,7 @@ public void OnIncomingMsg(DiscoveryMsg msg)
120141
return null;
121142
}
122143

123-
return _nodeLifecycleManagers.GetOrAdd(node.IdHash, _ =>
124-
{
125-
Interlocked.Increment(ref _managersCreated);
126-
INodeLifecycleManager manager = _nodeLifecycleManagerFactory.CreateNodeLifecycleManager(node);
127-
manager.OnStateChanged += ManagerOnOnStateChanged;
128-
if (!isPersisted)
129-
{
130-
_discoveryStorage.UpdateNodes(new[] { new NetworkNode(manager.ManagedNode.Id, manager.ManagedNode.Host, manager.ManagedNode.Port, manager.NodeStats.NewPersistedNodeReputation(DateTime.UtcNow)) });
131-
}
132-
133-
return manager;
134-
});
144+
return _nodeLifecycleManagers.GetOrAdd(node.IdHash, isPersisted ? _createNodeLifecycleManagerPersisted : _createNodeLifecycleManager, node);
135145
}
136146

137147
private void ManagerOnOnStateChanged(object? sender, NodeLifecycleState e)
@@ -168,7 +178,8 @@ public async Task SendMessageAsync(DiscoveryMsg discoveryMsg)
168178
}
169179
}
170180

171-
public async Task<bool> WasMessageReceived(Hash256 senderIdHash, MsgType msgType, int timeout)
181+
[AsyncMethodBuilder(typeof(PoolingAsyncValueTaskMethodBuilder<>))]
182+
public async ValueTask<bool> WasMessageReceived(Hash256 senderIdHash, MsgType msgType, int timeout)
172183
{
173184
TaskCompletionSource<DiscoveryMsg> completionSource = GetCompletionSource(senderIdHash, (int)msgType);
174185
CancellationTokenSource delayCancellation = new();

‎src/Nethermind/Nethermind.Network.Discovery/IDiscoveryManager.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public interface IDiscoveryManager : IDiscoveryMsgListener
1414
INodeLifecycleManager? GetNodeLifecycleManager(Node node, bool isPersisted = false);
1515
void SendMessage(DiscoveryMsg discoveryMsg);
1616
Task SendMessageAsync(DiscoveryMsg discoveryMsg);
17-
Task<bool> WasMessageReceived(Hash256 senderIdHash, MsgType msgType, int timeout);
17+
ValueTask<bool> WasMessageReceived(Hash256 senderIdHash, MsgType msgType, int timeout);
1818
event EventHandler<NodeEventArgs> NodeDiscovered;
1919

2020
IReadOnlyCollection<INodeLifecycleManager> GetNodeLifecycleManagers();

‎src/Nethermind/Nethermind.Network.Discovery/NettyDiscoveryHandler.cs

+7-7
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,14 @@ public async Task SendMsg(DiscoveryMsg discoveryMsg)
9797
}
9898

9999
IAddressedEnvelope<IByteBuffer> packet = new DatagramPacket(msgBuffer, discoveryMsg.FarAddress);
100-
101-
await _channel.WriteAndFlushAsync(packet).ContinueWith(t =>
100+
try
102101
{
103-
if (t.IsFaulted)
104-
{
105-
if (_logger.IsTrace) _logger.Trace($"Error when sending a discovery message Msg: {discoveryMsg} ,Exp: {t.Exception}");
106-
}
107-
});
102+
await _channel.WriteAndFlushAsync(packet);
103+
}
104+
catch (Exception e)
105+
{
106+
if (_logger.IsTrace) _logger.Trace($"Error when sending a discovery message Msg: {discoveryMsg} ,Exp: {e}");
107+
}
108108

109109
Interlocked.Add(ref Metrics.DiscoveryBytesSent, size);
110110
}

‎src/Nethermind/Nethermind.Network.Discovery/Serializers/NeighborsMsgSerializer.cs

+19-17
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,22 @@ namespace Nethermind.Network.Discovery.Serializers;
1313

1414
public class NeighborsMsgSerializer : DiscoveryMsgSerializerBase, IZeroInnerMessageSerializer<NeighborsMsg>
1515
{
16+
private static readonly Func<RlpStream, Node> _decodeItem = static ctx =>
17+
{
18+
int lastPosition = ctx.ReadSequenceLength() + ctx.Position;
19+
int count = ctx.PeekNumberOfItemsRemaining(lastPosition);
20+
21+
ReadOnlySpan<byte> ip = ctx.DecodeByteArraySpan();
22+
IPEndPoint address = GetAddress(ip, ctx.DecodeInt());
23+
if (count > 3)
24+
{
25+
ctx.DecodeInt();
26+
}
27+
28+
ReadOnlySpan<byte> id = ctx.DecodeByteArraySpan();
29+
return new Node(new PublicKey(id), address);
30+
};
31+
1632
public NeighborsMsgSerializer(IEcdsa ecdsa,
1733
IPrivateKeyGenerator nodeKey,
1834
INodeIdResolver nodeIdResolver) : base(ecdsa, nodeKey, nodeIdResolver)
@@ -53,30 +69,16 @@ public NeighborsMsg Deserialize(IByteBuffer msgBytes)
5369

5470
NettyRlpStream rlp = new(Data);
5571
rlp.ReadSequenceLength();
56-
Node[] nodes = DeserializeNodes(rlp) as Node[];
72+
Node[] nodes = DeserializeNodes(rlp);
5773

5874
long expirationTime = rlp.DecodeLong();
5975
NeighborsMsg msg = new(FarPublicKey, expirationTime, nodes);
6076
return msg;
6177
}
6278

63-
private static Node?[] DeserializeNodes(RlpStream rlpStream)
79+
private static Node[] DeserializeNodes(RlpStream rlpStream)
6480
{
65-
return rlpStream.DecodeArray(ctx =>
66-
{
67-
int lastPosition = ctx.ReadSequenceLength() + ctx.Position;
68-
int count = ctx.PeekNumberOfItemsRemaining(lastPosition);
69-
70-
ReadOnlySpan<byte> ip = ctx.DecodeByteArraySpan();
71-
IPEndPoint address = GetAddress(ip, ctx.DecodeInt());
72-
if (count > 3)
73-
{
74-
ctx.DecodeInt();
75-
}
76-
77-
ReadOnlySpan<byte> id = ctx.DecodeByteArraySpan();
78-
return new Node(new PublicKey(id), address);
79-
});
81+
return rlpStream.DecodeArray<Node>(_decodeItem);
8082
}
8183

8284
private static int GetNodesLength(Node[] nodes, out int contentLength)

‎src/Nethermind/Nethermind.Network.Test/ProtocolsManagerTests.cs

-7
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// SPDX-License-Identifier: LGPL-3.0-only
33

44
using System.Numerics;
5-
using System.Threading;
65
using DotNetty.Buffers;
76
using DotNetty.Transport.Channels;
87
using Nethermind.Blockchain;
@@ -184,12 +183,6 @@ public Context ReceiveDisconnect()
184183
return this;
185184
}
186185

187-
public Context Wait(int i)
188-
{
189-
Thread.Sleep(i);
190-
return this;
191-
}
192-
193186
public Context VerifyInitialized()
194187
{
195188
Assert.That(_currentSession.State, Is.EqualTo(SessionState.Initialized));

‎src/Nethermind/Nethermind.Network/P2P/ProtocolHandlers/P2PProtocolHandler.cs

+1-7
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,7 @@ public override void Init()
7979

8080
// We are expecting to receive Hello message anytime from the handshake completion,
8181
// irrespective of sending Hello from our side
82-
CheckProtocolInitTimeout().ContinueWith(x =>
83-
{
84-
if (x.IsFaulted && Logger.IsError)
85-
{
86-
Logger.Error("Error during p2pProtocol handler timeout logic", x.Exception);
87-
}
88-
});
82+
_ = CheckProtocolInitTimeout();
8983
}
9084

9185
public override void HandleMessage(Packet msg)

‎src/Nethermind/Nethermind.Network/P2P/ProtocolHandlers/ProtocolHandlerBase.cs

+21-11
Original file line numberDiff line numberDiff line change
@@ -93,22 +93,32 @@ protected internal void Send<T>(T message) where T : P2PMessage
9393

9494
protected async Task CheckProtocolInitTimeout()
9595
{
96-
Task<MessageBase> receivedInitMsgTask = _initCompletionSource.Task;
97-
CancellationTokenSource delayCancellation = new();
98-
Task firstTask = await Task.WhenAny(receivedInitMsgTask, Task.Delay(InitTimeout, delayCancellation.Token));
99-
100-
if (firstTask != receivedInitMsgTask)
96+
try
10197
{
102-
if (Logger.IsTrace)
98+
Task<MessageBase> receivedInitMsgTask = _initCompletionSource.Task;
99+
CancellationTokenSource delayCancellation = new();
100+
Task firstTask = await Task.WhenAny(receivedInitMsgTask, Task.Delay(InitTimeout, delayCancellation.Token));
101+
102+
if (firstTask != receivedInitMsgTask)
103103
{
104-
Logger.Trace($"Disconnecting due to timeout for protocol init message ({Name}): {Session.RemoteNodeId}");
105-
}
104+
if (Logger.IsTrace)
105+
{
106+
Logger.Trace($"Disconnecting due to timeout for protocol init message ({Name}): {Session.RemoteNodeId}");
107+
}
106108

107-
Session.InitiateDisconnect(DisconnectReason.ProtocolInitTimeout, "protocol init timeout");
109+
Session.InitiateDisconnect(DisconnectReason.ProtocolInitTimeout, "protocol init timeout");
110+
}
111+
else
112+
{
113+
delayCancellation.Cancel();
114+
}
108115
}
109-
else
116+
catch (Exception e)
110117
{
111-
delayCancellation.Cancel();
118+
if (Logger.IsError)
119+
{
120+
Logger.Error("Error during p2pProtocol handler timeout logic", e);
121+
}
112122
}
113123
}
114124

‎src/Nethermind/Nethermind.Network/P2P/Session.cs

+36-27
Original file line numberDiff line numberDiff line change
@@ -487,33 +487,7 @@ public void MarkDisconnected(DisconnectReason disconnectReason, DisconnectType d
487487

488488
Disconnecting?.Invoke(this, new DisconnectEventArgs(disconnectReason, disconnectType, details));
489489

490-
//Possible in case of disconnect before p2p initialization
491-
if (_context is null)
492-
{
493-
//in case pipeline did not get to p2p - no disconnect delay
494-
_channel.DisconnectAsync().ContinueWith(x =>
495-
{
496-
if (x.IsFaulted && _logger.IsTrace)
497-
_logger.Trace($"Error while disconnecting on channel on {this} : {x.Exception}");
498-
});
499-
}
500-
else
501-
{
502-
Task delayTask =
503-
disconnectType == DisconnectType.Local
504-
? Task.Delay(Timeouts.Disconnection)
505-
: Task.CompletedTask;
506-
delayTask.ContinueWith(t =>
507-
{
508-
if (_logger.IsTrace)
509-
_logger.Trace($"{this} disconnecting now after {Timeouts.Disconnection.TotalMilliseconds} milliseconds");
510-
_context.DisconnectAsync().ContinueWith(x =>
511-
{
512-
if (x.IsFaulted && _logger.IsTrace)
513-
_logger.Trace($"Error while disconnecting on context on {this} : {x.Exception}");
514-
});
515-
});
516-
}
490+
_ = DisconnectAsync(disconnectType);
517491

518492
lock (_sessionStateLock)
519493
{
@@ -530,6 +504,41 @@ public void MarkDisconnected(DisconnectReason disconnectReason, DisconnectType d
530504
_logger.Error($"DEBUG/ERROR No subscriptions for session disconnected event on {this}");
531505
}
532506

507+
private async Task DisconnectAsync(DisconnectType disconnectType)
508+
{
509+
//Possible in case of disconnect before p2p initialization
510+
if (_context is null)
511+
{
512+
//in case pipeline did not get to p2p - no disconnect delay
513+
try
514+
{
515+
await _channel.DisconnectAsync();
516+
}
517+
catch (Exception e)
518+
{
519+
if (_logger.IsTrace)
520+
_logger.Trace($"Error while disconnecting on context on {this} : {e}");
521+
}
522+
}
523+
else
524+
{
525+
if (disconnectType == DisconnectType.Local)
526+
{
527+
await Task.Delay(Timeouts.Disconnection);
528+
}
529+
530+
try
531+
{
532+
await _context.DisconnectAsync();
533+
}
534+
catch (Exception e)
535+
{
536+
if (_logger.IsTrace)
537+
_logger.Trace($"Error while disconnecting on context on {this} : {e}");
538+
}
539+
}
540+
}
541+
533542
public event EventHandler<DisconnectEventArgs> Disconnecting;
534543
public event EventHandler<DisconnectEventArgs> Disconnected;
535544
public event EventHandler<EventArgs> HandshakeComplete;

0 commit comments

Comments
 (0)