From e93c233d60fcf95cd3f660980c2d69abe4416661 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Sun, 17 Sep 2023 20:01:57 -0700 Subject: [PATCH] [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 | 45 +++++++++++++------ .../main/native/cpp/net/NetworkLoopQueue.cpp | 32 +------------ .../include/networktables/NetworkTableValue.h | 31 ++++++++----- 3 files changed, 53 insertions(+), 55 deletions(-) diff --git a/ntcore/src/main/native/cpp/Value.cpp b/ntcore/src/main/native/cpp/Value.cpp index 954e99a4bd..537f5ecc4e 100644 --- a/ntcore/src/main/native/cpp/Value.cpp +++ b/ntcore/src/main/native/cpp/Value.cpp @@ -4,7 +4,9 @@ #include +#include #include +#include #include #include @@ -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 strings; std::vector 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 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(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 value, int64_t time) { } Value Value::MakeBooleanArray(std::span 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(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 value, int64_t time) { } Value Value::MakeBooleanArray(std::vector&& 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::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&& value, int64_t time) { } Value Value::MakeIntegerArray(std::span 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(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 value, int64_t time) { } Value Value::MakeIntegerArray(std::vector&& 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::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&& value, int64_t time) { } Value Value::MakeFloatArray(std::span 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(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 value, int64_t time) { } Value Value::MakeFloatArray(std::vector&& 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::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&& value, int64_t time) { } Value Value::MakeDoubleArray(std::span 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(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 value, int64_t time) { } Value Value::MakeDoubleArray(std::vector&& 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::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&& value, int64_t time) { } Value Value::MakeStringArray(std::span value, int64_t time) { - Value val{NT_STRING_ARRAY, time, private_init{}}; auto data = std::make_shared(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 value, int64_t time) { } Value Value::MakeStringArray(std::vector&& value, int64_t time) { - Value val{NT_STRING_ARRAY, time, private_init{}}; auto data = std::make_shared(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); diff --git a/ntcore/src/main/native/cpp/net/NetworkLoopQueue.cpp b/ntcore/src/main/native/cpp/net/NetworkLoopQueue.cpp index 7c58c65549..944524a572 100644 --- a/ntcore/src/main/native/cpp/net/NetworkLoopQueue.cpp +++ b/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"); diff --git a/ntcore/src/main/native/include/networktables/NetworkTableValue.h b/ntcore/src/main/native/include/networktables/NetworkTableValue.h index eb05b04ec1..81e3653885 100644 --- a/ntcore/src/main/native/include/networktables/NetworkTableValue.h +++ b/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(value); + Value val{NT_STRING, data->capacity(), time, private_init{}}; val.m_val.data.v_string.str = const_cast(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 T> static Value MakeString(T&& value, int64_t time = 0) { - Value val{NT_STRING, time, private_init{}}; auto data = std::make_shared(std::forward(value)); + Value val{NT_STRING, data->capacity(), time, private_init{}}; val.m_val.data.v_string.str = const_cast(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 value, int64_t time = 0) { - Value val{NT_RAW, time, private_init{}}; auto data = std::make_shared>(value.begin(), value.end()); + Value val{NT_RAW, data->capacity(), time, private_init{}}; val.m_val.data.v_raw.data = const_cast(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 > T> static Value MakeRaw(T&& value, int64_t time = 0) { - Value val{NT_RAW, time, private_init{}}; auto data = std::make_shared>(std::forward(value)); + Value val{NT_RAW, data->capacity(), time, private_init{}}; val.m_val.data.v_raw.data = const_cast(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 m_storage; + size_t m_size; }; bool operator==(const Value& lhs, const Value& rhs);