HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Formatting a CAN bus message as a string

Submitted by: @import:stackexchange-codereview··
0
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 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.

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.