Due to how submessages are encoded (with a length prefix), nanopb currently does the encoding twice. It encodes once to get the length to write, then writes the length, then reencodes the entire message a 2nd time.
This results in a requirement that each encode always encodes the same. Generally, this is fine, but it'd be nice to not make this a requirement.
The double encode also requires going through the entire set of fields again, which has the possibility to be slow.
Instead of doing this, write to a temporary SmallVector. Then we can just encode the length of that buffer, and do a memcpy into primary stream. Theoretically in most cases, this should be much faster.
The Google C++ protobuf implementation has issues with dynamic linkage across DLL boundaries because it uses global variables. It also has a compile-time dependency because the protoc version must exactly match the libprotobuf version. Using nanopb with a customized generator fixes both of these issues.
Co-authored-by: Gold856 <117957790+Gold856@users.noreply.github.com>
This refactors Alert in both c++ and java to fix the issues with the current c++ implementation and improve performance.
Currently, constructing an Alert adds it to a list of Alerts with the same group and type. Activating an alert sets a flag on the alert. When the SendableAlerts is polled (GetStrings), the entire list is iterated over, filtered, and the filtered list is sorted by timestamp. This leads to a worst case O(m + nlog(n)) time complexity for GetStrings, where m and n are the count of total constructed alerts active alerts respectively. It also allocates intermediate data structures to hold the active alert strings for sorting.
This changes the implementation to improve the performance of GetStrings, by shifting the sorting overhead to Alert.Set
Constructing the Alert only initializes the alert's initial state, and initializes the SendableAlerts for the group if it is not already initialized.
Activating or deactivating an alert sets an internal flag for state tracking, and inserts or removes a structure containing the timestamp and text into a self-sorting data structure (std::set, TreeSet) containing other active alerts with the same group and type. (worst case O(log(n))
Now, SendableAlerts.GetStrings only has to iterate over the structure and copy the strings to the returned array. (amortized O(n))
This also fixes the c++ implementation by removing the need for SendableAlerts to directly access the Alert.
This also adds a helper method to SendableRegistry to force initialization of the instance to prevent static initialization ordering issues.
After a struct-type field descriptor had offsets calculated more than once, IsBitField would return true, causing the second call to CalculateOffsets to calculate incorrect offsets.
As string_view operations on std::map<std::string> won't be integrated
until C++26, placeholder implementations are used which are less efficient
in a couple of situations (e.g. insert with hint).
An empty path isn't valid on it's own, so fs::space always returns an error. This results in UINT_MAX bytes being used instead of the actual free space, which means a default constructed DataLogBackgroundWriter won't stop for low space.
Using "." instead makes the directory path the current working directory, which is the desired behavior
Previously, both wpi/expected and JSON's cpp_future.h would define enable_if_t and conjunction in wpi::detail, leading to conflicts if both were included in the same cpp source file. By renaming the namespace wpi/expected uses, there is no longer a conflict.
Group memory doxygen into one module.
Remove concept alias and add doxygen definitions for foonathan memory.
\concept was added as a doxygen command in 1.9.2 and is meant to be applied to concepts. Inserting them into standard comment paragraphs causes doxygen to interpret the following text as a concept name and add it to the documentation, as well as remove the text from the paragraph.
In the upstream repo, this alias links to markdown documentation, so it's not usable for us anyways.
That, plus adding the doxygen definitions/aliases from upstream cleans up most of the errors/weird output from doxygen for foonathan memory.
Reading exported data from shared objects on windows is broken. It requires __declspec(dllimport). However, this is problematic, as we use the same static libraries both from a shared and static context. So we can't just blindly apply dllimport.
The linker should have caught this, as data members are exported in a different way. However, due to a bug in native-utils, data member symbols were exposed directly. However, interacting with those data member was completely broken.
The only way we can really solve this is to just not use static data members. We're pretty good about this in WPILib itself. However, protobuf is absolutely terrible at this. There are a ton of inline functions that access global data. For the protobuf library itself, we can solve this easily enough.
However, for the generated protobuf code, this is much more problematic. The member needed to bypass the global data is private. This means using just the stock protobuf code, this problem is not solvable. But, protobuf generated code has insertion points. Those insertion points let us add our own code into the generated code via a protoc plugin. And it just so happens that an insertion point exists to add extra public methodsto the generated protobuf header. There is also an insertion point to let us add to the cpp file.
The methods we need are the getters, for unpacking protobufs. For any protobuf that has a message as a member, we generate a new wpi_x() getter (the existing one is just x(), where x is the field name). We then implement this in the cpp file. A trick we can use is in the cpp file, we can safely call the x() function, as the cpp file is in the same library as the global. Thus we can call that inline method, and not actually need to directly access any internal private state of the protobuf object.
TL;DR, all protobuf classes that have messages as fields now have a wpi_x() accessor that must be used instead of x() if you want the code to work on windows. After wpilibsuite/native-utils#212, the bad code will fail to link, rather then just fail at runtime.
