uniba-swt / libbidib

A library for communication with a BiDiB (www.bidib.org) system using a serial connection.
GNU General Public License v3.0
10 stars 4 forks source link

Flush in batch instead of byte-by-byte #39

Open BLuedtke opened 4 months ago

BLuedtke commented 4 months ago

The existing bidib_serial_port_write function (bidib_transmission_serial_port.c L121) writes one byte only. However, in bidib_flush_impl (bidib_transmission_send.c L84), we have a range of the buffer we want to flush, i.e., multiple contiguous bytes. The write call takes a pointer to an array of bytes/memory to write -> instead of calling write byte-per-byte, we could call it once for the whole buffer range we want to write. We still have to call it separately for the crc byte and the delimiter, but this could heavily reduce the number of write calls we perform during operation.
Therefore, I suggest we first benchmark whether this would produce any measurable improvement. If it does, we add a second function to set next to bidib_send_byte, probably calling it bidib_send_bytes.

BLuedtke commented 4 months ago

Turns out it's more complicated. send_byte is called byte-per-byte as some bytes may need escaping -> would have to insert that byte into the buffer. That would mean shifting the remaining bytes in the buffer around, potentially quite often.

I added some different solutions to this:

bidib_flush_batch_impl0

Uses an auxilliary (somewhat larger) buffer called buffer_aux. Everything to be sent (on flush) is written/copied to this buffer_aux, including the escape bytes that are needed. Then the whole buffer_aux is written in one go.

bidib_flush_batch_impl1

Dynamically allocate an array that has space for the buffer contents plus the needed escape chars. Delimiter and CRC bytes sent separately.

bidib_flush_batch_impl2

Like bidib_flush_batch_impl1, but also integrates the starting delimiter into the dynamically allocated array.

bidib_flush_batch_impl3

Like the original byte-per-byte flush, but looks for sequences of bytes that need no escaping, i.e. that can be written in one batch. If a byte needs escaping, it is written byte-by-byte.

Tests

The variants I then tested with two of the physical test cases for our SWTbahn-Full, specifically the point switching test cases. I had two versions of bidib_flush_batch_impl0 as initially I made a mistake in the size checks, though both versions worked as far as I can see.
I recorded the duration of each explicit flush in microseconds across the two test cases, and then analyzed the results:

2024-05-08_T1050_StandardFlush_TestCases1-2.txt

Average: 180.88652482269504
Standard Deviation: 113.76279241908128
Max: 1100
Min: 64

2024-05-08_T1056_BatchFlush0_TestCases1-2.txt

Average: 30.45744680851064
Standard Deviation: 16.48941088988317
Max: 83
Min: 7

2024-05-08_T1103_BatchFlush1_TestCases1-2.txt

Average: 77.31205673758865
Standard Deviation: 53.744949919681964
Max: 366
Min: 25

2024-05-08_T1109_BatchFlush2_TestCases1-2.txt

Average: 81.05281690140845
Standard Deviation: 64.37549966294472
Max: 595
Min: 6

2024-05-08_T1117_BatchFlush3_TestCases1-2.txt

Average: 93.8303886925795
Standard Deviation: 83.83641016984762
Max: 532
Min: 13

2024-05-08_T1124_BatchFlush0Fix_TestCases1-2.txt

Average: 31.685512367491167
Standard Deviation: 19.728099688438952
Max: 150
Min: 3


Here, two extra tests were less explicit flush's were called. My expectation is that this leads to flushes were the buffer is filled more.

2024-05-08_T1139_BatchFlush0Fix_TestCases1-2_NoExplicitFlush.txt

Average: 29.163265306122447
Standard Deviation: 23.386152311785303
Max: 251
Min: 3

2024-05-08_T1154_StandardFlush_TestCases1-2_NoExplicitFlush.txt

Average: 235.89795918367346
Standard Deviation: 200.8458892293413
Max: 1211
Min: 63

Preliminary Conclusions

The bidib_flush_batch_impl0 is by far the fastest and most reliable (lowest variance) way to flush. Now, we need to test if overall behaviour is still the same with the batch-flush implementation.

BLuedtke commented 4 months ago

Behaviour with bidib_flush_batch_impl0 in other tests (with trains) seems to match the old/existing implementation.
I noticed some other unexpected (hopefully unrelated) behaviour during testing, namely the "has the train arrived here" loop in the tests not working (mising the train) in all cases due to some lockup - maybe some lock contention. But I haven't been able to reproduce it.