Formatting a CAN bus message as a string

Question

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<PassThruMsg[]> messages, BlockingCollection<string[]> formattedMessages)
{
    try
    {
        PassThruMsg msg;
        String[] formatted;
        foreach (var item in messages.GetConsumingEnumerable(_cancellationTokenSource.Token))
        {
            formatted = new string[item.Length];
            for (uint i = 0; i < item.Length; i++)
            {
                msg = item[i];

                if (_firstTime == 0) _firstTime = msg.timestamp;

                formatted[i] = ((msg.timestamp - _firstTime) / 1000000.0).ToString("0.000000 ");

                // Assumed CAN Protocol
                //TODO:
                string canID = BitConverter.ToString(msg.data, 0, 4).Replace("-", "").TrimStart('0');
                string data = msg.dataSize > 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:X})", status));
    }
}

And the last node:

private void LogMessages(BlockingCollection<string[]> messages)
{
    try
    {
        foreach (var item in messages.GetConsumingEnumerable(_cancellationTokenSource.Token))
        {
            if (item.Length > 0)
            {
                _fileWriter.WriteLine(String.Join(Environment.NewLine, item));
                _fileWriter.Flush();
            }
        }
    }
    catch (OperationCanceledException)
    {
        //TODO:
    }
}

I have looked this over several times and have been working on it for quite a while, but cannot find any places where this is bottle necking in a way that I can fix. I am just hoping a second set of eyes to review this will see performance issues that I don't.

Answer 1

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.