[wpiutil] DataLog: Add last value and change detection

[PeterJohnson]

Update() checks/updates the last value and appends only if changed. GetLastValue() gets the last value.

This is only implemented at the Entry object level, not the overall DataLog, to avoid needing to add a hashtable lookup.

TODO:

• Finish StructArray C++
• Add tests
• Add mutexes for thread safety
• Add clone()/copy() interface for Java struct and protobuf to enable immutable objects to avoid serialization

[KangarooKoala]

Overall, looks pretty good! However, the documentation for getLastValue() in Java should be updated to indicate that it only does change checking against the last call to update(). Also, I'm concerned about what may happen if an entry has mixed update() and append() usage (e.g., intEntry.update(1); intEntry.append(0); intEntry.update(1))- Maybe just add a boolean flag to indicate when m_lastValue is stale? There's also the possibility of multiple log entry objects being made with the same entry ID- Maybe just document that the configuration is unsupported? (If the documentation only talks about one log entry being supported, that should be good enough too)

[PeterJohnson]

I suppose it would be not too bad to have append() simply clear the last value, although that would require grabbing a mutex. The only way to fix multiple objects with the same ID both calling update() would be to move all the update logic and implementation to the other side of a hashtable lookup in the main DataLog class. But that would impact append() as well, because it would have to do that lookup to clear the last value. So I would prefer to just choose between these two things, not do both. My sense is it will be pretty obvious to a user if they're mixing append() and update() to a single log entry. And across two log entries pointing to the same ID, you're going to end up with a mismash if one is doing update() and the other append(), unless we do both, which has an annoying impact to append(). So I'm kind of tempted to do neither and just caveat emptor it, or do just the second one (leave append as-is, but make update() common across the same ID).

The reason for this feature is actually to help out higher level log implementations which will switch between NT and DataLog. Those will already be doing a hashtable lookup (so duplicate IDs won't be a problem) and will always call update() (so append/update mixing won't be a problem), so doing anything more in DataLog itself will just be useless overhead.

[KangarooKoala]

Your logic makes sense! Either option you proposed (leave append() and update() as-is or make update() common across entries with the same ID) sounds good. From the user's side it would be ideal to prevent people from shooting themselves in the foot by making update() common across entries with the same ID, but looking at the DataLog class doing so would require a decent amount of effort (including figuring out how to handle mismatched types), so I'm leaning towards not doing that.

[jonahsnider]

Is checking for value changes by comparing current value vs previous value better than just comparing object hashcodes? Curious if hashcodes are any faster to compare, or how significant the memory savings are (since storing the full previous value isn't needed).

[PeterJohnson]

Hashcodes are not unique, so if they compared equal, we'd still have to compare the contents. And in general they're computed by looping over contents etc, so they're not really much faster (we'd still have to loop over the new value to generate the hashcode, and in the equal case, loop over it again to double-check it's truly equal). Note that larger objects, we only store a full object copy if it's specifically declared to be immutable or cloneable; we will otherwise just compare the serialized data.