Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
BLuedtke opened this issue May 6, 2024 · 2 comments
Open

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

BLuedtke opened this issue May 6, 2024 · 2 comments
Assignees

Comments

@BLuedtke
Copy link
Collaborator

BLuedtke commented May 6, 2024

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 BLuedtke self-assigned this May 6, 2024
@BLuedtke
Copy link
Collaborator Author

BLuedtke commented May 8, 2024

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
Copy link
Collaborator Author

BLuedtke commented May 8, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

1 participant