Presist logging across reconnect [CPP-383]

[notoriaga]

Initial plan was to persist the Tabs across reconnections. That ended up being really hard to do and I burned a bunch of time on that. Found an easier way to solve to problem though. I included a few changes that I made to try and get the Tabs to stick around between reconnections, because I think they are still generally useful

Currently the timer that tracks how long the log has been going for will still increment during a disconnect (size doesn't obviously because there are no messages). I assume this is ok, but probably could figure out a way around that if need be

[notoriaga]

A lot of the changes are switching the mutex to a parking lot mutex. I needed to use MappedMutexGuard and there is no equivalent for the std mutex.

[john-michaelburke]

Wow MappedMutexGuard is a pretty cool idea and beyond perfect for what we are doing. We might want to eventually move all use of shared_state to this approach. Possibly improve performance especially for things that touch shared_state on every new sbp message.

[notoriaga]

We already store the the current connection in shared_state, so to get the name you just get it and call .name() on it

[notoriaga]

Moves the logger into shared_state so it sticks around even when Tabs is dropped

[notoriaga]

The main ClientSender we use (the channel based one) doesn't actually need a mutable receiver. Immutable receivers are a bit more ergonomic, and allows us to avoid a few clones.

[john-michaelburke]

In the future, I think it would be cool if we added storage to the clientsender similar to how the TestSender is a vec and then have a function for just grabbing the last vec sent. Could be a way to make more thorough integration tests. I created a ticket for it:
https://swift-nav.atlassian.net/browse/CPP-412

[john-michaelburke]

Wow MappedMutexGuard is a pretty cool idea and beyond perfect for what we are doing. We might want to eventually move all use of shared_state to this approach. Possibly improve performance especially for things that touch shared_state on every new sbp message.

[notoriaga]

Wow MappedMutexGuard is a pretty cool idea and beyond perfect for what we are doing. We might want to eventually move all use of shared_state to this approach. Possibly improve performance especially for things that touch shared_state on every new sbp message.

Yeah definitely super cool. I think I nice way to refactor everything would be to make all the fields of SharedStateInner private, then fix the compiler errors by adding methods to SharedState. From what I can tell most of the places that directly modify SharedState without going thru one of its methods do so because of a lack of something like MappedMutexGuard. And I think it'd be nice from a maintainability standpoint to have all the access of SharedState thru methods rather than direct access.

R.e. performance, I think there could be a pretty big/easy win by using finer grained locks with SharedState. Currently we have Arc<Mutex<SharedStateInner>>, but we could drop the outer mutex and add locks to the interior fields. So something like

pub struct SharedState(Arc<SharedStateInner>);
struct SharedStateInner {
    logging_bar: Mutex<LoggingBarState>,
    log_panel: Mutex<LogPanelState>,
    tracking_tab: Mutex<TrackingTabState>,
    ...
}

That way when we have multiple threads accessing different parts of SharedState they won't block each other. Another bonus is fields like debug wouldn't even require locks, we could just use atomics

[john-michaelburke]

R.e. performance, I think there could be a pretty big/easy win by using finer grained locks with SharedState. Currently we have Arc<Mutex<SharedStateInner>>, but we could drop the outer mutex and add locks to the interior fields. So something like

pub struct SharedState(Arc<SharedStateInner>);
struct SharedStateInner {
    logging_bar: Mutex<LoggingBarState>,
    log_panel: Mutex<LogPanelState>,
    tracking_tab: Mutex<TrackingTabState>,
    ...
}

That way when we have multiple threads accessing different parts of SharedState they won't block each other. Another bonus is fields like debug wouldn't even require locks, we could just use atomics

Sounds good to me. I could definitely see a gain from that. I created a ticket for this suggestion: https://swift-nav.atlassian.net/browse/CPP-413