From 80c054faace0252d92de895191386f541b1fbfdd Mon Sep 17 00:00:00 2001
From: Sergey Yakubov <sergey.yakubov@desy.de>
Date: Tue, 19 Dec 2017 10:13:33 +0100
Subject: [PATCH] change FileData from vector to unique_ptr
---
common/cpp/include/common/file_info.h | 6 +--
.../cpp/include/system_wrappers/system_io.h | 10 ++--
common/cpp/src/system_io.cpp | 28 ++---------
common/cpp/src/system_io_linux.cpp | 2 +-
.../worker/process_folder/process_folder.cpp | 7 ++-
.../read_file_content/read_file_content.cpp | 7 +--
.../read_folder_content.cpp | 1 -
tests/system_io/read_files_in_folder/setup.sh | 6 +--
.../content_multithread.cpp | 6 +--
.../next_multithread/next_multithread.cpp | 7 +--
worker/api/cpp/include/worker/data_broker.h | 2 -
worker/api/cpp/src/folder_data_broker.h | 3 --
.../api/cpp/unittests/test_folder_broker.cpp | 47 ++++++++++---------
13 files changed, 55 insertions(+), 77 deletions(-)
diff --git a/common/cpp/include/common/file_info.h b/common/cpp/include/common/file_info.h
index a08967c1f..da182e342 100644
--- a/common/cpp/include/common/file_info.h
+++ b/common/cpp/include/common/file_info.h
@@ -2,7 +2,7 @@
#define HIDRA2_FILE_INFO_H
#include <cinttypes>
-#include <vector>
+#include <memory>
#include <chrono>
namespace hidra2 {
@@ -11,10 +11,10 @@ struct FileInfo {
std::string base_name;
std::string relative_path;
std::chrono::system_clock::time_point modify_date;
- uint64_t size;
+ uint64_t size{0};
};
-typedef std::vector<uint8_t> FileData;
+typedef std::unique_ptr<uint8_t[]> FileData;
}
diff --git a/common/cpp/include/system_wrappers/system_io.h b/common/cpp/include/system_wrappers/system_io.h
index 8bee93684..0f8fcb543 100644
--- a/common/cpp/include/system_wrappers/system_io.h
+++ b/common/cpp/include/system_wrappers/system_io.h
@@ -8,11 +8,11 @@ namespace hidra2 {
class SystemIO final : public IO {
public:
FileData GetDataFromFile(const std::string& fname, uint64_t fsize, IOErrors* err) override;
- int open(const char* __file, int __oflag);
- int close(int __fd);
- ssize_t read(int __fd, void* buf, size_t count);
- ssize_t write(int __fd, const void* __buf, size_t __n);
- std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err);
+ int open(const char* __file, int __oflag) override;
+ int close(int __fd) override;
+ ssize_t read(int __fd, void* buf, size_t count) override;
+ ssize_t write(int __fd, const void* __buf, size_t __n) override;
+ std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) override;
};
}
diff --git a/common/cpp/src/system_io.cpp b/common/cpp/src/system_io.cpp
index 6151c49f9..c92dbb0f1 100644
--- a/common/cpp/src/system_io.cpp
+++ b/common/cpp/src/system_io.cpp
@@ -1,6 +1,5 @@
#include <fcntl.h>
#include <unistd.h>
-#include <chrono>
#include <iostream>
#include <system_wrappers/system_io.h>
@@ -8,9 +7,6 @@
namespace hidra2 {
void ReadWholeFile(int fd, uint8_t* array, uint64_t fsize, IOErrors* err) {
-
- auto t1 = std::chrono::high_resolution_clock::now();
-
ssize_t totalbytes = 0;
ssize_t readbytes = 0;
do {
@@ -18,12 +14,6 @@ void ReadWholeFile(int fd, uint8_t* array, uint64_t fsize, IOErrors* err) {
totalbytes += readbytes;
} while (readbytes > 0 && totalbytes < fsize);
- auto t2 = std::chrono::high_resolution_clock::now();
-
- auto duration = std::chrono::duration_cast<std::chrono::milliseconds>( t2 - t1 ).count();
- std::cout << "Elapsed ReadWholeFile : " << duration << "ms" << std::endl;
-
-
if (totalbytes != fsize) {
*err = IOErrors::READ_ERROR;
}
@@ -33,28 +23,20 @@ FileData SystemIO::GetDataFromFile(const std::string& fname, uint64_t fsize, IOE
int fd = open(fname.c_str(), O_RDONLY);
*err = IOErrorFromErrno();
if (*err != IOErrors::NO_ERROR) {
- return {};
+ return nullptr;
}
- auto t1 = std::chrono::high_resolution_clock::now();
- FileData data(fsize);
-
- auto t2 = std::chrono::high_resolution_clock::now();
-
- auto duration = std::chrono::duration_cast<std::chrono::microseconds>( t2 - t1 ).count();
- std::cout << "Elapsed CreateVector : " << duration << "ms" << std::endl;
-
-
- ReadWholeFile(fd, &data[0], fsize, err);
+ FileData data{new uint8_t[fsize]};
+ ReadWholeFile(fd, data.get(), fsize, err);
if (*err != IOErrors::NO_ERROR) {
close(fd);
- return {};
+ return nullptr;
}
close(fd);
*err = IOErrorFromErrno();
if (*err != IOErrors::NO_ERROR) {
- return {};
+ return nullptr;
}
return data;
diff --git a/common/cpp/src/system_io_linux.cpp b/common/cpp/src/system_io_linux.cpp
index c4b3a5be8..81e874917 100644
--- a/common/cpp/src/system_io_linux.cpp
+++ b/common/cpp/src/system_io_linux.cpp
@@ -6,7 +6,7 @@
#include <sys/stat.h>
#include <algorithm>
-#include <errno.h>
+#include <cerrno>
using std::string;
using std::vector;
diff --git a/examples/worker/process_folder/process_folder.cpp b/examples/worker/process_folder/process_folder.cpp
index d1b596a07..b75d18bfc 100644
--- a/examples/worker/process_folder/process_folder.cpp
+++ b/examples/worker/process_folder/process_folder.cpp
@@ -4,6 +4,8 @@
#include <algorithm>
#include <thread>
#include <chrono>
+#include <iomanip>
+
#include "worker/data_broker.h"
@@ -50,10 +52,13 @@ int main(int argc, char* argv[]) {
high_resolution_clock::time_point t3 = high_resolution_clock::now();
auto duration_read = std::chrono::duration_cast<std::chrono::milliseconds>( t3 - t2 ).count();
+ double size_gb = double(size) / 1024 / 1024 / 1024;
+ double bandwidth = size_gb/duration_read*1000;
std::cout << "Processed " << nfiles << " files" << std::endl;
- std::cout << "Total size: " << size / 1024 / 1024 / 1024 << "GB" << std::endl;
+ std::cout << "Total size: " << std::setprecision(2) << size_gb << "GB" << std::endl;
std::cout << "Elapsed scan : " << duration_scan << "ms" << std::endl;
std::cout << "Elapsed read : " << duration_read << "ms" << std::endl;
+ std::cout << "Bandwidth: " << std::setprecision(2) << bandwidth << "GB/sec" << std::endl;
return 0;
}
diff --git a/tests/system_io/read_file_content/read_file_content.cpp b/tests/system_io/read_file_content/read_file_content.cpp
index f6af4619b..b039fcc64 100644
--- a/tests/system_io/read_file_content/read_file_content.cpp
+++ b/tests/system_io/read_file_content/read_file_content.cpp
@@ -1,8 +1,6 @@
#include <iostream>
#include <system_wrappers/system_io.h>
-#include <memory>
-#include <string>
#include "testing.h"
using hidra2::SystemIO;
@@ -27,9 +25,8 @@ int main(int argc, char* argv[]) {
result = "notfound";
break;
case IOErrors::NO_ERROR:
- for(auto symbol : data)
- result += symbol;
- M_AssertEq(expect.size(), result.size());
+ for(int i = 0; i< expect.size();i++)
+ result += data[i];
break;
case IOErrors::PERMISSIONS_DENIED:
result = "noaccess";
diff --git a/tests/system_io/read_files_in_folder/read_folder_content.cpp b/tests/system_io/read_files_in_folder/read_folder_content.cpp
index 62d69b20c..1a651e5f3 100644
--- a/tests/system_io/read_files_in_folder/read_folder_content.cpp
+++ b/tests/system_io/read_files_in_folder/read_folder_content.cpp
@@ -1,5 +1,4 @@
#include <iostream>
-#include <memory>
#include "system_wrappers/system_io.h"
#include "testing.h"
diff --git a/tests/system_io/read_files_in_folder/setup.sh b/tests/system_io/read_files_in_folder/setup.sh
index ad7a62176..4516ede2b 100644
--- a/tests/system_io/read_files_in_folder/setup.sh
+++ b/tests/system_io/read_files_in_folder/setup.sh
@@ -2,11 +2,11 @@
mkdir -p test/subtest/subtest2
touch test/2
-sleep 0.01
+sleep 0.1
touch test/3
-sleep 0.01
+sleep 0.1
touch test/subtest/subtest2/4
-sleep 0.01
+sleep 0.1
touch test/1
mkdir test_noaccess
diff --git a/tests/worker/connect_multithread/content_multithread.cpp b/tests/worker/connect_multithread/content_multithread.cpp
index f90da055f..a1f7fb36a 100644
--- a/tests/worker/connect_multithread/content_multithread.cpp
+++ b/tests/worker/connect_multithread/content_multithread.cpp
@@ -1,9 +1,7 @@
#include <iostream>
-#include <memory>
#include <vector>
-#include <algorithm>
#include <thread>
-
+#include <algorithm>
#include "worker/data_broker.h"
#include "testing.h"
@@ -49,7 +47,7 @@ int main(int argc, char* argv[]) {
std::vector<std::thread> threads;
for (int i = 0; i < args.nthreads; i++) {
- threads.push_back(std::thread([&, i] {
+ threads.emplace_back(std::thread([&, i] {
errors[i] = broker->Connect();
}));
}
diff --git a/tests/worker/next_multithread/next_multithread.cpp b/tests/worker/next_multithread/next_multithread.cpp
index b65e196d8..4107bd9f9 100644
--- a/tests/worker/next_multithread/next_multithread.cpp
+++ b/tests/worker/next_multithread/next_multithread.cpp
@@ -1,10 +1,7 @@
#include <iostream>
-#include <memory>
#include <vector>
-#include <algorithm>
#include <thread>
-#include <string>
-
+#include <algorithm>
#include "worker/data_broker.h"
#include "testing.h"
@@ -55,7 +52,7 @@ void ReadFiles(const Args& args) {
std::vector<std::thread> threads;
for (int i = 0; i < args.nthreads; i++) {
- threads.push_back(std::thread(exec_next, i));
+ threads.emplace_back(std::thread(exec_next, i));
}
for (auto& thread : threads) {
diff --git a/worker/api/cpp/include/worker/data_broker.h b/worker/api/cpp/include/worker/data_broker.h
index 49688e1e4..81d180477 100644
--- a/worker/api/cpp/include/worker/data_broker.h
+++ b/worker/api/cpp/include/worker/data_broker.h
@@ -3,9 +3,7 @@
#include <memory>
#include <string>
-#include <iostream>
#include <common/file_info.h>
-#include <vector>
namespace hidra2 {
diff --git a/worker/api/cpp/src/folder_data_broker.h b/worker/api/cpp/src/folder_data_broker.h
index ac7c007ab..5fd4f08e4 100644
--- a/worker/api/cpp/src/folder_data_broker.h
+++ b/worker/api/cpp/src/folder_data_broker.h
@@ -5,12 +5,9 @@
#include <string>
#include <mutex>
-#include <atomic>
-
#include "system_wrappers/io.h"
-
namespace hidra2 {
class FolderDataBroker final : public hidra2::DataBroker {
diff --git a/worker/api/cpp/unittests/test_folder_broker.cpp b/worker/api/cpp/unittests/test_folder_broker.cpp
index c7dfce1f6..7a6b9cda3 100644
--- a/worker/api/cpp/unittests/test_folder_broker.cpp
+++ b/worker/api/cpp/unittests/test_folder_broker.cpp
@@ -36,25 +36,30 @@ TEST(FolderDataBroker, SetCorrectIO) {
class FakeIO: public IO {
public:
- FileData GetDataFromFile(const std::string& fname, uint64_t fsize, IOErrors* err)override {
+
+ virtual uint8_t* GetDataFromFileProxy(const std::string& fname, uint64_t fsize, IOErrors* err) {
*err = IOErrors::NO_ERROR;
- return {};
+ return nullptr;
};
- int open(const char* __file, int __oflag) {
+ FileData GetDataFromFile(const std::string& fname, uint64_t fsize, IOErrors* err)override {
+ return FileData(GetDataFromFileProxy(fname,fsize,err));
+ };
+
+ int open(const char* __file, int __oflag)override {
return 0;
};
- int close(int __fd) {
+ int close(int __fd)override {
return 0;
};
- ssize_t read(int __fd, void* buf, size_t count) {
+ ssize_t read(int __fd, void* buf, size_t count)override {
return 0;
};
- ssize_t write(int __fd, const void* __buf, size_t __n) {
+ ssize_t write(int __fd, const void* __buf, size_t __n) override{
return 0;
};
- std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) {
+ std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) override{
*err = IOErrors::NO_ERROR;
std::vector<FileInfo> file_infos;
FileInfo fi;
@@ -72,7 +77,7 @@ class FakeIO: public IO {
class IOFolderNotFound: public FakeIO {
public:
- std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) {
+ std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) override{
*err = IOErrors::FILE_NOT_FOUND;
return {};
}
@@ -80,7 +85,7 @@ class IOFolderNotFound: public FakeIO {
class IOFodlerUnknownError: public FakeIO {
public:
- std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) {
+ std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) override{
*err = IOErrors::UNKWOWN_ERROR;
return {};
}
@@ -88,7 +93,7 @@ class IOFodlerUnknownError: public FakeIO {
class IOEmptyFodler: public FakeIO {
public:
- std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) {
+ std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) override{
*err = IOErrors::NO_ERROR;
return {};
}
@@ -96,7 +101,7 @@ class IOEmptyFodler: public FakeIO {
class IOCannotOpenFile: public FakeIO {
public:
- FileData GetDataFromFile(const std::string& fname, uint64_t fsize, IOErrors* err) {
+ FileData GetDataFromFile(const std::string& fname, uint64_t fsize, IOErrors* err) override {
*err = IOErrors::PERMISSIONS_DENIED;
return {};
};
@@ -207,7 +212,7 @@ TEST_F(FolderDataBrokerTests, GetNextReturnsErrorWhenFilePermissionsDenied) {
class OpenFileMock : public FakeIO {
public:
- MOCK_METHOD3(GetDataFromFile, FileData(const std::string&, uint64_t, IOErrors*));
+ MOCK_METHOD3(GetDataFromFileProxy, uint8_t* (const std::string&, uint64_t, IOErrors*));
};
@@ -228,16 +233,17 @@ class GetDataFromFileTests : public Test {
};
TEST_F(GetDataFromFileTests, GetNextCallsGetDataFileWithFileName) {
- EXPECT_CALL(mock, GetDataFromFile("/path/to/file/1", _, _)).
- WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::NO_ERROR), testing::Return(FileData{})));
+ EXPECT_CALL(mock, GetDataFromFileProxy("/path/to/file/1", _, _)).
+ WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::NO_ERROR), testing::Return(nullptr)));
data_broker->GetNext(&fi, &data);
}
+
TEST_F(GetDataFromFileTests, GetNextReturnsDataAndInfo) {
- EXPECT_CALL(mock, GetDataFromFile(_, _, _)).
- WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::NO_ERROR), testing::Return(FileData{'1'})));
+ EXPECT_CALL(mock, GetDataFromFileProxy(_, _, _)).
+ WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::NO_ERROR), testing::Return(new uint8_t[1]{'1'})));
data_broker->GetNext(&fi, &data);
@@ -247,8 +253,8 @@ TEST_F(GetDataFromFileTests, GetNextReturnsDataAndInfo) {
}
TEST_F(GetDataFromFileTests, GetNextReturnsOnlyData) {
- EXPECT_CALL(mock, GetDataFromFile(_, _, _)).
- WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::NO_ERROR), testing::Return(FileData{'1'})));
+ EXPECT_CALL(mock, GetDataFromFileProxy(_, _, _)).
+ WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::NO_ERROR), testing::Return(new uint8_t[1]{'1'})));
data_broker->GetNext(nullptr, &data);
@@ -257,13 +263,12 @@ TEST_F(GetDataFromFileTests, GetNextReturnsOnlyData) {
TEST_F(GetDataFromFileTests, GetNextReturnsErrorWhenCannotReadData) {
- EXPECT_CALL(mock, GetDataFromFile(_, _, _)).
- WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::READ_ERROR), testing::Return(FileData{})));
+ EXPECT_CALL(mock, GetDataFromFileProxy(_, _, _)).
+ WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::READ_ERROR), testing::Return(nullptr)));
auto err = data_broker->GetNext(&fi, &data);
ASSERT_THAT(err, Eq(WorkerErrorCode::ERROR_READING_FROM_SOURCE));
- ASSERT_TRUE(data.empty());
}
--
GitLab