patterncsharpMinor
High udpclient CPU use
Viewed 0 times
cpuusehighudpclient
Problem
I have a bunch of different ports I want to listen on and the iterate over the available data to send to a Dataflow pipeline. In total I'm listening on 14 ports. I'm looking for any advice on how to reduce the CPU usage of the following code.
I'm just passing in the ports to a method then adding them to a list:
I then iterate the sockets for any available data
```
Task.Run(async delegate
{
while (Receive)
{
try
{
// get any channels that have data availble
// Iterate over the the channels and s
I'm just passing in the ports to a method then adding them to a list:
public static void AddPorts(Dictionary ports)
{
try
{
var NewEndPoints = new List();
foreach (var port in ports)
{
var endpoint = new IPEndPoint(IPAddress.Any, port.Key);
NewEndPoints.Add(endpoint);
if (!Endpoints.Contains(endpoint))
{
Endpoints.Add(endpoint);
var client = new UdpClient(endpoint);
logger.Info("New Client added on port: {0}", endpoint.Port);
Clients.Add(client);
}
else
{
if (IgnoredPorts.Contains(endpoint.Port))
{
logger.Info("Existing client enabled on port: {0}", endpoint.Port);
IgnoredPorts.Remove(port.Key);
}
}
}
var differences = Endpoints.Except(NewEndPoints);
differences.ToList().ForEach(d =>
{
if (!IgnoredPorts.Contains(d.Port))
{
IgnoredPorts.Add(d.Port);
logger.Info("Client removed on port: {0}", d.Port);
}
});
}
catch (Exception ex)
{
logger.Error("Error creating udpclients", ex);
}
}I then iterate the sockets for any available data
```
Task.Run(async delegate
{
while (Receive)
{
try
{
// get any channels that have data availble
// Iterate over the the channels and s
Solution
Note that
In addition to @mjolka's description why you have a 50% CPU load, I would like to suggest how you can avoid it.
Currently you have a central place that manages all sockets in one place. I suggest to do it differently - have each endpoint handle the data on its own, and manage all sockets only as a collection of ongoing processing (tasks) like this:
Note that in this implementation the only way to stop listening the port is to get an exception on
With this approach registering and listening for all ports will be as simple as:
Endpoints.Contains(endpoint) will never be true, because you create a new IPEndPoint, and it does not implement IEquatable interface.In addition to @mjolka's description why you have a 50% CPU load, I would like to suggest how you can avoid it.
Currently you have a central place that manages all sockets in one place. I suggest to do it differently - have each endpoint handle the data on its own, and manage all sockets only as a collection of ongoing processing (tasks) like this:
public async Task RunListenerAsync(int port)
{
var endpoint = new IPEndPoint(IPAddress.Any, port);
using (var client = new UdpClient(endpoint))
{
while (true)
{
try
{
var result = await client.ReceiveAsync().ConfigureAwait(false);
ProcessEndpointData(client, result);
}
catch (Exception exc)
{
logger.Error("Error receiving from channel: " + exc.Message, exc);
return;
}
}
}
}
private void ProcessEndpointData(UdpClient client, UdpReceiveResult result)
{
var localEndPoint = (IPEndPoint)client.Client.LocalEndPoint;
var device = (from d in Ports
where d.Key == localEndPoint.Port
select d.Value).FirstOrDefault();
var data = new UdpData
{
Client = client,
Data = result.Buffer,
LocalPort = localEndPoint.Port,
LocalIP = localEndPoint.Address,
RemoteEndpoint = result.RemoteEndPoint,
Device = device
};
Flow.bufferBlock.Post(data);
}Note that in this implementation the only way to stop listening the port is to get an exception on
await client.ReceiveAsync(). You might want to introduce the CancellationToken to have a control when to stop this process.With this approach registering and listening for all ports will be as simple as:
public Task ListenPortsAsync(IEnumerable ports)
{
return Task.WhenAll(ports.Select(RunListenerAsync));
}Code Snippets
public async Task RunListenerAsync(int port)
{
var endpoint = new IPEndPoint(IPAddress.Any, port);
using (var client = new UdpClient(endpoint))
{
while (true)
{
try
{
var result = await client.ReceiveAsync().ConfigureAwait(false);
ProcessEndpointData(client, result);
}
catch (Exception exc)
{
logger.Error("Error receiving from channel: " + exc.Message, exc);
return;
}
}
}
}
private void ProcessEndpointData(UdpClient client, UdpReceiveResult result)
{
var localEndPoint = (IPEndPoint)client.Client.LocalEndPoint;
var device = (from d in Ports
where d.Key == localEndPoint.Port
select d.Value).FirstOrDefault();
var data = new UdpData
{
Client = client,
Data = result.Buffer,
LocalPort = localEndPoint.Port,
LocalIP = localEndPoint.Address,
RemoteEndpoint = result.RemoteEndPoint,
Device = device
};
Flow.bufferBlock.Post(data);
}public Task ListenPortsAsync(IEnumerable<int> ports)
{
return Task.WhenAll(ports.Select(RunListenerAsync));
}Context
StackExchange Code Review Q#82697, answer score: 4
Revisions (0)
No revisions yet.