Kelson has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/295518 )
Change subject: Use a Queue object to handle threadsafe access to a queue. ...................................................................... Use a Queue object to handle threadsafe access to a queue. By using a Queue object we avoid the declaration of popFromFilenameQueue function in articlesource.cpp. Change-Id: Ic685f7e22e4ce95f6e0eb280f65809fd0dff1a6a --- M zimwriterfs/articlesource.cpp M zimwriterfs/articlesource.h A zimwriterfs/queue.h M zimwriterfs/zimwriterfs.cpp 4 files changed, 116 insertions(+), 55 deletions(-) Approvals: Kelson: Verified; Looks good to me, approved diff --git a/zimwriterfs/articlesource.cpp b/zimwriterfs/articlesource.cpp index 8b0b34c..6cf5f30 100644 --- a/zimwriterfs/articlesource.cpp +++ b/zimwriterfs/articlesource.cpp @@ -28,7 +28,6 @@ #include <sstream> #include <map> -bool popFromFilenameQueue(std::string &filename); bool isVerbose(); extern std::string welcome; @@ -44,8 +43,9 @@ unsigned int dataSize = 0; - -ArticleSource::ArticleSource() { +ArticleSource::ArticleSource(Queue<std::string>& filenameQueue): + filenameQueue(filenameQueue) +{ /* Prepare metadata */ metadataQueue.push("Language"); metadataQueue.push("Publisher"); @@ -88,10 +88,10 @@ std::string line = redirectsQueue.front(); redirectsQueue.pop(); article = new RedirectArticle(line); - } else if (popFromFilenameQueue(path)) { + } else if (filenameQueue.popFromQueue(path)) { do { article = new Article(path); - } while (article && article->isInvalid() && popFromFilenameQueue(path)); + } while (article && article->isInvalid() && filenameQueue.popFromQueue(path)); } else { article = NULL; } diff --git a/zimwriterfs/articlesource.h b/zimwriterfs/articlesource.h index adbdbda..1ad6524 100644 --- a/zimwriterfs/articlesource.h +++ b/zimwriterfs/articlesource.h @@ -24,12 +24,13 @@ #include <string> #include <queue> #include <fstream> +#include "queue.h" #include <zim/writer/zimcreator.h> class ArticleSource : public zim::writer::ArticleSource { public: - explicit ArticleSource(); + explicit ArticleSource(Queue<std::string>& filenameQueue); virtual const zim::writer::Article* getNextArticle(); virtual zim::Blob getData(const std::string& aid); virtual std::string getMainPage(); @@ -39,6 +40,7 @@ private: std::queue<std::string> metadataQueue; std::queue<std::string> redirectsQueue; + Queue<std::string>& filenameQueue; }; #endif //OPENZIM_ZIMWRITERFS_ARTICLESOURCE_H diff --git a/zimwriterfs/queue.h b/zimwriterfs/queue.h new file mode 100644 index 0000000..d177568 --- /dev/null +++ b/zimwriterfs/queue.h @@ -0,0 +1,88 @@ +/* + * Copyright 2016 Matthieu Gautier <mgaut...@kymeria.fr> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301, USA. + */ + +#ifndef OPENZIM_ZIMWRITERFS_QUEUE_H +#define OPENZIM_ZIMWRITERFS_QUEUE_H + +#define MAX_QUEUE_SIZE 100 + +#include <pthread.h> +#include <unistd.h> + +template<typename T> +class Queue { + public: + Queue() {pthread_mutex_init(&m_queueMutex,NULL);}; + virtual ~Queue() {pthread_mutex_destroy(&m_queueMutex);}; + virtual bool isEmpty(); + virtual void pushToQueue(const T& element); + virtual bool popFromQueue(T &filename); + + protected: + std::queue<T> m_realQueue; + pthread_mutex_t m_queueMutex; + + private: + // Make this queue non copyable + Queue(const Queue&); + Queue& operator=(const Queue&); +}; + +template<typename T> +bool Queue<T>::isEmpty() { + pthread_mutex_lock(&m_queueMutex); + bool retVal = m_realQueue.empty(); + pthread_mutex_unlock(&m_queueMutex); + return retVal; +} + +template<typename T> +void Queue<T>::pushToQueue(const T &element) { + unsigned int wait = 0; + unsigned int queueSize = 0; + + do { + usleep(wait); + pthread_mutex_lock(&m_queueMutex); + queueSize = m_realQueue.size(); + pthread_mutex_unlock(&m_queueMutex); + wait += 10; + } while (queueSize > MAX_QUEUE_SIZE); + + pthread_mutex_lock(&m_queueMutex); + m_realQueue.push(element); + pthread_mutex_unlock(&m_queueMutex); +} + +template<typename T> +bool Queue<T>::popFromQueue(T &element) { + pthread_mutex_lock(&m_queueMutex); + if (m_realQueue.empty()) { + pthread_mutex_unlock(&m_queueMutex); + return false; + } + + element = m_realQueue.front(); + m_realQueue.pop(); + pthread_mutex_unlock(&m_queueMutex); + + return true; +} + +#endif // OPENZIM_ZIMWRITERFS_QUEUE_H \ No newline at end of file diff --git a/zimwriterfs/zimwriterfs.cpp b/zimwriterfs/zimwriterfs.cpp index de44cb8..09a62af 100644 --- a/zimwriterfs/zimwriterfs.cpp +++ b/zimwriterfs/zimwriterfs.cpp @@ -35,8 +35,7 @@ #include "tools.h" #include "article.h" #include "articlesource.h" - -#define MAX_QUEUE_SIZE 100 +#include "queue.h" std::string language; std::string creator; @@ -50,8 +49,6 @@ std::string zimPath; zim::writer::ZimCreator zimCreator; pthread_t directoryVisitor; -pthread_mutex_t filenameQueueMutex; -std::queue<std::string> filenameQueue; bool isDirectoryVisitorRunningFlag = false; pthread_mutex_t directoryVisitorRunningMutex; @@ -83,50 +80,26 @@ return retVal; } -bool isFilenameQueueEmpty() { - pthread_mutex_lock(&filenameQueueMutex); - bool retVal = filenameQueue.empty(); - pthread_mutex_unlock(&filenameQueueMutex); - return retVal; -} -void pushToFilenameQueue(const std::string &filename) { - unsigned int wait = 0; - unsigned int queueSize = 0; +class FilenameQueue: public Queue<std::string> { + bool popFromQueue(std::string &filename) { + bool retVal = false; + unsigned int wait = 0; - do { - usleep(wait); - pthread_mutex_lock(&filenameQueueMutex); - unsigned queueSize = filenameQueue.size(); - pthread_mutex_unlock(&filenameQueueMutex); - wait += 10; - } while (queueSize > MAX_QUEUE_SIZE); + do { + usleep(wait); + retVal = Queue::popFromQueue(filename); + if (retVal) { + break; + } + wait += 10; + } while (isDirectoryVisitorRunning() || !isEmpty()); - pthread_mutex_lock(&filenameQueueMutex); - filenameQueue.push(filename); - pthread_mutex_unlock(&filenameQueueMutex); -} - -bool popFromFilenameQueue(std::string &filename) { - bool retVal = false; - unsigned int wait = 0; - - do { - usleep(wait); - if (!isFilenameQueueEmpty()) { - pthread_mutex_lock(&filenameQueueMutex); - filename = filenameQueue.front(); - filenameQueue.pop(); - pthread_mutex_unlock(&filenameQueueMutex); - retVal = true; - break; - } else { - wait += 10; + return retVal; } - } while (isDirectoryVisitorRunning() || !isFilenameQueueEmpty()); +}; - return retVal; -} +FilenameQueue filenameQueue; /* Non ZIM related code */ void usage() { @@ -196,7 +169,7 @@ switch (entry->d_type) { case DT_REG: - pushToFilenameQueue(fullEntryName); + filenameQueue.pushToQueue(fullEntryName); break; case DT_DIR: visitDirectory(fullEntryName); @@ -211,7 +184,7 @@ std::cerr << "Unable to deal with " << fullEntryName << " (this is a named pipe)" << std::endl; break; case DT_LNK: - pushToFilenameQueue(fullEntryName); + filenameQueue.pushToQueue(fullEntryName); break; case DT_SOCK: std::cerr << "Unable to deal with " << fullEntryName << " (this is a UNIX domain socket)" << std::endl; @@ -220,7 +193,7 @@ struct stat s; if (stat(fullEntryName.c_str(), &s) == 0) { if (S_ISREG(s.st_mode)) { - pushToFilenameQueue(fullEntryName); + filenameQueue.pushToQueue(fullEntryName); } else if (S_ISDIR(s.st_mode)) { visitDirectory(fullEntryName); } else { @@ -251,7 +224,7 @@ } int main(int argc, char** argv) { - ArticleSource source; + ArticleSource source(filenameQueue); int minChunkSize = 2048; @@ -371,7 +344,6 @@ /* Init */ magic = magic_open(MAGIC_MIME); magic_load(magic, NULL); - pthread_mutex_init(&filenameQueueMutex, NULL); pthread_mutex_init(&directoryVisitorRunningMutex, NULL); pthread_mutex_init(&verboseMutex, NULL); @@ -392,5 +364,4 @@ /* Destroy mutex */ pthread_mutex_destroy(&directoryVisitorRunningMutex); pthread_mutex_destroy(&verboseMutex); - pthread_mutex_destroy(&filenameQueueMutex); } -- To view, visit https://gerrit.wikimedia.org/r/295518 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ic685f7e22e4ce95f6e0eb280f65809fd0dff1a6a Gerrit-PatchSet: 1 Gerrit-Project: openzim Gerrit-Branch: master Gerrit-Owner: Mgautierfr <mgaut...@kymeria.fr> Gerrit-Reviewer: Kelson <kel...@kiwix.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits