From 1ab538be4359867d399868eceb4ded1a8d571fbb Mon Sep 17 00:00:00 2001 From: yah01 Date: Tue, 5 Dec 2023 10:22:32 +0800 Subject: [PATCH] enhance: enable assert method to format arguments (#28812) (#28907) for now the assert method in segcore could accept a string information, too many codes don't print the value they assert. make it happy related #28811 pr: #28812 Signed-off-by: yah01 --- internal/core/src/common/Array.h | 16 +++++------ internal/core/src/common/BitsetView.h | 12 ++++---- internal/core/src/common/EasyAssert.h | 35 ++++++++++++++--------- internal/core/src/common/FieldMeta.h | 2 +- internal/core/src/common/File.h | 6 ++-- internal/core/src/common/Json.h | 22 +++++++------- internal/core/src/common/QueryResult.h | 3 +- internal/core/src/common/protobuf_utils.h | 4 +-- internal/core/src/mmap/Column.h | 29 +++++++++---------- 9 files changed, 69 insertions(+), 60 deletions(-) diff --git a/internal/core/src/common/Array.h b/internal/core/src/common/Array.h index d36c6b3392..61f444d7a5 100644 --- a/internal/core/src/common/Array.h +++ b/internal/core/src/common/Array.h @@ -234,10 +234,10 @@ class Array { template T get_data(const int index) const { - AssertInfo( - index >= 0 && index < length_, - fmt::format( - "index out of range, index={}, length={}", index, length_)); + AssertInfo(index >= 0 && index < length_, + "index out of range, index={}, length={}", + index, + length_); if constexpr (std::is_same_v || std::is_same_v) { size_t element_length = (index == length_ - 1) @@ -461,10 +461,10 @@ class ArrayView { template T get_data(const int index) const { - AssertInfo( - index >= 0 && index < length_, - fmt::format( - "index out of range, index={}, length={}", index, length_)); + AssertInfo(index >= 0 && index < length_, + "index out of range, index={}, length={}", + index, + length_); if constexpr (std::is_same_v || std::is_same_v) { diff --git a/internal/core/src/common/BitsetView.h b/internal/core/src/common/BitsetView.h index 57ce86a97e..dc0e9d8a59 100644 --- a/internal/core/src/common/BitsetView.h +++ b/internal/core/src/common/BitsetView.h @@ -57,13 +57,13 @@ class BitsetView : public knowhere::BitsetView { return {}; } - AssertInfo((offset & 0x7) == 0, "offset is not divisible by 8"); + AssertInfo( + (offset & 0x7) == 0, "offset {} is not divisible by 8", offset); AssertInfo(offset + size <= this->size(), - fmt::format( - "index out of range, offset={}, size={}, bitset.size={}", - offset, - size, - this->size())); + "index out of range, offset={}, size={}, bitset.size={}", + offset, + size, + this->size()); return {data() + (offset >> 3), size}; } }; diff --git a/internal/core/src/common/EasyAssert.h b/internal/core/src/common/EasyAssert.h index 07a444c4a7..98b9eb7218 100644 --- a/internal/core/src/common/EasyAssert.h +++ b/internal/core/src/common/EasyAssert.h @@ -24,6 +24,8 @@ #include "pb/common.pb.h" #include "common/type_c.h" +#include "fmt/core.h" + /* Paste this on the file if you want to debug. */ namespace milvus { enum ErrorCode { @@ -116,21 +118,28 @@ FailureCStatus(std::exception* ex) { } // namespace milvus -#define AssertInfo(expr, info) \ - do { \ - auto _expr_res = bool(expr); \ - /* call func only when needed */ \ - if (!_expr_res) { \ - milvus::impl::EasyAssertInfo( \ - _expr_res, #expr, __FILE__, __LINE__, (info)); \ - } \ +#define AssertInfo(expr, info, args...) \ + do { \ + auto _expr_res = bool(expr); \ + /* call func only when needed */ \ + if (!_expr_res) { \ + milvus::impl::EasyAssertInfo(_expr_res, \ + #expr, \ + __FILE__, \ + __LINE__, \ + fmt::format(info, ##args)); \ + } \ } while (0) #define Assert(expr) AssertInfo((expr), "") -#define PanicInfo(errcode, info) \ - do { \ - milvus::impl::EasyAssertInfo( \ - false, "", __FILE__, __LINE__, (info), errcode); \ - __builtin_unreachable(); \ +#define PanicInfo(errcode, info, args...) \ + do { \ + milvus::impl::EasyAssertInfo(false, \ + "", \ + __FILE__, \ + __LINE__, \ + fmt::format(info, ##args), \ + errcode); \ + __builtin_unreachable(); \ } while (0) diff --git a/internal/core/src/common/FieldMeta.h b/internal/core/src/common/FieldMeta.h index c6d1775595..36ff505b8e 100644 --- a/internal/core/src/common/FieldMeta.h +++ b/internal/core/src/common/FieldMeta.h @@ -45,7 +45,7 @@ datatype_sizeof(DataType data_type, int dim = 1) { case DataType::VECTOR_FLOAT: return sizeof(float) * dim; case DataType::VECTOR_BINARY: { - AssertInfo(dim % 8 == 0, "dim=" + std::to_string(dim)); + AssertInfo(dim % 8 == 0, "dim={}", dim); return dim / 8; } case DataType::VECTOR_FLOAT16: { diff --git a/internal/core/src/common/File.h b/internal/core/src/common/File.h index 3622bcbf03..db8e0c304f 100644 --- a/internal/core/src/common/File.h +++ b/internal/core/src/common/File.h @@ -35,9 +35,9 @@ class File { Open(const std::string_view filepath, int flags) { int fd = open(filepath.data(), flags, S_IRUSR | S_IWUSR); AssertInfo(fd != -1, - fmt::format("failed to create mmap file {}: {}", - filepath, - strerror(errno))); + "failed to create mmap file {}: {}", + filepath, + strerror(errno)); return File(fd); } diff --git a/internal/core/src/common/Json.h b/internal/core/src/common/Json.h index 892652955a..640dc03724 100644 --- a/internal/core/src/common/Json.h +++ b/internal/core/src/common/Json.h @@ -48,11 +48,11 @@ class Json { } Json(const char* data, size_t len, size_t cap) : data_(data, len) { - AssertInfo(len + simdjson::SIMDJSON_PADDING <= cap, - fmt::format("create json without enough memory size for " - "SIMD, len={}, cap={}", - len, - cap)); + AssertInfo( + len + simdjson::SIMDJSON_PADDING <= cap, + "create json without enough memory size for SIMD, len={}, cap={}", + len, + cap); } // WARN: this is used for fast non-copy construction, @@ -105,9 +105,9 @@ class Json { auto doc = parser.iterate(data_, data_.size() + simdjson::SIMDJSON_PADDING); AssertInfo(doc.error() == simdjson::SUCCESS, - fmt::format("failed to parse the json {}: {}", - data_, - simdjson::error_message(doc.error()))); + "failed to parse the json {}: {}", + data_, + simdjson::error_message(doc.error())); return doc; } @@ -119,9 +119,9 @@ class Json { // as we have allocated the memory with this padding auto doc = parser.parse(data_); AssertInfo(doc.error() == simdjson::SUCCESS, - fmt::format("failed to parse the json {}: {}", - data_, - simdjson::error_message(doc.error()))); + "failed to parse the json {}: {}", + data_, + simdjson::error_message(doc.error())); return doc; } diff --git a/internal/core/src/common/QueryResult.h b/internal/core/src/common/QueryResult.h index 152a93f675..c46b2334fe 100644 --- a/internal/core/src/common/QueryResult.h +++ b/internal/core/src/common/QueryResult.h @@ -39,7 +39,8 @@ struct SearchResult { return 0; } AssertInfo(topk_per_nq_prefix_sum_.size() == total_nq_ + 1, - "wrong topk_per_nq_prefix_sum_ size"); + "wrong topk_per_nq_prefix_sum_ size {}", + topk_per_nq_prefix_sum_.size()); return topk_per_nq_prefix_sum_[total_nq_]; } diff --git a/internal/core/src/common/protobuf_utils.h b/internal/core/src/common/protobuf_utils.h index 4a6502fc4f..0ce6f8f573 100644 --- a/internal/core/src/common/protobuf_utils.h +++ b/internal/core/src/common/protobuf_utils.h @@ -32,8 +32,8 @@ RepeatedKeyValToMap( kvs) { std::map mapping; for (auto& kv : kvs) { - AssertInfo(!mapping.count(kv.key()), - "repeat key(" + kv.key() + ") in protobuf"); + AssertInfo( + !mapping.count(kv.key()), "repeat key({}) in protobuf", kv.key()); mapping.emplace(kv.key(), kv.value()); } return mapping; diff --git a/internal/core/src/mmap/Column.h b/internal/core/src/mmap/Column.h index 2d18d977c9..451d86367e 100644 --- a/internal/core/src/mmap/Column.h +++ b/internal/core/src/mmap/Column.h @@ -57,9 +57,9 @@ class ColumnBase { MAP_PRIVATE | MAP_ANON, -1, 0)); - AssertInfo( - data_ != MAP_FAILED, - fmt::format("failed to create anon map, err: {}", strerror(errno))); + AssertInfo(data_ != MAP_FAILED, + "failed to create anon map, err: {}", + strerror(errno)); } // mmap mode ctor @@ -79,8 +79,8 @@ class ColumnBase { file.Descriptor(), 0)); AssertInfo(data_ != MAP_FAILED, - fmt::format("failed to create file-backed map, err: {}", - strerror(errno))); + "failed to create file-backed map, err: {}", + strerror(errno)); madvise(data_, cap_size_ + padding_, MADV_WILLNEED); } @@ -102,16 +102,16 @@ class ColumnBase { file.Descriptor(), 0)); AssertInfo(data_ != MAP_FAILED, - fmt::format("failed to create file-backed map, err: {}", - strerror(errno))); + "failed to create file-backed map, err: {}", + strerror(errno)); } virtual ~ColumnBase() { if (data_ != nullptr) { if (munmap(data_, cap_size_ + padding_)) { AssertInfo(true, - fmt::format("failed to unmap variable field, err={}", - strerror(errno))); + "failed to unmap variable field, err={}", + strerror(errno)); } } } @@ -193,16 +193,15 @@ class ColumnBase { -1, 0)); - AssertInfo(data != MAP_FAILED, - fmt::format("failed to create map: {}", strerror(errno))); + AssertInfo( + data != MAP_FAILED, "failed to create map: {}", strerror(errno)); if (data_ != nullptr) { std::memcpy(data, data_, size_); if (munmap(data_, cap_size_ + padding_)) { - AssertInfo( - false, - fmt::format("failed to unmap while expanding, err={}", - strerror(errno))); + AssertInfo(false, + "failed to unmap while expanding, err={}", + strerror(errno)); } }