microsoft
/
cpp_client_telemetry
зеркало из https://github.com/microsoft/cpp_client_telemetry.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Merged PR 833662: IStorageRecord::GetRecords should return a std::vector<T> 

Rather than return a raw pointer std::vector<StorageRecord>, thus inducing memory managment on the caller and breaking the raw-pointers-are-non-owning rule, IOfflineStorgage::GetRecords should just return the vector. std::vector<T> is cheap to move, and thus can easily be returned from a function on any modern compiler without inducing a massive copy cost.
This commit is contained in:
Matt Koscumb 2018-11-19 17:39:23 +00:00
Родитель a1b20804bd 0bd999eeea
Коммит 0425c83b24
9 изменённых файлов: 36 добавлений и 44 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

4
lib/include/aria/IOfflineStorage.hpp
Просмотреть файл

@ -101,7 +101,7 @@ namespace ARIASDK_NS_BEGIN {
class IOfflineStorage {
public:
virtual ~IOfflineStorage() {}
virtual ~IOfflineStorage() noexcept = default;
/// <summary>
/// Initialize the offline storage
@ -271,7 +271,7 @@ namespace ARIASDK_NS_BEGIN {
/// <param name="minPriority">lowest priority selected</param>
/// <param name="maxCount"> max count to be selected</param>
/// <returns>Value of the requested setting or an empty string</returns>
virtual std::vector<StorageRecord>* GetRecords(bool shutdown, EventLatency minLatency = EventLatency_Unspecified, unsigned maxCount = 0) = 0;
virtual std::vector<StorageRecord> GetRecords(bool shutdown, EventLatency minLatency = EventLatency_Unspecified, unsigned maxCount = 0) = 0;
virtual bool ResizeDb() = 0;

8
lib/offline/MemoryStorage.cpp
Просмотреть файл

@ -369,15 +369,15 @@ namespace ARIASDK_NS_BEGIN {
return numRecords;
}
std::vector<StorageRecord>* MemoryStorage::GetRecords(bool shutdown, EventLatency minLatency, unsigned maxCount)
std::vector<StorageRecord> MemoryStorage::GetRecords(bool shutdown, EventLatency minLatency, unsigned maxCount)
{
UNREFERENCED_PARAMETER(shutdown);
UNREFERENCED_PARAMETER(minLatency);
UNREFERENCED_PARAMETER(maxCount);
std::vector<StorageRecord>* records = new std::vector<StorageRecord>();
auto consumer = [records](StorageRecord&& record) -> bool {
records->push_back(std::move(record));
std::vector<StorageRecord> records;
auto consumer = [&records](StorageRecord&& record) -> bool {
records.push_back(std::move(record));
return true; // want more
};
GetAndReserveRecords(consumer, 0, minLatency, maxCount);

2
lib/offline/MemoryStorage.hpp
Просмотреть файл

@ -57,7 +57,7 @@ namespace ARIASDK_NS_BEGIN {
virtual size_t GetReservedCount();
virtual std::vector<StorageRecord>* GetRecords(bool shutdown = false, EventLatency minLatency = EventLatency_Unspecified, unsigned maxCount = 0) override;
virtual std::vector<StorageRecord> GetRecords(bool shutdown = false, EventLatency minLatency = EventLatency_Unspecified, unsigned maxCount = 0) override;
virtual bool ResizeDb() override;

17
lib/offline/OfflineStorageHandler.cpp
Просмотреть файл

@ -147,7 +147,7 @@ namespace ARIASDK_NS_BEGIN {
if ((m_offlineStorageMemory) && (dbSizeBeforeFlush > 0) && (m_offlineStorageDisk))
{
std::vector<StorageRecord>* records = m_offlineStorageMemory->GetRecords(false, EventLatency_Unspecified);
auto records = m_offlineStorageMemory->GetRecords(false, EventLatency_Unspecified);
std::vector<StorageRecordId> ids;
size_t totalSaved = 0;
@ -156,12 +156,12 @@ namespace ARIASDK_NS_BEGIN {
// if (sqlite)
// sqlite->Execute("BEGIN");
while (records->size())
while (records.size())
{
ids.push_back(records->back().id);
if (m_offlineStorageDisk->StoreRecord(std::move(records->back())))
ids.push_back(records.back().id);
if (m_offlineStorageDisk->StoreRecord(std::move(records.back())))
totalSaved++;
records->pop_back();
records.pop_back();
}
// TODO: [MG] - consider running the batch in transaction
@ -173,8 +173,6 @@ namespace ARIASDK_NS_BEGIN {
bool fromMemory = true;
m_offlineStorageMemory->DeleteRecords(ids, dummy, fromMemory);
delete records;
// Notify event listener about the records cached
OnStorageRecordsSaved(totalSaved);
@ -305,7 +303,7 @@ namespace ARIASDK_NS_BEGIN {
return returnValue;
}
std::vector<StorageRecord>* OfflineStorageHandler::GetRecords(bool shutdown, EventLatency minLatency, unsigned maxCount)
std::vector<StorageRecord> OfflineStorageHandler::GetRecords(bool shutdown, EventLatency minLatency, unsigned maxCount)
{
// This method should not be called directly because it's a no-op
assert(false);
@ -313,8 +311,7 @@ namespace ARIASDK_NS_BEGIN {
UNREFERENCED_PARAMETER(shutdown);
UNREFERENCED_PARAMETER(minLatency);
UNREFERENCED_PARAMETER(maxCount);
std::vector<StorageRecord>* records = new std::vector<StorageRecord>();
return records;
return std::vector<StorageRecord>{};
}
void OfflineStorageHandler::DeleteRecords(std::vector<StorageRecordId> const& ids, HttpHeaders headers, bool& fromMemory)

2
lib/offline/OfflineStorageHandler.hpp
Просмотреть файл

@ -35,7 +35,7 @@ namespace ARIASDK_NS_BEGIN {
virtual size_t GetSize() override;
virtual size_t GetRecordCount(EventLatency latency = EventLatency_Unspecified) const override;
virtual std::vector<StorageRecord>* GetRecords(bool shutdown, EventLatency minLatency = EventLatency_Unspecified, unsigned maxCount = 0) override;
virtual std::vector<StorageRecord> GetRecords(bool shutdown, EventLatency minLatency = EventLatency_Unspecified, unsigned maxCount = 0) override;
virtual bool ResizeDb() override;
virtual void OnStorageOpened(std::string const& type) override;

8
lib/offline/OfflineStorage_SQLite.cpp
Просмотреть файл

@ -311,9 +311,9 @@ namespace ARIASDK_NS_BEGIN {
return m_lastReadCount;
}
std::vector<StorageRecord>* OfflineStorage_SQLite::GetRecords(bool shutdown, EventLatency minLatency, unsigned maxCount)
std::vector<StorageRecord> OfflineStorage_SQLite::GetRecords(bool shutdown, EventLatency minLatency, unsigned maxCount)
{
std::vector<StorageRecord>* records = new std::vector<StorageRecord>();
std::vector<StorageRecord> records;
StorageRecord record;
if (shutdown)
@ -325,7 +325,7 @@ namespace ARIASDK_NS_BEGIN {
while (selectStmt.getRow(record.id, record.tenantToken, latency, record.timestamp, record.retryCount, record.reservedUntil, record.blob))
{
record.latency = static_cast<EventLatency>(latency);
records->push_back(record);
records.push_back(record);
}
selectStmt.reset();
}
@ -339,7 +339,7 @@ namespace ARIASDK_NS_BEGIN {
while (selectStmt.getRow(record.id, record.tenantToken, latency, record.timestamp, record.retryCount, record.reservedUntil, record.blob))
{
record.latency = static_cast<EventLatency>(latency);
records->push_back(record);
records.push_back(record);
}
selectStmt.reset();
}

2
lib/offline/OfflineStorage_SQLite.hpp
Просмотреть файл

@ -41,7 +41,7 @@ namespace ARIASDK_NS_BEGIN {
virtual std::string GetSetting(std::string const& name) override;
virtual size_t GetSize() override;
virtual size_t GetRecordCount(EventLatency latency) const override;
virtual std::vector<StorageRecord>* GetRecords(bool shutdown, EventLatency minLatency = EventLatency_Normal, unsigned maxCount = 0) override;
virtual std::vector<StorageRecord> GetRecords(bool shutdown, EventLatency minLatency = EventLatency_Normal, unsigned maxCount = 0) override;
virtual bool ResizeDb() override;
protected:

2
tests/common/MockIOfflineStorage.hpp
Просмотреть файл

@ -25,7 +25,7 @@ class MockIOfflineStorage : public ARIASDK_NS::IOfflineStorage {
MOCK_METHOD1(GetSetting, std::string(std::string const &));
MOCK_METHOD0(GetSize, size_t());
MOCK_CONST_METHOD1(GetRecordCount, size_t(ARIASDK_NS::EventLatency));
MOCK_METHOD3(GetRecords, std::vector<ARIASDK_NS::StorageRecord>*(bool, ARIASDK_NS::EventLatency, unsigned));
MOCK_METHOD3(GetRecords, std::vector<ARIASDK_NS::StorageRecord>(bool, ARIASDK_NS::EventLatency, unsigned));
MOCK_METHOD0(ResizeDb, bool());
};

35
tests/unittests/MemoryStorageTests.cpp
Просмотреть файл

@ -127,13 +127,12 @@ TEST(MemoryStorageTests, StoreAndGetRecords)
EXPECT_THAT(storage.GetSize(), total_db_size);
// Verify the contents
size_t total_records = num_iterations * latencies.size();
std::unique_ptr< std::vector<StorageRecord> > records;
for (auto latency : { EventLatency_Max , EventLatency_RealTime , EventLatency_CostDeferred , EventLatency_Normal })
{
records.reset(storage.GetRecords(false, latency, 0));
EXPECT_THAT(records->size(), num_iterations);
EXPECT_THAT(records->size(), storage.LastReadRecordCount());
total_records -= records->size();
auto records = storage.GetRecords(false, latency, 0);
EXPECT_THAT(records.size(), num_iterations);
EXPECT_THAT(records.size(), storage.LastReadRecordCount());
total_records -= records.size();
}
EXPECT_THAT(storage.GetSize(), 0);
EXPECT_THAT(total_records, 0);
@ -146,10 +145,9 @@ TEST(MemoryStorageTests, StoreAndGetRecords)
EXPECT_THAT(storage.GetSize(), total_db_size);
// Verify the contents
size_t total_records = num_iterations * latencies.size();
std::unique_ptr< std::vector<StorageRecord> > records;
records.reset(storage.GetRecords(false, EventLatency_Normal, 0));
EXPECT_THAT(records->size(), total_records);
EXPECT_THAT(records->size(), storage.LastReadRecordCount());
auto records = storage.GetRecords(false, EventLatency_Normal, 0);
EXPECT_THAT(records.size(), total_records);
EXPECT_THAT(records.size(), storage.LastReadRecordCount());
EXPECT_THAT(storage.GetSize(), 0);
}
@ -168,14 +166,13 @@ TEST(MemoryStorageTests, GetRecordsDeletesRecords)
EXPECT_THAT(storage.GetSize(), total_db_size);
// Retrieve those into records
std::unique_ptr< std::vector<StorageRecord> > records;
records.reset(storage.GetRecords());
auto records = storage.GetRecords();
// Storage size is "zero" because all records are fetched
EXPECT_THAT(storage.GetSize(), 0);
EXPECT_THAT(storage.GetRecordCount(), 0);
EXPECT_THAT(storage.GetReservedCount(), 0);
EXPECT_THAT(records->size(), num_iterations * 4); // 4 latencies
EXPECT_THAT(records.size(), num_iterations * 4); // 4 latencies
}
TEST(MemoryStorageTests, ReleaseRecords)
@ -192,11 +189,10 @@ TEST(MemoryStorageTests, ReleaseRecords)
auto total_records = storage.GetRecordCount();
// Retrieve those into records
std::unique_ptr< std::vector<StorageRecord> > records;
records.reset(new std::vector<StorageRecord>);
std::vector<StorageRecord> records;
auto consumer = [&records](StorageRecord&& record) -> bool {
records->push_back(std::move(record));
records.push_back(std::move(record));
return true; // want more
};
@ -206,10 +202,10 @@ TEST(MemoryStorageTests, ReleaseRecords)
EXPECT_THAT(storage.GetSize(), 0);
EXPECT_THAT(storage.GetRecordCount(), 0);
EXPECT_THAT(storage.GetReservedCount(), total_records);
EXPECT_THAT(records->size(), total_records);
EXPECT_THAT(records.size(), total_records);
// Track IDs of all reserved records
for (auto &record : *records)
for (auto &record : records)
{
ids.push_back(record.id);
}
@ -262,12 +258,11 @@ TEST(MemoryStorageTests, MultiThreadPerfTest)
addEvents(storage);
// Retrieve them
std::unique_ptr< std::vector<StorageRecord> > records;
records.reset(storage.GetRecords());
auto records = storage.GetRecords();
// Acquire IDs
std::vector<StorageRecordId> ids;
for (auto &record : *records)
for (const auto &record : records)
ids.push_back(record.id);
// Release records