Performance of SafeRingBufferN - (Used in the UART class)

[KurtE]

Note: earlier today I documented more of this up on the forum thread:
https://forum.arduino.cc/t/performance-of-saferingbuffern/1152977

The WIP code is up at: https://github.com/KurtE/ArduinoCore-renesas/tree/SerialX_use_write_fifo

I am experimenting with implementing the use of the RingBuffer in the Serial Transmit code, and noticed that my first cut of the code was spending a lot more time in the write method than I was expecting. There are things I can do to speed this up, like in this actual case bypass using the Ring buffer.

The beginning part of the write code:

Size_t  UART::write(const uint8_t* c, size_t len) {
  if(init_ok && (len > 0)) {
    DBGDigitalWrite(A5, HIGH);
    size_t cb_left = len;

    while (cb_left) {
      // see if we have room in the FIFO to store this character.
      bool tx_active_on_entry = tx_fsi_active;

      // Put as much of data into txBuffer as will fit
      while(cb_left && !txBuffer.isFull()) {
        txBuffer.store_char(*c++);
        cb_left--;
      }

      // if the UART was not active at the start we will start it.
      if (!tx_active_on_entry) {
        size_t cb = 0;
        while (cb < sizeof(tx_fsi_buffer)) {
          int ch = txBuffer.read_char();
          if (ch == -1) break;
          tx_fsi_buffer[cb++] = ch;            
        }
        tx_fsi_active = true;
        DBGDigitalToggle(A2);
        R_SCI_UART_Write(&(uart_ctrl), tx_fsi_buffer, cb);
,,,

In the case below I could detect that the queue is empty and transmit is not active and transfer directly to a temporary buffer that I pass off to R_SCI_UART_Write,

But in my first test case I was trying, which is to control Robotis servos at 1mbs half duplex. I see the timings of sending a logical packet

Channel 6 shows the time while we are in the write method. Channel 7 I do a toggle of the pin when I actually call out to the R_SCI_UART_Write.

It is taking about 50us in the write method when using the SafeRingBuffer. I did a quck and dirty check to see how long it would take if I used the RingBufferN instead, and the time was cut to about 17us.

I was hoping that the straight RingBufferN would be safe enough to use for the typical case of a single producer of data, and a single consumer of it. But I don't believe your implementation of this class is safe enough.

As the two methods store_char and read_char both modify a common member variable _numElems.

void RingBufferN<N>::store_char( uint8_t c )
{
  // if we should be storing the received character into the location
  // just before the tail (meaning that the head would advance to the
  // current location of the tail), we're about to overflow the buffer
  // and so we don't write the character or advance the head.
  if (!isFull())
  {
    _aucBuffer[_iHead] = c ;
    _iHead = nextIndex(_iHead);
    _numElems++;
  }
}

template <int N>
int RingBufferN<N>::read_char()
{
  if (isEmpty())
    return -1;

  uint8_t value = _aucBuffer[_iTail];
  _iTail = nextIndex(_iTail);
  _numElems--;

  return value;
}

In other implementations of like code, the store_char would only modify the _iHead member variable and the read_char would only modify the _iTail member.

When you need information like Available, you can easily compute it, something like:

template <int N>
int RingBufferN<N>::available()
{
  uint32_t head = _iHead;
  uint32_t tail = _iTail;
  if (head >= tail) return (head - tail);
  return = N + head - tail;
}

typed on fly...

There are more details up on the forum thread including the example sketch I was running. And more details on my attempt to describe the code base to another forum member.

EDIT: I am not sure if it was clear above, but the differences between the SafeRingBufferN and the simple RingBufferNm is the methods like store_char use the Synchronized code like:

template <int N>
void SafeRingBufferN<N>::store_char(uint8_t c) {
  synchronized {
    RingBufferN<N>::store_char(c);
  }
}

use the synchronized loop around code, which creates an __Guard object on entry, who disables all interrupt on creation, and restores the global interrupt enable state on exit (i.e., when the code leaves the scope).

Thoughts?

[KurtE]

Quick update: I thought that some of the stuff I mentioned here, maybe belongs as an issue against the API project, so I created the one mentioned.

I am experimenting with an implementation that implemented the RingBufferin the way I mentioned. In the same file I also put a modified version of the SafeRingBuffer code where I created two versions, one for Write and one for Read.

Which I then updated the Serial class like:

    //arduino::SafeRingBufferN<SERIAL_BUFFER_SIZE> rxBuffer;
    //arduino::SafeRingBufferN<SERIAL_BUFFER_SIZE> txBuffer;
    arduino::FasterSafeReadRingBufferN<SERIAL_BUFFER_SIZE> rxBuffer;
    arduino::FasterSafeWriteRingBufferN<SERIAL_BUFFER_SIZE> txBuffer;

FasterSafeRingBuffer.zip
Note: Still testing have not uploaded with this change to my fork/branch