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

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);
    }    
}

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

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

Пожалуйста, обратите внимание, что DataEncapsulator и DataCollector являются компонентами , предоставляемыми MEF, но их производительность довольно высока.

Класс имеет слушайте() метод, который запускает фоновый рабочий, чтобы получение данных.

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.Невозможно узнать, все ли байты отправленных данных считаны за одну операцию чтения.Таким образом, каждый раз, когда данные считываются, они передаются в datacollectorкоторый возвращает 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 - это постоянное выделение памяти для байтового буфера.Помещаем здесь оператор "new", специально обращаясь к .Диспетчер сетевой памяти выделяет память для буфера, одновременно забирая память, выделенную на последней итерации, и отправляя ее обратно в неиспользуемый пул для последующей сборки мусора.Кажется, что нужно проделать ужасно много работы в относительно узком цикле.

Мне любопытно, какого повышения производительности вы добились бы, создав этот буфер во время разработки разумного размера, скажем, 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 байт, выделенного на итерации цикла 1, более чем достаточно для обработки 75 байт, полученных на итерации цикла 2.Нет необходимости уничтожать 100-байтовый буфер и создавать 75-байтовый буфер.(Конечно, это спорно, если вы просто статически создаете буфер и полностью выводите его из цикла.)

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

Answer 2

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

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

или

Запустите поток (BgWorker) и выполните (блокирующий) serialPort1.Read(...), и снова отправьте или соберите полученные данные.

Редактировать:

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

Правка2, касающаяся Обновления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 байт, чтобы завершить одну из них.Вы можете настроить это, уменьшив размер буфера или установив время ожидания чтения.Чтобы получить очень детальную информацию, вы могли бы использовать port.ReadByte().Поскольку это считывается из буфера чтения, на самом деле это ничуть не медленнее.

Answer 3

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

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;
}