Skip to content

Commit ca9d371

Browse files
authored
Fix some potential performance defects (#445) (#447)
Signed-off-by: yhmo <yihua.mo@zilliz.com>
1 parent f0a7362 commit ca9d371

File tree

7 files changed

+46
-39
lines changed

7 files changed

+46
-39
lines changed

src/impl/MilvusClientV2Impl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ MilvusClientV2Impl::CreateCollection(const CreateCollectionRequest& request) {
136136
}
137137

138138
// properties
139-
for (auto it : request.Properties()) {
139+
for (const auto& it : request.Properties()) {
140140
auto kv = rpc_request.add_properties();
141141
kv->set_key(it.first);
142142
kv->set_value(it.second);

src/impl/types/FieldData.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -359,14 +359,14 @@ BinaryVecFieldData::ToBinaryStrings(const std::vector<std::vector<uint8_t>>& dat
359359
std::vector<std::string> ret;
360360
ret.reserve(data.size());
361361
for (const auto& item : data) {
362-
ret.emplace_back(std::move(ToBinaryString(item)));
362+
ret.emplace_back(ToBinaryString(item));
363363
}
364-
return std::move(ret);
364+
return ret;
365365
}
366366

367367
std::string
368368
BinaryVecFieldData::ToBinaryString(const std::vector<uint8_t>& data) {
369-
return std::move(std::string{data.begin(), data.end()});
369+
return std::string{data.begin(), data.end()};
370370
}
371371

372372
std::vector<std::vector<uint8_t>>
@@ -376,15 +376,15 @@ BinaryVecFieldData::ToUnsignedChars(const std::vector<std::string>& data) {
376376
for (const auto& item : data) {
377377
ret.emplace_back(item.begin(), item.end());
378378
}
379-
return std::move(ret);
379+
return ret;
380380
}
381381

382382
std::vector<uint8_t>
383383
BinaryVecFieldData::ToUnsignedChars(const std::string& data) {
384384
std::vector<uint8_t> ret;
385385
ret.reserve(data.size());
386386
std::copy(data.begin(), data.end(), std::back_inserter(ret));
387-
return std::move(ret);
387+
return ret;
388388
}
389389

390390
///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

src/impl/utils/CompareUtils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ operator==(const proto::schema::FieldData& lhs, const JSONFieldData& rhs) {
203203

204204
EntityRows jsons;
205205
for (const auto& str : scalars_data) {
206-
jsons.emplace_back(std::move(nlohmann::json::parse(str)));
206+
jsons.emplace_back(nlohmann::json::parse(str));
207207
}
208208
return std::equal(jsons.begin(), jsons.end(), rhs.Data().begin());
209209
}

src/impl/utils/DqlUtils.cpp

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
namespace milvus {
2929

3030
SparseFloatVecFieldData::ElementT
31-
DecodeSparseFloatVector(std::string& bytes) {
31+
DecodeSparseFloatVector(const std::string& bytes) {
3232
if (bytes.size() % 8 != 0) {
3333
throw std::runtime_error("Unexpected binary string is received from server side!");
3434
}
@@ -65,8 +65,8 @@ BuildFieldDataSparseVectors(const google::protobuf::RepeatedPtrField<std::string
6565
auto end = cursor;
6666
std::advance(end, count);
6767
while (cursor != end) {
68-
std::string bytes = *cursor;
69-
data.emplace_back(std::move(DecodeSparseFloatVector(bytes)));
68+
const std::string& bytes = *cursor;
69+
data.emplace_back(DecodeSparseFloatVector(bytes));
7070
cursor++;
7171
}
7272
return data;
@@ -358,13 +358,21 @@ CreateMilvusFieldData(const proto::schema::FieldData& proto_data, size_t offset,
358358
return Status::OK();
359359
}
360360
case proto::schema::DataType::JSON: {
361-
std::vector<nlohmann::json> objects;
361+
// Don't use BuildFieldDataScalars here: JSON requires json::parse() for each element,
362+
// so we parse only the [offset, end) range to avoid parsing all elements upfront.
362363
const auto& scalars_data = proto_scalars.json_data().data();
363-
for (const auto& s : scalars_data) {
364-
objects.emplace_back(std::move(nlohmann::json::parse(s)));
364+
auto total = static_cast<size_t>(scalars_data.size());
365+
if (offset >= total) {
366+
field_data = std::make_shared<JSONFieldData>(std::move(name), std::vector<JSONFieldData::ElementT>{},
367+
std::move(valid_data));
368+
return Status::OK();
369+
}
370+
size_t end = (offset + count > total) ? total : offset + count;
371+
std::vector<JSONFieldData::ElementT> values;
372+
values.reserve(end - offset);
373+
for (size_t i = offset; i < end; ++i) {
374+
values.emplace_back(nlohmann::json::parse(scalars_data[static_cast<int>(i)]));
365375
}
366-
std::vector<JSONFieldData::ElementT> values =
367-
BuildFieldDataScalars<JSONFieldData::ElementT>(objects, offset, count);
368376
field_data = std::make_shared<JSONFieldData>(std::move(name), std::move(values), std::move(valid_data));
369377
return Status::OK();
370378
}
@@ -498,7 +506,7 @@ FillStructValue(const FieldDataPtr& array_data, std::vector<std::vector<nlohmann
498506
for (auto k = 0; k < actual_count; k++) {
499507
const auto& arr = actual_ptr->Value(k);
500508
if (structs.size() <= k) {
501-
structs.emplace_back(std::move(std::vector<nlohmann::json>()));
509+
structs.emplace_back();
502510
structs[k].resize(arr.size());
503511
}
504512
for (auto j = 0; j < arr.size(); j++) {
@@ -593,7 +601,7 @@ ConvertStructFieldData(const proto::schema::FieldData& proto_data, size_t offset
593601
vector_field.dim() * 4, floats.data(), floats.size(), 0, floats.size());
594602
auto num = k - offset;
595603
if (structs.size() <= num) {
596-
structs.emplace_back(std::move(std::vector<nlohmann::json>()));
604+
structs.emplace_back();
597605
structs[num].resize(vectors.size());
598606
}
599607
for (auto j = 0; j < vectors.size(); j++) {
@@ -746,8 +754,7 @@ SetEmbeddingLists(const std::vector<EmbeddingList>& emb_lists, proto::milvus::Se
746754
std::string content;
747755
content.reserve(emb_list.Count() * emb_list.Dim() * 4);
748756
for (const auto& vector : vectors.Data()) {
749-
std::string single_content(reinterpret_cast<const char*>(vector.data()), vector.size() * sizeof(float));
750-
content += single_content;
757+
content.append(reinterpret_cast<const char*>(vector.data()), vector.size() * sizeof(float));
751758
}
752759
rpc_request->set_nq(static_cast<int64_t>(emb_list.Count()));
753760
placeholder_value.add_values(std::move(content));
@@ -798,8 +805,8 @@ GenGetter(const FieldDataPtr& field) {
798805
// special process float16/bfloat16 vector to float arrays
799806
if (field->Type() == DataType::FLOAT16_VECTOR || field->Type() == DataType::BFLOAT16_VECTOR) {
800807
bool is_fp16 = (field->Type() == DataType::FLOAT16_VECTOR);
801-
std::vector<uint16_t> f16_vec = is_fp16 ? std::static_pointer_cast<Float16VecFieldData>(field)->Value(i)
802-
: std::static_pointer_cast<BFloat16VecFieldData>(field)->Value(i);
808+
const auto& f16_vec = is_fp16 ? std::static_pointer_cast<Float16VecFieldData>(field)->Data()[i]
809+
: std::static_pointer_cast<BFloat16VecFieldData>(field)->Data()[i];
803810
std::vector<float> f32_vec;
804811
f32_vec.reserve(f16_vec.size());
805812
std::transform(f16_vec.begin(), f16_vec.end(), std::back_inserter(f32_vec),
@@ -938,7 +945,7 @@ GenGetters(const std::vector<FieldDataPtr>& fields) {
938945
break;
939946
}
940947
}
941-
return std::move(getters);
948+
return getters;
942949
}
943950

944951
Status

src/impl/utils/FP16.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,43 +100,43 @@ BF16toF32(uint16_t val) {
100100
}
101101

102102
std::vector<uint16_t>
103-
ArrayF32toF16(std::vector<float> array) {
103+
ArrayF32toF16(const std::vector<float>& array) {
104104
std::vector<uint16_t> result;
105105
result.reserve(array.size());
106106
for (float v : array) {
107107
result.push_back(F32toF16(v));
108108
}
109-
return std::move(result);
109+
return result;
110110
}
111111

112112
std::vector<float>
113-
ArrayF16toF32(std::vector<uint16_t> array) {
113+
ArrayF16toF32(const std::vector<uint16_t>& array) {
114114
std::vector<float> result;
115115
result.reserve(array.size());
116116
for (uint16_t v : array) {
117117
result.push_back(F16toF32(v));
118118
}
119-
return std::move(result);
119+
return result;
120120
}
121121

122122
std::vector<uint16_t>
123-
ArrayF32toBF16(std::vector<float> array) {
123+
ArrayF32toBF16(const std::vector<float>& array) {
124124
std::vector<uint16_t> result;
125125
result.reserve(array.size());
126126
for (float v : array) {
127127
result.push_back(F32toBF16(v));
128128
}
129-
return std::move(result);
129+
return result;
130130
}
131131

132132
std::vector<float>
133-
ArrayBF16toF32(std::vector<uint16_t> array) {
133+
ArrayBF16toF32(const std::vector<uint16_t>& array) {
134134
std::vector<float> result;
135135
result.reserve(array.size());
136136
for (uint16_t v : array) {
137137
result.push_back(BF16toF32(v));
138138
}
139-
return std::move(result);
139+
return result;
140140
}
141141

142142
} // namespace milvus

src/include/milvus/types/ConnectParam.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -339,11 +339,11 @@ class ConnectParam {
339339
private:
340340
std::string uri_ = "http://localhost:19530";
341341

342-
uint64_t connect_timeout_ms_ = 10000; // the same with pymilvus
343-
uint64_t keepalive_time_ms_ = 10000; // Send keepalive pings every 10 seconds
344-
uint64_t keepalive_timeout_ms_ = 5000; // Keepalive ping timeout after 5 seconds
345-
bool keepalive_without_calls_ = true; // Allow keepalive pings when there are no gRPC calls
346-
uint64_t rpc_deadline_ms_ = 0; // the same with java sdk
342+
uint64_t connect_timeout_ms_ = 10000; // the same with pymilvus
343+
uint64_t keepalive_time_ms_ = 10000; // Send keepalive pings every 10 seconds
344+
uint64_t keepalive_timeout_ms_ = 5000; // Keepalive ping timeout after 5 seconds
345+
bool keepalive_without_calls_ = true; // Allow keepalive pings when there are no gRPC calls
346+
uint64_t rpc_deadline_ms_ = 0; // the same with java sdk
347347

348348
bool tls_{false};
349349
std::string server_name_;

src/include/milvus/utils/FP16.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,24 +49,24 @@ BF16toF32(uint16_t val);
4949
* @brief Convert a float32 array to a float16 array represented by uint16_t.
5050
*/
5151
std::vector<uint16_t>
52-
ArrayF32toF16(std::vector<float> array);
52+
ArrayF32toF16(const std::vector<float>& array);
5353

5454
/**
5555
* @brief Convert a float16 array to a float32 array.
5656
*/
5757
std::vector<float>
58-
ArrayF16toF32(std::vector<uint16_t> array);
58+
ArrayF16toF32(const std::vector<uint16_t>& array);
5959

6060
/**
6161
* @brief Convert a float32 array to a bfloat16 array represented by uint16_t.
6262
*/
6363
std::vector<uint16_t>
64-
ArrayF32toBF16(std::vector<float> array);
64+
ArrayF32toBF16(const std::vector<float>& array);
6565

6666
/**
6767
* @brief Convert a bfloat16 array to a float32 array.
6868
*/
6969
std::vector<float>
70-
ArrayBF16toF32(std::vector<uint16_t> array);
70+
ArrayBF16toF32(const std::vector<uint16_t>& array);
7171

7272
} // namespace milvus

0 commit comments

Comments
 (0)