Как я могу улучшить этот неэффективный, ужасный код последовательного порта?

Question

У меня некрасивый кусок кода последовательного порта, который очень нестабилен.

void port_DataReceived(object sender, SerialDataReceivedEventArgs e)
{
    Thread.Sleep(100);
    while (port.BytesToRead > 0)
    {
        var count = port.BytesToRead;

        byte[] buffer = new byte[count];

        var read = port.Read(buffer, 0, count);

        if (DataEncapsulator != null)
            buffer = DataEncapsulator.UnWrap(buffer);


       var response = dataCollector.Collect(buffer);

       if (response != null)
       {
           this.OnDataReceived(response);
       }

       Thread.Sleep(100);
    }    
}

Если я удаляю вызовы Thread.Sleep (100), код перестает работать.

Конечно, это действительно тормозит, и если много потоков данных, он тоже перестает работать, если я не заставлю спать еще больше. (Перестает работать как в чистом тупике)

Обратите внимание, что DataEncapsulator и DataCollector являются компонентами предоставлено MEF, но их производительность довольно хорошая.

В классе есть метод Listen (), который запускает фонового работника в получить данные.

public void Listen(IDataCollector dataCollector)
{
    this.dataCollector = dataCollector;
    BackgroundWorker worker = new BackgroundWorker();

    worker.DoWork += new DoWorkEventHandler(worker_DoWork);
    worker.RunWorkerAsync();
}

