Sequenced vectors with efficient replication #239
Conversation
csibbitt
commented
Oct 15, 2023
csibbitt
commented
- Adds sp::SeqVector for tracking elements that have a sequence number
- Tracks last sequence # to replicate
- Provides significant performance improvements for EE ships_log
* Adds sp::SeqVector for tracking elements that have a sequence number * Tracks last sequence # to replicate * Provides significant performance improvements for EE ships_log
I did these tests
|
| uint16_t count = ptr->size(); | ||
| unsigned int to_send = std::min(ptr->back().seq - ptr->last_seq, static_cast<unsigned int>(count)); | ||
|
|
||
| ptr->last_seq = ptr->back().seq; |
There was a problem hiding this comment.
The process of sending shouldn't modify the vector state. The concept of this code is that the isChanged call will check if it is changed and update data a the prev_data_ptr to know there is no change anymore.
There was a problem hiding this comment.
I agree with you in principle ("The process of sending shouldn't modify the vector state"). We need somewhere to track what has been sent so that we don't resend the entire vector (say, of a sequence of log entries) every time we sync. A class that wraps the vector (whose state doesn't change) seemed like the right place. I experimented with just using a struct with a vector member and a last_seq member for clearer separation, but this was a cleaner fit for the templating. Maybe I've overlooked a simpler way.
I understand the prev_data approach of keeping a copy and comparing them, and started that way originally since that follows the pattern of the rest of the code. In addition to needlessly storing a second copy, I didn't come up with an easy solution to new client sync. Basically, instead of last_seq, I just used prev_data_ptr->back()->seq in it's place. The isChanged code was if (ptr->back().seq != prev_data_ptr->back().seq), and the send code was to_send = ... (ptr->back().seq - prev_data_ptr->back().seq)... and they worked fine, but of course would not sync the backlog to a new client connection. I agree that the last_seq-shuffle in sendWholeDataSeqVector is a bit hacky, but I think it's pretty clear at least?
This solution reduces memory footprint and makes for an easy way to sync new clients. The vector itself doesn't get modified by sending, just the super-class that is specifically FOR tracking. If there is a clearer way of expressing this within the framework we have here, I'm happy to learn it. :)
There was a problem hiding this comment.
Well, I don't think it has to be a huge issue to send the whole log each time it changes (high logs/sec would be unreadable no mater the log size, so not really a realistic scenario). The main issue I see with the master code is that it's CPU heavy if you increase the log size. Keeping track of the sequence number inside the prev_data would solve that part.
