From fb7f49a6bba5d5213fb9237d9188b5d6019c8a42 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Mon, 15 Jun 2015 02:05:56 +0200 Subject: [PATCH] Making single file download fully (and correctly) multithreaded --- apps/Launcher/syncwidget.cpp | 32 +-- include/openspace/engine/downloadmanager.h | 36 ++-- src/engine/downloadmanager.cpp | 238 +++++++++++---------- 3 files changed, 163 insertions(+), 143 deletions(-) diff --git a/apps/Launcher/syncwidget.cpp b/apps/Launcher/syncwidget.cpp index 668194ee51..4158ba6c79 100644 --- a/apps/Launcher/syncwidget.cpp +++ b/apps/Launcher/syncwidget.cpp @@ -168,16 +168,18 @@ void SyncWidget::handleDirectFiles() { qDebug() << f.url << " -> " << f.destination; auto finishedCallback = - [w](const ghoul::filesystem::File& f) { - qDebug() << QString::fromStdString(f.filename()) << "finished"; - delete w; - qApp->processEvents(); + [w](const openspace::DownloadManager::FileFuture& f) { + std::cout << f.filePath << ": Finished" << std::endl; + //qDebug() << QString::fromStdString(f.file.filename()) << "finished"; + //delete w; + //qApp->processEvents(); }; auto progressCallback = - [w](const ghoul::filesystem::File& f, float progress) { - qDebug() << QString::fromStdString(f.filename()) << ": " << progress; - w->update(progress); - qApp->processEvents(); + [w](const openspace::DownloadManager::FileFuture& f) { + std::cout << f.filePath << ": " << f.progress << std::endl; + //qDebug() << QString::fromStdString(f.file.filename()) << ": " << f.progress; + //w->update(f.progress); + //qApp->processEvents(); }; DlManager.downloadFile( @@ -214,13 +216,13 @@ void SyncWidget::handleFileRequest() { std::string identifier = f.identifier.toStdString(); std::string path = fullPath(f.module, f.destination).toStdString(); int version = f.version; - DlManager.downloadRequestFiles( - identifier, - path, - version, - finishedCallback, - progressCallback - ); + //DlManager.downloadRequestFiles( + // identifier, + // path, + // version, + // finishedCallback, + // progressCallback + //); } } diff --git a/include/openspace/engine/downloadmanager.h b/include/openspace/engine/downloadmanager.h index b000620b6e..b3f651b038 100644 --- a/include/openspace/engine/downloadmanager.h +++ b/include/openspace/engine/downloadmanager.h @@ -28,40 +28,44 @@ #include #include #include + +#include #include #include namespace openspace { +// Multithreaded class DownloadManager : public ghoul::Singleton { public: - typedef std::function < - void(const ghoul::filesystem::File&, float progress) - > DownloadProgressCallback; - typedef std::function< - void (const ghoul::filesystem::File&) - > DownloadFinishedCallback; + struct FileFuture { + FileFuture(std::string file); - //struct FileFuture { + float progress; // [0,1] + std::string filePath; + std::string errorMessage; + }; - //}; + typedef std::function DownloadProgressCallback; + typedef std::function DownloadFinishedCallback; DownloadManager(std::string requestURL, int applicationVersion); - bool downloadFile( + // callers responsibility to delete + FileFuture* downloadFile( const std::string& url, const ghoul::filesystem::File& file, DownloadFinishedCallback finishedCallback = DownloadFinishedCallback(), DownloadProgressCallback progressCallback = DownloadProgressCallback() ); - bool downloadRequestFiles( - const std::string& identifier, - const ghoul::filesystem::Directory& destination, - int version, - DownloadFinishedCallback finishedCallback = DownloadFinishedCallback(), - DownloadProgressCallback progressCallback = DownloadProgressCallback() - ); + //bool downloadRequestFiles( + // const std::string& identifier, + // const ghoul::filesystem::Directory& destination, + // int version, + // DownloadFinishedCallback finishedCallback = DownloadFinishedCallback(), + // DownloadProgressCallback progressCallback = DownloadProgressCallback() + //); private: std::string _requestURL; diff --git a/src/engine/downloadmanager.cpp b/src/engine/downloadmanager.cpp index b05bd42bcc..68fc47aae3 100644 --- a/src/engine/downloadmanager.cpp +++ b/src/engine/downloadmanager.cpp @@ -28,7 +28,7 @@ #include #include -//#include +#include #ifdef OPENSPACE_CURL_ENABLED #include @@ -41,46 +41,47 @@ namespace { const std::string RequestFileVersion = "file_version"; const std::string RequestApplicationVersion = "application_version"; + struct ProgressInformation { + CURL* curl; + openspace::DownloadManager::FileFuture* future; + const openspace::DownloadManager::DownloadProgressCallback* callback; + }; + size_t writeData(void* ptr, size_t size, size_t nmemb, FILE* stream) { size_t written; written = fwrite(ptr, size, nmemb, stream); return written; } - struct Progress { - CURL* curl; - const ghoul::filesystem::File* file; - openspace::DownloadManager::DownloadProgressCallback* callback; - }; - int xferinfo(void *p, + int xferinfo(void* p, curl_off_t dltotal, curl_off_t dlnow, curl_off_t ultotal, curl_off_t ulnow) { - Progress* myp = static_cast(p); + if (dltotal == 0) + return 0; - if (myp->callback && *(myp->callback)) { - (*(myp->callback))( - *(myp->file), - static_cast(dlnow) / static_cast(dltotal) - ); + ghoul_assert(p, "Passed progress information is nullptr"); + ProgressInformation* i = static_cast(p); + ghoul_assert(i, "Passed pointer is not a ProgressInformation"); + ghoul_assert(i->curl, "CURL pointer is nullptr"); + ghoul_assert(i->future, "FileFuture is not initialized"); + ghoul_assert(i->callback, "Callback pointer is nullptr"); + + ghoul_assert(dltotal, "Download total is 0"); + ghoul_assert(dlnow <= dltotal, "Downloaded filesize is bigger then total size"); + i->future->progress = static_cast(dlnow) / static_cast(dltotal); + + if (*(i->callback)) { + // The callback function is a pointer to an std::function; that is the reason + // for the excessive referencing + (*(i->callback))(*(i->future)); } - //if (*(myp->callback)) { - // *(myp->callback)( - // myp->file, - // static_cast(dlnow) / static_cast(dltotal) - // ); - //} //CURL* curl = myp->curl; - - - //double curtime = 0; - //curl_easy_getinfo(curl, CURLINFO_TOTAL_TIME, &curtime); - //fprintf(stderr, "UP: %" CURL_FORMAT_CURL_OFF_T " of %" CURL_FORMAT_CURL_OFF_T // " DOWN: %" CURL_FORMAT_CURL_OFF_T " of %" CURL_FORMAT_CURL_OFF_T // "\r\n", @@ -88,9 +89,6 @@ namespace { return 0; } - - - } namespace openspace { @@ -100,104 +98,120 @@ DownloadManager::DownloadManager(std::string requestURL, int applicationVersion) , _applicationVersion(std::move(applicationVersion)) { curl_global_init(CURL_GLOBAL_ALL); - // Check if URL is accessible + // TODO: Check if URL is accessible ---abock + // TODO: Allow for multiple requestURLs } -bool DownloadManager::downloadFile( +DownloadManager::FileFuture* DownloadManager::downloadFile( const std::string& url, const ghoul::filesystem::File& file, DownloadFinishedCallback finishedCallback, DownloadProgressCallback progressCallback) { - CURL* curl = curl_easy_init(); - if (curl) { - - FILE* fp = fopen(file.path().c_str(), "wb"); - curl_easy_setopt(curl, CURLOPT_URL, url.c_str()); - curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L); - curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writeData); - curl_easy_setopt(curl, CURLOPT_WRITEDATA, fp); - - if (progressCallback) { - Progress p = { curl, &file, &progressCallback }; - curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, xferinfo); - curl_easy_setopt(curl, CURLOPT_XFERINFODATA, &p); - curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L); - } - - LDEBUG("Starting download for file: '" << url << - "' into file '" << file.path() << "'"); - CURLcode res = curl_easy_perform(curl); - curl_easy_cleanup(curl); - fclose(fp); - - if (res != CURLE_OK) { - LERROR("Error downloading file 'url': " << curl_easy_strerror(res)); - return false; - } - - if (finishedCallback) - finishedCallback(file); - return true; - } -} - -bool DownloadManager::downloadRequestFiles( - const std::string& identifier, - const ghoul::filesystem::Directory& destination, - int version, - DownloadFinishedCallback finishedCallback, - DownloadProgressCallback progressCallback) -{ - bool s = FileSys.createDirectory(destination, true); - // TODO: Check s ---abock - // TODO: Escaping is necessary ---abock - const std::string fullRequest =_requestURL + "?" + - RequestIdentifier + "=" + identifier + "&" + - RequestFileVersion + "=" + std::to_string(version) + "&" + - RequestApplicationVersion + "=" + std::to_string(_applicationVersion); - LDEBUG("Request: " << fullRequest); - - std::string requestFile = absPath("${TEMPORARY}/" + identifier); - LDEBUG("Request File: " << requestFile); - - //std::vector threads; - - bool success = downloadFile( - fullRequest, - requestFile, - [destination, &progressCallback](const ghoul::filesystem::File& f) { - LDEBUG("Finished: " << f.path()); - std::ifstream temporary(f.path()); - std::string line; - int nFiles = 0; - int nFinished = 0; - while (std::getline(temporary, line)) { - ++nFiles; - std::string file = ghoul::filesystem::File(line).filename(); - - LDEBUG("\tLine: " << line << " ; Dest: " << destination.path() + "/" + file); - //threads.push_back( - //std::thread([&nFinished, line, destination, file, progressCallback](){ - DlManager.downloadFile( - line, - destination.path() + "/" + file, - [&nFinished](const ghoul::filesystem::File& f) { ++nFinished; }, - [&progressCallback](const ghoul::filesystem::File& f, float progress) { progressCallback(f, progress); } - //);} - //) - ); - } - } + FileFuture* future = new FileFuture( + file.filename() ); + FILE* fp = fopen(file.path().c_str(), "wb"); - //for (std::thread& t : threads) - //t.join(); + LDEBUG("Starting download for file: '" << url << + "' into file '" << file.path() << "'"); + std::thread([url, finishedCallback, progressCallback, future, fp]() { + CURL* curl = curl_easy_init(); + if (curl) { + curl_easy_setopt(curl, CURLOPT_URL, url.c_str()); + curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, fp); + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writeData); + if (progressCallback) { + ProgressInformation p = { + curl, + future, + &progressCallback + }; + curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, xferinfo); + curl_easy_setopt(curl, CURLOPT_XFERINFODATA, &p); + curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L); + } + CURLcode res = curl_easy_perform(curl); + curl_easy_cleanup(curl); + fclose(fp); - return true; + if (res != CURLE_OK) { + future->errorMessage = curl_easy_strerror(res); + } + + if (finishedCallback) + finishedCallback(*future); + } + }).detach(); + + return future; } +//bool DownloadManager::downloadRequestFiles( +// const std::string& identifier, +// const ghoul::filesystem::Directory& destination, +// int version, +// DownloadFinishedCallback finishedCallback, +// DownloadProgressCallback progressCallback) +//{ +// bool s = FileSys.createDirectory(destination, true); +// // TODO: Check s ---abock +// // TODO: Escaping is necessary ---abock +// const std::string fullRequest =_requestURL + "?" + +// RequestIdentifier + "=" + identifier + "&" + +// RequestFileVersion + "=" + std::to_string(version) + "&" + +// RequestApplicationVersion + "=" + std::to_string(_applicationVersion); +// LDEBUG("Request: " << fullRequest); +// +// std::string requestFile = absPath("${TEMPORARY}/" + identifier); +// LDEBUG("Request File: " << requestFile); +// +// //std::vector threads; +// +// bool success = downloadFile( +// fullRequest, +// requestFile, +// [destination, &progressCallback](const ghoul::filesystem::File& f) { +// LDEBUG("Finished: " << f.path()); +// std::ifstream temporary(f.path()); +// std::string line; +// int nFiles = 0; +// int nFinished = 0; +// while (std::getline(temporary, line)) { +// ++nFiles; +// std::string file = ghoul::filesystem::File(line).filename(); +// +// LDEBUG("\tLine: " << line << " ; Dest: " << destination.path() + "/" + file); +// //threads.push_back( +// //std::thread([&nFinished, line, destination, file, progressCallback](){ +// DlManager.downloadFile( +// line, +// destination.path() + "/" + file, +// [&nFinished](const ghoul::filesystem::File& f) { ++nFinished; }, +// [&progressCallback](const ghoul::filesystem::File& f, float progress) { progressCallback(f, progress); } +// //);} +// //) +// ); +// } +// } +// ); +// +// //for (std::thread& t : threads) +// //t.join(); +// +// +// +// return true; +//} + + + +DownloadManager::FileFuture::FileFuture(std::string file) + : progress(0.f) + , filePath(std::move(file)) +{} } // namespace openspace