void worker_DoWork(object sender, DoWorkEventArgs e)
{
    port = new SerialPort();

    //Event handlers
    port.ReceivedBytesThreshold = 15;
    port.DataReceived += new SerialDataReceivedEventHandler(port_DataReceived);

    ..... remainder of code ...

Предложения приветствуются!

Обновление: * Просто краткая заметка о том, что делают классы IDataCollector. Невозможно узнать, все ли байты данных были отправлены читаются в одной операции чтения. Таким образом, каждый раз, когда данные читаются, это передается в DataColllector, который возвращает true, когда завершено и действительное сообщение протокола было получено. В данном случае это просто проверяет байт синхронизации, длину, crc и хвостовой байт. Настоящая работа делается позже другими классами. *

Обновление 2: Я заменил код сейчас, как предложено, но все же что-то не так:

void port_DataReceived(object sender, SerialDataReceivedEventArgs e)
{
        var count = port.BytesToRead;

        byte[] buffer = new byte[count];

        var read = port.Read(buffer, 0, count);

        if (DataEncapsulator != null)
            buffer = DataEncapsulator.UnWrap(buffer);

        var response = dataCollector.Collect(buffer);

        if (response != null)
        {
            this.OnDataReceived(response);
        }     
}

Вы видите, что это работает нормально с быстрым и стабильным соединением. Но OnDataReceived НЕ вызывается при каждом получении данных. (Подробнее смотрите в документах MSDN). Так что, если данные становятся фрагментированными и вы читаете только один раз в случае потери данных.

И теперь я помню, почему у меня была петля во-первых, потому что на самом деле он должен читать несколько раз, если соединение медленное или нестабильное.

Очевидно, я не могу вернуться к решению цикла while, что я могу сделать?

Answer 1

Моя первая проблема с исходным фрагментом кода на основе while - это постоянное выделение памяти для байтового буфера. Помещение «нового» оператора здесь специально предназначено для менеджера памяти .NET, чтобы выделить память для буфера, в то время как память, выделенная на последней итерации, отправляется обратно в неиспользуемый пул для возможной сборки мусора. Это кажется огромной работой, которую нужно выполнить в относительно узком цикле.

Мне любопытно, как можно улучшить производительность, если бы вы создали этот буфер во время разработки с разумным размером, скажем, 8 КБ, чтобы у вас не было всего этого выделения памяти, освобождения и фрагментации. Это поможет?

private byte[] buffer = new byte[8192];

void port_DataReceived(object sender, SerialDataReceivedEventArgs e)
{
    Thread.Sleep(100);
    while (port.BytesToRead > 0)
    {
        var count = port.BytesToRead;

        var read = port.Read(buffer, 0, count);

        // ... more code   
    }
}

Моя другая проблема с перераспределением этого буфера на каждой итерации цикла состоит в том, что перераспределение может быть ненужным, если буфер уже достаточно велик. Учтите следующее:

• Итерация цикла 1: получено 100 байт; выделить буфер из 100 байт
• Итерация цикла 2: получено 75 байт; выделить буфер 75 байтов

В этом сценарии вам на самом деле не нужно перераспределять буфер, потому что буфер из 100 байтов, выделенный в итерации цикла, более чем достаточно для обработки 75 байтов, полученных в итерации цикла. Нет необходимости. уничтожить 100-байтовый буфер и создать 75-байтовый буфер. (Конечно, это спорный вопрос, если вы просто статически создаете буфер и вообще выводите его из цикла.)

С другой стороны, я могу предположить, что цикл DataReceived касается только приема данных. Я не уверен, что делают эти компоненты MEF, но я сомневаюсь, что их работа должна выполняться в цикле приема данных. Возможно ли, чтобы полученные данные были помещены в какую-то очередь, и компоненты MEF могли бы забрать их там? Я заинтересован в том, чтобы цикл DataReceived был максимально быстрым. Возможно, полученные данные можно поместить в очередь, чтобы они могли вернуться к работе, получая больше данных. Вы можете настроить другой поток, возможно, для наблюдения за данными, поступающими в очередь, и чтобы компоненты MEF собирали данные оттуда и выполняли свою работу оттуда. Это может быть больше кодирования, но это может помочь петле приема данных быть максимально отзывчивой.

Answer 2

И это может быть так просто ...

Либо вы используете обработчик DataReceived, но без цикла и, конечно, без Sleep (), читайте, какие данные готовы, и отправляйте их куда-нибудь (в очередь или в MemoryStream),

или

Запустите поток (BgWorker) и выполните (блокирование) serialPort1.Read (...) и снова нажмите или соберите полученные данные.

Edit:

Из того, что вы опубликовали, я бы сказал: отбросьте обработчик событий и просто прочитайте байты внутри Dowork (). Это дает то преимущество, что вы можете указать, какой объем данных вы хотите, если он (намного) меньше, чем ReadBufferSize.

Edit2, относительно Обновления2:

Вам все равно будет намного лучше с циклом while внутри BgWorker, вообще не использующим событие. Простой способ:

byte[] buffer = new byte[128];  // 128 = (average) size of a record
while(port.IsOpen && ! worker.CancelationPending)
{
   int count = port.Read(buffer, 0, 128);
   // proccess count bytes

}

Теперь, возможно, ваши записи имеют переменный размер, и вы не хотите ждать завершения следующих 126 байтов, чтобы завершить их. Вы можете настроить это, уменьшив размер буфера или установив ReadTimeOut. Чтобы получить очень детальную информацию, вы можете использовать port.ReadByte (). Так как он читает из ReadBuffer, он на самом деле не медленнее.

Answer 3

Если вы хотите записать данные в файл, и последовательный порт время от времени останавливается, это простой способ сделать это. Если возможно, сделайте ваш буфер достаточно большим, чтобы вместить все байты, которые вы планируете поместить в один файл. Затем напишите код в обработчике событий, полученном от данных, как показано ниже. Затем, когда вы получаете возможность, запишите весь буфер в файл, как показано ниже. Если вы должны читать ИЗ буфера, пока последовательный порт читает ТО из вашего буфера, попробуйте использовать объект буферизованного потока, чтобы избежать тупиков и условий гонки.

private byte[] buffer = new byte[8192]; 
var index = 0;
void port_DataReceived(object sender, SerialDataReceivedEventArgs e)
{    
    index += port.Read(buffer, index, port.BytesToRead);
} 

void WriteDataToFile()
{
    binaryWriter.Write(buffer, 0, index); 
    index = 0;
}