Here is what I've got. It work and seems to be fast. A few notes, though. 1) I'm still investigating how to solve the startup problem. (images get loaded in reverse order). Clues welcomed.
2) I've changed bucket_ to a pointer, so to reduce the swapping time to one operation (which is indeed threads-safe), because swap is defined as a template: template <class _Tp> inline void swap(_Tp& __a, _Tp& __b) { _Tp __tmp = __a; __a = __b; __b = __tmp; } and as you see makes three full copies. 3) I've put a locking while accessing the bucket, and a check on emptyBucket before emptying it. This is because we don't want emptyBucket to empty the bucket while touch() is pushing something into it. Note that this locking is one side only, i.e. it doesn't introduce waiting time on touch() but only on the threaded part. 4) I'm mostly unsatisfied with the final complexity of the result. I think that the code is efficient (and I hope correct) but somewhat ugly. I plan to simplify it enormously when real threads become available. For instance, eliminate the bucket_, simplify the locking (using real mutexes). Depending on the performance, maybe even eliminate cache_set_. The real bottleneck with performance now comes from startup: when all previews become suddenly available, I think that GraphicsLoader::startLoading (and then touch()) gets called _too_many_ more times than necesary. I will try to look further. 5) The 'priority' of the queue has to be decided somehow. Right now it loads 10 images every .1 second, but these are arbitrary numbers. If this gets applied, GraphicsSupport.[Ch] will not be needed any more. Please comment (and if you want to make me really happy, try it), Ciao, Alfredo
/** * \file LoaderQueue.C * This file is part of LyX, the document processor. * Licence details can be found in the file COPYING. * * \author Alfredo Braunstein * * Full author contact details are available in file CREDITS */ #include "LoaderQueue.h" #include "debug.h" #include <boost/bind.hpp> namespace grfx { LoaderQueue & LoaderQueue::get() { static LoaderQueue singleton; return singleton; } void LoaderQueue::loadNext() { emptyBucket(); lyxerr[Debug::GRAPHICS] << "LoaderQueue: " << cache_queue_.size() << " items in the queue" << endl; int counter = 10; while ( cache_queue_.size() && counter-- ) { if(cache_queue_.front()->status() == WaitingToLoad) cache_queue_.front()->startLoading(); cache_set_.erase( cache_queue_.front() ); cache_queue_.pop_front(); } if ( cache_queue_.size() || bucket_locked_ || bucket_->size() ) { startLoader(); } else { stopLoader(); } } LoaderQueue::LoaderQueue() : timer( 100 , Timeout::ONETIME ), running_( false ), bucket_locked_( false ) { timer.timeout.connect( boost::bind( &LoaderQueue::loadNext, this)); bucket_ = new std::queue<Cache::ItemPtr>; } void LoaderQueue::emptyBucket() { std::queue<Cache::ItemPtr> *tmp = new std::queue<Cache::ItemPtr>; if (! bucket_locked_) { swap(tmp, bucket_); lyxerr[Debug::GRAPHICS] << "LoaderQueue: emptying bucket" << endl; while (! tmp->empty()) { addToQueue(tmp->front()); tmp->pop(); } } delete tmp; } void LoaderQueue::startLoader() { lyxerr[Debug::GRAPHICS] << "LoaderQueue: waking up" << endl; running_ = true ; timer.start(); } void LoaderQueue::stopLoader() { timer.stop(); running_ = false ; lyxerr[Debug::GRAPHICS] << "LoaderQueue: I'm going to sleep" << endl; } bool LoaderQueue::running() const { return running_ ; } void LoaderQueue::touch(Cache::ItemPtr const & item) { if ( ! running_ ) startLoader(); bucket_locked_ = true; bucket_->push(item); bucket_locked_ = false; } void LoaderQueue::addToQueue(Cache::ItemPtr const & item) { if ( !cache_set_.insert(item).second ) { list<Cache::ItemPtr>::iterator it = cache_queue_.begin(); list<Cache::ItemPtr>::iterator end = cache_queue_.end(); it = std::find( it, end, item ); if (it != end) cache_queue_.erase( it ); } cache_queue_.push_front( item ); } } // namespace grfx
// -*- C++ -*- /** * \file LoaderQueue.h * This file is part of LyX, the document processor. * Licence details can be found in the file COPYING. * * \author Alfredo Braunstein * * Full author contact details are available in file CREDITS. * * This implements a threaded service queue which loads images on background. * In order to request an image loading you call touch() with the pointer to * the cached image. Then it will try to satisfy the request as soon as * posible (that's it: after finishing an eventual loading on progress) * touch() returns inmediately, in order not tu disrupt the flow of the main * thread. * The service thread is the method loadNext(). * */ #ifndef LOADERQUEUE_H #define LOADERQUEUE_H #include "GraphicsCache.h" #include "GraphicsCacheItem.h" #include "frontends/Timeout.h" #include <set> #include <queue> #include <algorithm> namespace grfx { class LoaderQueue { public: //use this to request a loading void touch(Cache::ItemPtr const & item); //query if the clock is ticking bool running() const; //get the and only instance of the class static LoaderQueue & get(); private: //this class is a singleton class... use LoaderQueue::get() instead LoaderQueue(); //in-progress loading queue (elements are unique here) std::list<Cache::ItemPtr> cache_queue_; //makes faster the insertion of new elements std::set<Cache::ItemPtr> cache_set_; //newly touched element go here, loadNext move them to cache_queue_ std::queue<Cache::ItemPtr> *bucket_; // Timeout timer; // bool running_; //needed in order to avoid touching the bucket_ if it's in use. bool bucket_locked_; //moves bucket_ to cache_queue_ void emptyBucket(); //adds or reprioritizes one element in cache_queue_ void addToQueue(Cache::ItemPtr const & item); //this is the 'threaded' method, that does the loading in background void loadNext(); // void startLoader(); // void stopLoader(); }; } // namespace grfx #endif // LOADERQUEUE_H
Index: ChangeLog =================================================================== RCS file: /cvs/lyx/lyx-devel/src/graphics/ChangeLog,v retrieving revision 1.139 diff -u -r1.139 ChangeLog --- ChangeLog 2003/01/21 17:54:03 1.139 +++ ChangeLog 2003/02/21 00:44:27 @@ -1,3 +1,11 @@ +2003-02-20 Alfredo Braunstein <[EMAIL PROTECTED]> + + * LoaderQueue.[Ch]: added. Implements a service queue that loads + images in background + * GraphicsSupport.[Ch]: removed + * Makefile.am: the changes above + * GraphicsLoader.C: use the loading queue + 2003-01-21 Angus Leeming <[EMAIL PROTECTED]> * PreviewLoader.C (dumpPreamble): ensure that \lyxlock does not prevent Index: Makefile.am =================================================================== RCS file: /cvs/lyx/lyx-devel/src/graphics/Makefile.am,v retrieving revision 1.26 diff -u -r1.26 Makefile.am --- Makefile.am 2002/12/16 23:17:43 1.26 +++ Makefile.am 2003/02/21 00:44:27 @@ -17,8 +17,8 @@ GraphicsLoader.C \ GraphicsParams.C \ GraphicsParams.h \ - GraphicsSupport.h \ - GraphicsSupport.C \ + LoaderQueue.h \ + LoaderQueue.C \ GraphicsTypes.h \ GraphicsTypes.C \ PreviewImage.h \ Index: GraphicsLoader.C =================================================================== RCS file: /cvs/lyx/lyx-devel/src/graphics/GraphicsLoader.C,v retrieving revision 1.12 diff -u -r1.12 GraphicsLoader.C --- GraphicsLoader.C 2003/02/13 16:53:00 1.12 +++ GraphicsLoader.C 2003/02/21 00:44:27 @@ -18,7 +18,7 @@ #include "GraphicsCacheItem.h" #include "GraphicsImage.h" #include "GraphicsParams.h" -#include "GraphicsSupport.h" +#include "LoaderQueue.h" #include "frontends/LyXView.h" #include "frontends/Timeout.h" @@ -66,8 +66,6 @@ /// Params params_; - /// - Timeout timer; // Multiple Insets can share the same image typedef std::list<Inset const *> InsetList; /// @@ -196,10 +194,8 @@ Loader::Impl::Impl(Params const & params) - : status_(WaitingToLoad), params_(params), - timer(2000, Timeout::ONETIME) + : status_(WaitingToLoad), params_(params) { - timer.timeout.connect(boost::bind(&Impl::checkedLoading, this)); } @@ -293,7 +289,7 @@ void Loader::Impl::startLoading(Inset const & inset, BufferView const & bv) { - if (status_ != WaitingToLoad || timer.running()) + if (status_ != WaitingToLoad) return; InsetList::const_iterator it = insets.begin(); @@ -302,47 +298,8 @@ if (it == end) insets.push_back(&inset); view = bv.owner()->view(); - - timer.start(); -} - - -namespace { - -struct FindVisibleInset { - - FindVisibleInset(std::list<VisibleParagraph> const & vps) : vps_(vps) {} - - bool operator()(Inset const * inset_ptr) - { - if (!inset_ptr) - return false; - return isInsetVisible(*inset_ptr, vps_); - } - -private: - std::list<VisibleParagraph> const & vps_; -}; - -} // namespace anon - - -void Loader::Impl::checkedLoading() -{ - if (insets.empty() || !view.get()) - return; - - std::list<VisibleParagraph> const vps = - getVisibleParagraphs(*view.get()); - - InsetList::const_iterator it = insets.begin(); - InsetList::const_iterator end = insets.end(); - - it = std::find_if(it, end, FindVisibleInset(vps)); - // One of the insets is visible, so start loading the image. - if (it != end) - cached_item_->startLoading(); + LoaderQueue::get().touch(cached_item_); }