[ntcore] Compute Value memory size when creating value (#5657)

This avoids a later switch-based computation in NetworkLoopQueue.

ntcore/src/main/native/cpp/Value.cpp

@@ -4,7 +4,9 @@
 
 #include <stdint.h>
 
+#include <algorithm>
 #include <cstring>
+#include <numeric>
 #include <span>
 
 #include <wpi/MemAlloc.h>
@@ -27,6 +29,15 @@ struct StringArrayStorage {
     InitNtStrings();
   }
   void InitNtStrings();
+  size_t EstimateSize() const {
+    return sizeof(StringArrayStorage) +
+           strings.capacity() * sizeof(std::string) +
+           ntStrings.capacity() * sizeof(NT_String) +
+           std::accumulate(strings.begin(), strings.end(), 0,
+                           [](const auto& sum, const auto& val) {
+                             return sum + val.capacity();
+                           });
+  }
 
   std::vector<std::string> strings;
   std::vector<NT_String> ntStrings;
@@ -59,12 +70,13 @@ Value::Value() {
   m_val.type = NT_UNASSIGNED;
   m_val.last_change = 0;
   m_val.server_time = 0;
+  m_size = 0;
 }
 
-Value::Value(NT_Type type, int64_t time, const private_init&)
-    : Value{type, time == 0 ? nt::Now() : time, 1, private_init{}} {}
+Value::Value(NT_Type type, size_t size, int64_t time, const private_init&)
+    : Value{type, size, time == 0 ? nt::Now() : time, 1, private_init{}} {}
 
-Value::Value(NT_Type type, int64_t time, int64_t serverTime,
+Value::Value(NT_Type type, size_t size, int64_t time, int64_t serverTime,
              const private_init&) {
   m_val.type = type;
   m_val.last_change = time;
@@ -80,10 +92,11 @@ Value::Value(NT_Type type, int64_t time, int64_t serverTime,
   } else if (m_val.type == NT_STRING_ARRAY) {
     m_val.data.arr_string.arr = nullptr;
   }
+  m_size = size;
 }
 
 Value Value::MakeBooleanArray(std::span<const bool> value, int64_t time) {
-  Value val{NT_BOOLEAN_ARRAY, time, private_init{}};
+  Value val{NT_BOOLEAN_ARRAY, value.size() * sizeof(int), time, private_init{}};
   auto data = AllocateArray<int>(value.size());
   std::copy(value.begin(), value.end(), data.get());
   val.m_val.data.arr_boolean.arr = data.get();
@@ -93,7 +106,7 @@ Value Value::MakeBooleanArray(std::span<const bool> value, int64_t time) {
 }
 
 Value Value::MakeBooleanArray(std::span<const int> value, int64_t time) {
-  Value val{NT_BOOLEAN_ARRAY, time, private_init{}};
+  Value val{NT_BOOLEAN_ARRAY, value.size() * sizeof(int), time, private_init{}};
   auto data = AllocateArray<int>(value.size());
   std::copy(value.begin(), value.end(), data.get());
   val.m_val.data.arr_boolean.arr = data.get();
@@ -103,7 +116,7 @@ Value Value::MakeBooleanArray(std::span<const int> value, int64_t time) {
 }
 
 Value Value::MakeBooleanArray(std::vector<int>&& value, int64_t time) {
-  Value val{NT_BOOLEAN_ARRAY, time, private_init{}};
+  Value val{NT_BOOLEAN_ARRAY, value.size() * sizeof(int), time, private_init{}};
   auto data = std::make_shared<std::vector<int>>(std::move(value));
   val.m_val.data.arr_boolean.arr = data->data();
   val.m_val.data.arr_boolean.size = data->size();
@@ -112,7 +125,8 @@ Value Value::MakeBooleanArray(std::vector<int>&& value, int64_t time) {
 }
 
 Value Value::MakeIntegerArray(std::span<const int64_t> value, int64_t time) {
-  Value val{NT_INTEGER_ARRAY, time, private_init{}};
+  Value val{NT_INTEGER_ARRAY, value.size() * sizeof(int64_t), time,
+            private_init{}};
   auto data = AllocateArray<int64_t>(value.size());
   std::copy(value.begin(), value.end(), data.get());
   val.m_val.data.arr_int.arr = data.get();
@@ -122,7 +136,8 @@ Value Value::MakeIntegerArray(std::span<const int64_t> value, int64_t time) {
 }
 
 Value Value::MakeIntegerArray(std::vector<int64_t>&& value, int64_t time) {
-  Value val{NT_INTEGER_ARRAY, time, private_init{}};
+  Value val{NT_INTEGER_ARRAY, value.size() * sizeof(int64_t), time,
+            private_init{}};
   auto data = std::make_shared<std::vector<int64_t>>(std::move(value));
   val.m_val.data.arr_int.arr = data->data();
   val.m_val.data.arr_int.size = data->size();
@@ -131,7 +146,7 @@ Value Value::MakeIntegerArray(std::vector<int64_t>&& value, int64_t time) {
 }
 
 Value Value::MakeFloatArray(std::span<const float> value, int64_t time) {
-  Value val{NT_FLOAT_ARRAY, time, private_init{}};
+  Value val{NT_FLOAT_ARRAY, value.size() * sizeof(float), time, private_init{}};
   auto data = AllocateArray<float>(value.size());
   std::copy(value.begin(), value.end(), data.get());
   val.m_val.data.arr_float.arr = data.get();
@@ -141,7 +156,7 @@ Value Value::MakeFloatArray(std::span<const float> value, int64_t time) {
 }
 
 Value Value::MakeFloatArray(std::vector<float>&& value, int64_t time) {
-  Value val{NT_FLOAT_ARRAY, time, private_init{}};
+  Value val{NT_FLOAT_ARRAY, value.size() * sizeof(float), time, private_init{}};
   auto data = std::make_shared<std::vector<float>>(std::move(value));
   val.m_val.data.arr_float.arr = data->data();
   val.m_val.data.arr_float.size = data->size();
@@ -150,7 +165,8 @@ Value Value::MakeFloatArray(std::vector<float>&& value, int64_t time) {
 }
 
 Value Value::MakeDoubleArray(std::span<const double> value, int64_t time) {
-  Value val{NT_DOUBLE_ARRAY, time, private_init{}};
+  Value val{NT_DOUBLE_ARRAY, value.size() * sizeof(double), time,
+            private_init{}};
   auto data = AllocateArray<double>(value.size());
   std::copy(value.begin(), value.end(), data.get());
   val.m_val.data.arr_double.arr = data.get();
@@ -160,7 +176,8 @@ Value Value::MakeDoubleArray(std::span<const double> value, int64_t time) {
 }
 
 Value Value::MakeDoubleArray(std::vector<double>&& value, int64_t time) {
-  Value val{NT_DOUBLE_ARRAY, time, private_init{}};
+  Value val{NT_DOUBLE_ARRAY, value.size() * sizeof(double), time,
+            private_init{}};
   auto data = std::make_shared<std::vector<double>>(std::move(value));
   val.m_val.data.arr_double.arr = data->data();
   val.m_val.data.arr_double.size = data->size();
@@ -169,8 +186,8 @@ Value Value::MakeDoubleArray(std::vector<double>&& value, int64_t time) {
 }
 
 Value Value::MakeStringArray(std::span<const std::string> value, int64_t time) {
-  Value val{NT_STRING_ARRAY, time, private_init{}};
   auto data = std::make_shared<StringArrayStorage>(value);
+  Value val{NT_STRING_ARRAY, data->EstimateSize(), time, private_init{}};
   val.m_val.data.arr_string.arr = data->ntStrings.data();
   val.m_val.data.arr_string.size = data->ntStrings.size();
   val.m_storage = std::move(data);
@@ -178,8 +195,8 @@ Value Value::MakeStringArray(std::span<const std::string> value, int64_t time) {
 }
 
 Value Value::MakeStringArray(std::vector<std::string>&& value, int64_t time) {
-  Value val{NT_STRING_ARRAY, time, private_init{}};
   auto data = std::make_shared<StringArrayStorage>(std::move(value));
+  Value val{NT_STRING_ARRAY, data->EstimateSize(), time, private_init{}};
   val.m_val.data.arr_string.arr = data->ntStrings.data();
   val.m_val.data.arr_string.size = data->ntStrings.size();
   val.m_storage = std::move(data);

ntcore/src/main/native/cpp/net/NetworkLoopQueue.cpp

@@ -12,37 +12,7 @@ static constexpr size_t kMaxSize = 2 * 1024 * 1024;
 
 void NetworkLoopQueue::SetValue(NT_Publisher pubHandle, const Value& value) {
   std::scoped_lock lock{m_mutex};
-  switch (value.type()) {
-    case NT_STRING:
-      m_size += value.GetString().size();  // imperfect but good enough
-      break;
-    case NT_RAW:
-      m_size += value.GetRaw().size_bytes();
-      break;
-    case NT_BOOLEAN_ARRAY:
-      m_size += value.GetBooleanArray().size_bytes();
-      break;
-    case NT_INTEGER_ARRAY:
-      m_size += value.GetIntegerArray().size_bytes();
-      break;
-    case NT_FLOAT_ARRAY:
-      m_size += value.GetFloatArray().size_bytes();
-      break;
-    case NT_DOUBLE_ARRAY:
-      m_size += value.GetDoubleArray().size_bytes();
-      break;
-    case NT_STRING_ARRAY: {
-      auto arr = value.GetStringArray();
-      m_size += arr.size_bytes();
-      for (auto&& s : arr) {
-        m_size += s.capacity();
-      }
-      break;
-    }
-    default:
-      break;
-  }
-  m_size += sizeof(ClientMessage);
+  m_size += sizeof(ClientMessage) + value.size();
   if (m_size > kMaxSize) {
     if (!m_sizeErrored) {
       WPI_ERROR(m_logger, "NT: dropping value set due to memory limits");

ntcore/src/main/native/include/networktables/NetworkTableValue.h

@@ -29,8 +29,9 @@ class Value final {
 
  public:
   Value();
-  Value(NT_Type type, int64_t time, const private_init&);
-  Value(NT_Type type, int64_t time, int64_t serverTime, const private_init&);
+  Value(NT_Type type, size_t size, int64_t time, const private_init&);
+  Value(NT_Type type, size_t size, int64_t time, int64_t serverTime,
+        const private_init&);
 
   explicit operator bool() const { return m_val.type != NT_UNASSIGNED; }
 
@@ -62,6 +63,15 @@ class Value final {
    */
   int64_t time() const { return m_val.last_change; }
 
+  /**
+   * Get the approximate in-memory size of the value in bytes. This is zero for
+   * values that do not require additional memory beyond the memory of the Value
+   * itself.
+   *
+   * @return The size in bytes.
+   */
+  size_t size() const { return m_size; }
+
   /**
    * Set the local creation time of the value.
    *
@@ -305,7 +315,7 @@ class Value final {
    * @return The entry value
    */
   static Value MakeBoolean(bool value, int64_t time = 0) {
-    Value val{NT_BOOLEAN, time, private_init{}};
+    Value val{NT_BOOLEAN, 0, time, private_init{}};
     val.m_val.data.v_boolean = value;
     return val;
   }
@@ -319,7 +329,7 @@ class Value final {
    * @return The entry value
    */
   static Value MakeInteger(int64_t value, int64_t time = 0) {
-    Value val{NT_INTEGER, time, private_init{}};
+    Value val{NT_INTEGER, 0, time, private_init{}};
     val.m_val.data.v_int = value;
     return val;
   }
@@ -333,7 +343,7 @@ class Value final {
    * @return The entry value
    */
   static Value MakeFloat(float value, int64_t time = 0) {
-    Value val{NT_FLOAT, time, private_init{}};
+    Value val{NT_FLOAT, 0, time, private_init{}};
     val.m_val.data.v_float = value;
     return val;
   }
@@ -347,7 +357,7 @@ class Value final {
    * @return The entry value
    */
   static Value MakeDouble(double value, int64_t time = 0) {
-    Value val{NT_DOUBLE, time, private_init{}};
+    Value val{NT_DOUBLE, 0, time, private_init{}};
     val.m_val.data.v_double = value;
     return val;
   }
@@ -361,8 +371,8 @@ class Value final {
    * @return The entry value
    */
   static Value MakeString(std::string_view value, int64_t time = 0) {
-    Value val{NT_STRING, time, private_init{}};
     auto data = std::make_shared<std::string>(value);
+    Value val{NT_STRING, data->capacity(), time, private_init{}};
     val.m_val.data.v_string.str = const_cast<char*>(data->c_str());
     val.m_val.data.v_string.len = data->size();
     val.m_storage = std::move(data);
@@ -379,8 +389,8 @@ class Value final {
    */
   template <std::same_as<std::string> T>
   static Value MakeString(T&& value, int64_t time = 0) {
-    Value val{NT_STRING, time, private_init{}};
     auto data = std::make_shared<std::string>(std::forward<T>(value));
+    Value val{NT_STRING, data->capacity(), time, private_init{}};
     val.m_val.data.v_string.str = const_cast<char*>(data->c_str());
     val.m_val.data.v_string.len = data->size();
     val.m_storage = std::move(data);
@@ -396,9 +406,9 @@ class Value final {
    * @return The entry value
    */
   static Value MakeRaw(std::span<const uint8_t> value, int64_t time = 0) {
-    Value val{NT_RAW, time, private_init{}};
     auto data =
         std::make_shared<std::vector<uint8_t>>(value.begin(), value.end());
+    Value val{NT_RAW, data->capacity(), time, private_init{}};
     val.m_val.data.v_raw.data = const_cast<uint8_t*>(data->data());
     val.m_val.data.v_raw.size = data->size();
     val.m_storage = std::move(data);
@@ -415,8 +425,8 @@ class Value final {
    */
   template <std::same_as<std::vector<uint8_t>> T>
   static Value MakeRaw(T&& value, int64_t time = 0) {
-    Value val{NT_RAW, time, private_init{}};
     auto data = std::make_shared<std::vector<uint8_t>>(std::forward<T>(value));
+    Value val{NT_RAW, data->capacity(), time, private_init{}};
     val.m_val.data.v_raw.data = const_cast<uint8_t*>(data->data());
     val.m_val.data.v_raw.size = data->size();
     val.m_storage = std::move(data);
@@ -631,6 +641,7 @@ class Value final {
  private:
   NT_Value m_val = {};
   std::shared_ptr<void> m_storage;
+  size_t m_size;
 };
 
 bool operator==(const Value& lhs, const Value& rhs);