Skip to content

Commit fc29348

Browse files
Various performance optimizations (#1452)
Mostly removing excess copying, or unnecessary allocations. Relates-To: OLPEDGE-2848 Signed-off-by: Andrey Kashcheev <ext-andrey.kashcheev@here.com>
1 parent 78d49bb commit fc29348

File tree

9 files changed

+88
-57
lines changed

9 files changed

+88
-57
lines changed

olp-cpp-sdk-core/include/olp/core/generated/parser/ParserWrapper.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2021 HERE Europe B.V.
2+
* Copyright (C) 2019-2023 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -24,8 +24,8 @@
2424
#include <string>
2525
#include <vector>
2626

27-
#include <boost/optional.hpp>
2827
#include <rapidjson/rapidjson.h>
28+
#include <boost/optional.hpp>
2929

3030
namespace olp {
3131
namespace parser {
@@ -80,7 +80,7 @@ inline void from_json(const rapidjson::Value& value, std::vector<T>& results) {
8080
itr != value.End(); ++itr) {
8181
T result;
8282
from_json(*itr, result);
83-
results.push_back(result);
83+
results.emplace_back(std::move(result));
8484
}
8585
}
8686

olp-cpp-sdk-core/src/cache/KeyGenerator.cpp

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2021 HERE Europe B.V.
2+
* Copyright (C) 2021-2023 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,6 +21,12 @@
2121

2222
namespace olp {
2323
namespace cache {
24+
namespace {
25+
const std::string kColons = "::";
26+
const std::string kDataSuffix = "Data";
27+
const std::string kPartitionSuffix = "partition";
28+
} // namespace
29+
2430
std::string KeyGenerator::CreateApiKey(const std::string& hrn,
2531
const std::string& service,
2632
const std::string& version) {
@@ -38,8 +44,27 @@ std::string KeyGenerator::CreateLatestVersionKey(const std::string& hrn) {
3844
std::string KeyGenerator::CreatePartitionKey(
3945
const std::string& hrn, const std::string& layer_id,
4046
const std::string& partition_id, const boost::optional<int64_t>& version) {
41-
return hrn + "::" + layer_id + "::" + partition_id +
42-
"::" + (version ? std::to_string(*version) + "::" : "") + "partition";
47+
// Key format: hrn::layer_id::partition_id::[version::]partition
48+
49+
std::string version_str =
50+
version ? std::to_string(*version) + kColons : std::string();
51+
52+
std::string result;
53+
result.reserve(hrn.size() + layer_id.size() + partition_id.size() +
54+
version_str.size() + kPartitionSuffix.size() +
55+
3 * kColons.size());
56+
57+
result.append(hrn);
58+
result.append(kColons);
59+
result.append(layer_id);
60+
result.append(kColons);
61+
result.append(partition_id);
62+
result.append(kColons);
63+
result.append(version_str);
64+
// No need to append colons here, since they are already in version_str.
65+
result.append(kPartitionSuffix);
66+
67+
return result;
4368
}
4469

4570
std::string KeyGenerator::CreatePartitionsKey(
@@ -65,7 +90,21 @@ std::string KeyGenerator::CreateQuadTreeKey(
6590
std::string KeyGenerator::CreateDataHandleKey(const std::string& hrn,
6691
const std::string& layer_id,
6792
const std::string& data_handle) {
68-
return hrn + "::" + layer_id + "::" + data_handle + "::Data";
93+
// Key format: hrn::layer_id::data_handle::Data
94+
95+
std::string result;
96+
result.reserve(hrn.size() + layer_id.size() + data_handle.size() +
97+
kDataSuffix.size() + 3 * kColons.size());
98+
99+
result.append(hrn);
100+
result.append(kColons);
101+
result.append(layer_id);
102+
result.append(kColons);
103+
result.append(data_handle);
104+
result.append(kColons);
105+
result.append(kDataSuffix);
106+
107+
return result;
69108
}
70109

71110
} // namespace cache

olp-cpp-sdk-dataservice-read/include/olp/dataservice/read/model/Partitions.h

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,8 @@ class DATASERVICE_READ_API Partition {
8282
*
8383
* @param value The partition checksum.
8484
*/
85-
void SetChecksum(const boost::optional<std::string>& value) {
86-
this->checksum_ = value;
85+
void SetChecksum(boost::optional<std::string> value) {
86+
this->checksum_ = std::move(value);
8787
}
8888

8989
/**
@@ -120,7 +120,7 @@ class DATASERVICE_READ_API Partition {
120120
*
121121
* @param value The compressed size of the partition data.
122122
*/
123-
void SetCompressedDataSize(const boost::optional<int64_t>& value) {
123+
void SetCompressedDataSize(boost::optional<int64_t> value) {
124124
this->compressed_data_size_ = value;
125125
}
126126

@@ -151,7 +151,9 @@ class DATASERVICE_READ_API Partition {
151151
*
152152
* @param value The partition data handle.
153153
*/
154-
void SetDataHandle(const std::string& value) { this->data_handle_ = value; }
154+
void SetDataHandle(std::string value) {
155+
this->data_handle_ = std::move(value);
156+
}
155157

156158
/**
157159
* @brief (Optional) Gets the uncompressed size of the partition data in
@@ -184,9 +186,7 @@ class DATASERVICE_READ_API Partition {
184186
*
185187
* @param value The uncompressed size of the partition data.
186188
*/
187-
void SetDataSize(const boost::optional<int64_t>& value) {
188-
this->data_size_ = value;
189-
}
189+
void SetDataSize(boost::optional<int64_t> value) { this->data_size_ = value; }
190190

191191
/**
192192
* Optional value for the CRC of the partition data in bytes.
@@ -243,7 +243,7 @@ class DATASERVICE_READ_API Partition {
243243
*
244244
* @param value The partition key.
245245
*/
246-
void SetPartition(const std::string& value) { this->partition_ = value; }
246+
void SetPartition(std::string value) { this->partition_ = std::move(value); }
247247

248248
/**
249249
* @brief (Optional) Gets the version of the catalog when this partition was
@@ -274,9 +274,7 @@ class DATASERVICE_READ_API Partition {
274274
* @param value The version of the catalog when this partition was last
275275
* changed.
276276
*/
277-
void SetVersion(const boost::optional<int64_t>& value) {
278-
this->version_ = value;
279-
}
277+
void SetVersion(boost::optional<int64_t> value) { this->version_ = value; }
280278
};
281279

282280
/**
@@ -313,8 +311,8 @@ class Partitions {
313311
*
314312
* @param value The list of partitions for the given layer and layer version.
315313
*/
316-
void SetPartitions(const std::vector<Partition>& value) {
317-
this->partitions_ = value;
314+
void SetPartitions(std::vector<Partition> value) {
315+
this->partitions_ = std::move(value);
318316
}
319317
};
320318

olp-cpp-sdk-dataservice-read/src/repositories/DataCacheRepository.cpp

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2021 HERE Europe B.V.
2+
* Copyright (C) 2019-2023 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -43,16 +43,15 @@ namespace repository {
4343
DataCacheRepository::DataCacheRepository(
4444
const client::HRN& hrn, std::shared_ptr<cache::KeyValueCache> cache,
4545
std::chrono::seconds default_expiry)
46-
: hrn_(hrn),
46+
: hrn_(hrn.ToCatalogHRNString()),
4747
cache_(std::move(cache)),
4848
default_expiry_(ConvertTime(default_expiry)) {}
4949

5050
client::ApiNoResponse DataCacheRepository::Put(const model::Data& data,
5151
const std::string& layer_id,
5252
const std::string& data_handle) {
53-
const std::string hrn(hrn_.ToCatalogHRNString());
5453
const auto key =
55-
cache::KeyGenerator::CreateDataHandleKey(hrn, layer_id, data_handle);
54+
cache::KeyGenerator::CreateDataHandleKey(hrn_, layer_id, data_handle);
5655
OLP_SDK_LOG_DEBUG_F(kLogTag, "Put -> '%s'", key.c_str());
5756

5857
if (!cache_->Put(key, data, default_expiry_)) {
@@ -65,9 +64,8 @@ client::ApiNoResponse DataCacheRepository::Put(const model::Data& data,
6564

6665
boost::optional<model::Data> DataCacheRepository::Get(
6766
const std::string& layer_id, const std::string& data_handle) {
68-
const std::string hrn(hrn_.ToCatalogHRNString());
6967
const auto key =
70-
cache::KeyGenerator::CreateDataHandleKey(hrn, layer_id, data_handle);
68+
cache::KeyGenerator::CreateDataHandleKey(hrn_, layer_id, data_handle);
7169
OLP_SDK_LOG_DEBUG_F(kLogTag, "Get '%s'", key.c_str());
7270

7371
auto cached_data = cache_->Get(key);
@@ -80,19 +78,17 @@ boost::optional<model::Data> DataCacheRepository::Get(
8078

8179
bool DataCacheRepository::IsCached(const std::string& layer_id,
8280
const std::string& data_handle) const {
83-
const std::string hrn(hrn_.ToCatalogHRNString());
8481
const auto key =
85-
cache::KeyGenerator::CreateDataHandleKey(hrn, layer_id, data_handle);
82+
cache::KeyGenerator::CreateDataHandleKey(hrn_, layer_id, data_handle);
8683
OLP_SDK_LOG_DEBUG_F(kLogTag, "IsCached key -> '%s'", key.c_str());
8784

8885
return cache_->Contains(key);
8986
}
9087

9188
bool DataCacheRepository::Clear(const std::string& layer_id,
9289
const std::string& data_handle) {
93-
const std::string hrn(hrn_.ToCatalogHRNString());
9490
const auto key =
95-
cache::KeyGenerator::CreateDataHandleKey(hrn, layer_id, data_handle);
91+
cache::KeyGenerator::CreateDataHandleKey(hrn_, layer_id, data_handle);
9692
OLP_SDK_LOG_DEBUG_F(kLogTag, "Clear -> '%s'", key.c_str());
9793

9894
return cache_->RemoveKeysWithPrefix(key);

olp-cpp-sdk-dataservice-read/src/repositories/DataCacheRepository.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2021 HERE Europe B.V.
2+
* Copyright (C) 2019-2023 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -55,7 +55,7 @@ class DataCacheRepository final {
5555
bool Clear(const std::string& layer_id, const std::string& data_handle);
5656

5757
private:
58-
client::HRN hrn_;
58+
const std::string hrn_;
5959
std::shared_ptr<cache::KeyValueCache> cache_;
6060
time_t default_expiry_;
6161
};

olp-cpp-sdk-dataservice-read/src/repositories/NamedMutex.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2020-2022 HERE Europe B.V.
2+
* Copyright (C) 2020-2023 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -132,12 +132,12 @@ boost::optional<client::ApiError> NamedMutexStorage::GetError(
132132
return impl_->GetError(resource);
133133
}
134134

135-
NamedMutex::NamedMutex(NamedMutexStorage& storage, const std::string& name,
135+
NamedMutex::NamedMutex(NamedMutexStorage& storage, std::string name,
136136
client::CancellationContext& context)
137137
: storage_{storage},
138138
context_{context},
139139
is_locked_{false},
140-
name_{name},
140+
name_{std::move(name)},
141141
mutex_{storage_.AcquireLock(name_)},
142142
lock_condition_{storage_.GetLockCondition(name_)},
143143
lock_mutex_{storage_.GetLockMutex(name_)},

olp-cpp-sdk-dataservice-read/src/repositories/NamedMutex.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2020-2022 HERE Europe B.V.
2+
* Copyright (C) 2020-2023 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -80,7 +80,7 @@ class NamedMutexStorage {
8080
*/
8181
class NamedMutex final {
8282
public:
83-
NamedMutex(NamedMutexStorage& storage, const std::string& name,
83+
NamedMutex(NamedMutexStorage& storage, std::string name,
8484
client::CancellationContext& context);
8585

8686
NamedMutex(const NamedMutex&) = delete;

olp-cpp-sdk-dataservice-read/src/repositories/PartitionsCacheRepository.cpp

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2021 HERE Europe B.V.
2+
* Copyright (C) 2019-2023 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -95,10 +95,10 @@ client::ApiNoResponse PartitionsCacheRepository::Put(
9595
cache::KeyGenerator::CreatePartitionsKey(catalog_, layer_id_, version);
9696
OLP_SDK_LOG_DEBUG_F(kLogTag, "Put -> '%s'", key.c_str());
9797

98-
const auto put_result = cache_->Put(
99-
key, partition_ids,
100-
[&]() { return serializer::serialize(partition_ids); },
101-
expiry.get_value_or(default_expiry_));
98+
const auto put_result =
99+
cache_->Put(key, partition_ids,
100+
[&]() { return serializer::serialize(partition_ids); },
101+
expiry.get_value_or(default_expiry_));
102102

103103
if (!put_result) {
104104
OLP_SDK_LOG_ERROR_F(kLogTag, "Failed to write -> '%s'", key.c_str());
@@ -128,7 +128,7 @@ model::Partitions PartitionsCacheRepository::Get(
128128

129129
if (!cached_partition.empty()) {
130130
cached_partitions.emplace_back(
131-
boost::any_cast<model::Partition>(cached_partition));
131+
std::move(boost::any_cast<model::Partition&&>(cached_partition)));
132132
}
133133
}
134134

@@ -175,9 +175,9 @@ void PartitionsCacheRepository::Put(
175175
cache::KeyGenerator::CreateLayerVersionsKey(catalog_, catalog_version);
176176
OLP_SDK_LOG_DEBUG_F(kLogTag, "Put -> '%s'", key.c_str());
177177

178-
cache_->Put(
179-
key, layer_versions,
180-
[&]() { return serializer::serialize(layer_versions); }, default_expiry_);
178+
cache_->Put(key, layer_versions,
179+
[&]() { return serializer::serialize(layer_versions); },
180+
default_expiry_);
181181
}
182182

183183
boost::optional<model::LayerVersions> PartitionsCacheRepository::Get(
@@ -195,7 +195,8 @@ boost::optional<model::LayerVersions> PartitionsCacheRepository::Get(
195195
return boost::none;
196196
}
197197

198-
return boost::any_cast<model::LayerVersions>(cached_layer_versions);
198+
return std::move(
199+
boost::any_cast<model::LayerVersions&&>(cached_layer_versions));
199200
}
200201

201202
client::ApiNoResponse PartitionsCacheRepository::Put(
@@ -284,7 +285,8 @@ bool PartitionsCacheRepository::ClearPartitionMetadata(
284285
return true;
285286
}
286287

287-
out_partition = boost::any_cast<model::Partition>(cached_partition);
288+
out_partition =
289+
std::move(boost::any_cast<model::Partition&&>(cached_partition));
288290
return cache_->RemoveKeysWithPrefix(key);
289291
}
290292

@@ -295,6 +297,7 @@ bool PartitionsCacheRepository::GetPartitionHandle(
295297
catalog_, layer_id_, partition_id, catalog_version);
296298
OLP_SDK_LOG_DEBUG_F(kLogTag, "IsPartitionCached -> '%s'", key.c_str());
297299

300+
// Memory cache may save whole boost::any so sometimes there are no parsing
298301
auto cached_partition =
299302
cache_->Get(key, [](const std::string& serialized_object) {
300303
return parser::parse<model::Partition>(serialized_object);
@@ -303,8 +306,8 @@ bool PartitionsCacheRepository::GetPartitionHandle(
303306
if (cached_partition.empty()) {
304307
return false;
305308
}
306-
auto partition = boost::any_cast<model::Partition>(cached_partition);
307-
data_handle = partition.GetDataHandle();
309+
auto& partition = boost::any_cast<model::Partition&>(cached_partition);
310+
data_handle = std::move(partition.GetMutableDataHandle());
308311
return true;
309312
}
310313

olp-cpp-sdk-dataservice-read/src/repositories/PartitionsRepository.cpp

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -336,21 +336,16 @@ PartitionsResponse PartitionsRepository::GetPartitionById(
336336
}
337337

338338
auto fetch_option = request.GetFetchOption();
339+
const auto key = request.CreateKey(layer_id_, version);
339340

340-
const auto request_key =
341-
catalog_.ToString() + request.CreateKey(layer_id_, version);
342-
343-
NamedMutex mutex(storage_, request_key, context);
341+
NamedMutex mutex(storage_, catalog_.ToString() + key, context);
344342
std::unique_lock<repository::NamedMutex> lock(mutex, std::defer_lock);
345343

346344
// If we are not planning to go online or access the cache, do not lock.
347345
if (fetch_option != CacheOnly && fetch_option != OnlineOnly) {
348346
lock.lock();
349347
}
350348

351-
std::chrono::seconds timeout{settings_.retry_settings.timeout};
352-
const auto key = request.CreateKey(layer_id_, version);
353-
354349
const std::vector<std::string> partitions{partition_id.value()};
355350

356351
if (fetch_option != OnlineOnly && fetch_option != CacheWithUpdate) {

0 commit comments

Comments
 (0)