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_);
 }
 
 

Reply via email to