patterncsharpMinor
Formatting a CAN bus message as a string
Viewed 0 times
formattingcanmessagebusstring
Problem
I am writing a CAN bus logger application and need to format messages as quick as possible in order to prevent buffer overflows on our device. In some cases I am quick enough, but under heavy loads it is still pretty slow. I believe the bulk of the problem happens in my formatting block (I am using TPL to separate the tasks as much as possible). Here is my consumer block which takes a
If it is relevant, here is the snippet where I pass into the BlockingCollection
```
uint numMsgs;
while (!_cancellationTokenSource.IsCancellationRequested)
{
numMsgs = (uint)msgs.Length;
status = j2534.PassThruReadMsgs(channelID, msgs, ref numMsgs, 1000);
PassThruMsg[] copiedMessages = new PassThruMsg[numMsgs];
Array.Copy(msgs, copiedMessages, numMsgs);
if (status == Status.ERR_BUFFER_OVERFLOW)
{
_fileWriter.WriteLine("DEVICE INDICATED BUFFER OVERFLOW - MESSAGES LOST!");
logMessages.Add(copiedMessages);
}
else if (status == Status.STATUS_NOERROR || status == Status.ERR_TIMEOUT)
{
logMessages.Add(copiedMessages);
}
else if (status == Status.ERR_BUFFER_EMPTY)
{
throw new Exception(String.Format("PassThruReadMsgs Failed(0x{0
PassThruMsg[] blocking collection, formats those messages, and passes them into a new blocking collection where they are later printed to a file:private void FormatMessages(BlockingCollection messages, BlockingCollection formattedMessages)
{
try
{
PassThruMsg msg;
String[] formatted;
foreach (var item in messages.GetConsumingEnumerable(_cancellationTokenSource.Token))
{
formatted = new string[item.Length];
for (uint i = 0; i 4 ? BitConverter.ToString(msg.data, 4, (int)msg.dataSize - 4).Replace("-", " ") : String.Empty;
formatted[i] += canID + " " + data;
}
formattedMessages.Add(formatted);
}
}
catch (OperationCanceledException)
{
//TODO:
}
}If it is relevant, here is the snippet where I pass into the BlockingCollection
```
uint numMsgs;
while (!_cancellationTokenSource.IsCancellationRequested)
{
numMsgs = (uint)msgs.Length;
status = j2534.PassThruReadMsgs(channelID, msgs, ref numMsgs, 1000);
PassThruMsg[] copiedMessages = new PassThruMsg[numMsgs];
Array.Copy(msgs, copiedMessages, numMsgs);
if (status == Status.ERR_BUFFER_OVERFLOW)
{
_fileWriter.WriteLine("DEVICE INDICATED BUFFER OVERFLOW - MESSAGES LOST!");
logMessages.Add(copiedMessages);
}
else if (status == Status.STATUS_NOERROR || status == Status.ERR_TIMEOUT)
{
logMessages.Add(copiedMessages);
}
else if (status == Status.ERR_BUFFER_EMPTY)
{
throw new Exception(String.Format("PassThruReadMsgs Failed(0x{0
Solution
I would start with extracting the message formatting logic to a separate method. It will make profiling and performance testing easier.
Then you can check the FormatSingleMessage method independently to see if the problem is really there.
When you format the messages, you perform several operations on strings. Strings are immutable, so each operation creates a new string on the heap. All these strings have to be garbage collected, which is another expensive operation.
Try to use a
foreach (var item in messages.GetConsumingEnumerable(_cancellationTokenSource.Token))
{
formatted = FormatSingleMessage(item);
formattedMessages.Add(formatted);
}Then you can check the FormatSingleMessage method independently to see if the problem is really there.
When you format the messages, you perform several operations on strings. Strings are immutable, so each operation creates a new string on the heap. All these strings have to be garbage collected, which is another expensive operation.
Try to use a
StringBuilder instead and compare performance of both approaches. Depending on the size of the messages, you might see a big improvement.Code Snippets
foreach (var item in messages.GetConsumingEnumerable(_cancellationTokenSource.Token))
{
formatted = FormatSingleMessage(item);
formattedMessages.Add(formatted);
}Context
StackExchange Code Review Q#106379, answer score: 2
Revisions (0)
No revisions yet.