Il 07/11/2011 10:55, Lars Gullik Bjønnes ha scritto:
Tommaso Cucinotta<tomm...@lyx.org>  writes:

| +#include<QThread>
| +#include<QThreadPool>
| +#include<QMutex>
| +#include<QMutexLocker>
| +#include<QWaitCondition>

Oh. how I dislike the Qt-fication of lyx...

I know, but as Qt is a strong dependency, it is anyway ok to rely on its portable extra features, right ? For example, if we used directly pthreads, would it compile
on Windows ?
| +static pid_t gettid() {
| +     pid_t tid;
| +     tid = syscall(SYS_gettid);
| +     return tid;
| +}

Should be in placed in support somewhere.
Also you should not need it. Why not use the thread-ids from the
threading library
(right debugging code.)

it should be QThread::currentThreadId(), I'll remove the gettid() entirely.

| +
| +static QMutex dbg_mtx;
| +#define SyncErr(lev, expr) do {                                              
\
| +             if (lyx::lyxerr.debugging(lev)) {                       \
| +                     QMutexLocker lock(&dbg_mtx);                        \
| +                     LYXERR(lev, "thread="<<  gettid()<<  ": "<<  expr); \
| +             }                                                       \
| +     } while (0)
| +

For debugging ok... for a permanent solution make LYXERR do the locking.

it will likely "hurt" the performance of logging from the single-threaded parts of LyX (e.g., everywhere else, so far). I know, debugging slows down anyway, so, it should not be a big problem. Perhaps this is another part that can take advantage of a compile-time flag enabling/disabling parallelism (when disabled, compile away all the locking logics).

The use of this macro really muddles the whole patch.

I know, let me propose a more readable one.. please, find them attached. I separated the parallel debugging patch from the parallel findadv one.

However, now I broke compilation of lyxclient which complains with errors about not knowing anything about #include <QThread> and you can imagine what else...

Again, any further comment is very welcome.

    T.

Index: src/support/debug.h
===================================================================
--- src/support/debug.h	(revisione 40135)
+++ src/support/debug.h	(copia locale)
@@ -16,7 +16,13 @@
 #define LYXDEBUG_H
 
 #include "support/strfwd.h"
+#include <QThread>
+#include <QMutex>
+#include <QMutexLocker>
 
+// To be moved to automake/autoconf & the like
+#define LYX_PARALLEL
+
 namespace std {
 
 class ios_base;
@@ -202,6 +208,10 @@
 
 extern LyXErr lyxerr;
 
+#ifdef LYX_PARALLEL
+extern QMutex lyxerr_mtx;
+#endif
+
 } // namespace lyx
 
 #if USE_BOOST_CURRENT_FUNCTION
@@ -211,9 +221,30 @@
 # define CURRENT_POSITION __FILE__ << "(" << __LINE__ << "): "
 #endif
 
+#ifdef LYX_PARALLEL
+
 #define LYXERR(type, msg) \
 	do { \
 		if (!lyx::lyxerr.debugging(type)) {} \
+		else { \
+			QMutexLocker lock(&lyx::lyxerr_mtx);		\
+			lyx::lyxerr << "[thread " << QThread::currentThreadId() << "] "; \
+			lyx::lyxerr << CURRENT_POSITION << msg; lyx::lyxerr.endl(); \
+		} \
+	} while (0)
+
+#define LYXERR0(msg) \
+	do { \
+		QMutexLocker lock(&lyx::lyxerr_mtx); \
+		lyx::lyxerr << "[thread " << QThread::currentThreadId() << "] "; \
+		lyx::lyxerr << CURRENT_POSITION << msg; lyx::lyxerr.endl(); \
+	} while (0)
+
+#else
+
+#define LYXERR(type, msg) \
+	do { \
+		if (!lyx::lyxerr.debugging(type)) {} \
 		else { lyx::lyxerr << CURRENT_POSITION << msg; lyx::lyxerr.endl(); } \
 	} while (0)
 
@@ -222,5 +253,6 @@
 		lyx::lyxerr << CURRENT_POSITION << msg; lyx::lyxerr.endl(); \
 	} while (0)
 
+#endif
 
 #endif
Index: src/support/debug.cpp
===================================================================
--- src/support/debug.cpp	(revisione 40135)
+++ src/support/debug.cpp	(copia locale)
@@ -28,6 +28,10 @@
 
 namespace lyx {
 
+#ifdef LYX_PARALLEL
+QMutex lyxerr_mtx;
+#endif
+
 namespace {
 
 struct ErrorItem {
Index: configure.ac
===================================================================
--- configure.ac	(revisione 40135)
+++ configure.ac	(copia locale)
@@ -291,7 +291,6 @@
 #endif
 #define BOOST_ENABLE_ASSERT_HANDLER 1
 
-#define BOOST_DISABLE_THREADS 1
 #define BOOST_NO_WREGEX 1
 #define BOOST_NO_WSTRING 1
 
Index: src/lyxfind.cpp
===================================================================
--- src/lyxfind.cpp	(revisione 40135)
+++ src/lyxfind.cpp	(copia locale)
@@ -49,11 +49,18 @@
 #include "support/lassert.h"
 #include "support/lstrings.h"
 
-#include "support/regex.h"
-#include <boost/next_prior.hpp>
+#include "boost/regex.hpp"
+#include "boost/next_prior.hpp"
 
+#include <QThread>
+#include <QThreadPool>
+#include <QMutex>
+#include <QMutexLocker>
+#include <QWaitCondition>
+
 using namespace std;
 using namespace lyx::support;
+using namespace boost;
 
 namespace lyx {
 
@@ -489,59 +496,62 @@
 
 typedef vector<pair<string, string> > Escapes;
 
+struct StaticEscapes {
+	Escapes regexp_escapes;
+	Escapes lyx_unescapes;
+	Escapes regexp_latex_escapes;
+	StaticEscapes() {
+		regexp_escapes.push_back(pair<string, string>("$", "\\$"));
+		regexp_escapes.push_back(pair<string, string>("{", "\\{"));
+		regexp_escapes.push_back(pair<string, string>("}", "\\}"));
+		regexp_escapes.push_back(pair<string, string>("[", "\\["));
+		regexp_escapes.push_back(pair<string, string>("]", "\\]"));
+		regexp_escapes.push_back(pair<string, string>("(", "\\("));
+		regexp_escapes.push_back(pair<string, string>(")", "\\)"));
+		regexp_escapes.push_back(pair<string, string>("+", "\\+"));
+		regexp_escapes.push_back(pair<string, string>("*", "\\*"));
+		regexp_escapes.push_back(pair<string, string>(".", "\\."));
+		regexp_escapes.push_back(pair<string, string>("\\", "(?:\\\\|\\\\backslash)"));
+		regexp_escapes.push_back(pair<string, string>("~", "(?:\\\\textasciitilde|\\\\sim)"));
+		regexp_escapes.push_back(pair<string, string>("^", "(?:\\^|\\\\textasciicircum\\{\\}|\\\\mathcircumflex)"));
+
+		lyx_unescapes.push_back(pair<string, string>("\\%", "%"));
+		lyx_unescapes.push_back(pair<string, string>("\\mathcircumflex ", "^"));
+		lyx_unescapes.push_back(pair<string, string>("\\mathcircumflex", "^"));
+		lyx_unescapes.push_back(pair<string, string>("\\backslash ", "\\"));
+		lyx_unescapes.push_back(pair<string, string>("\\backslash", "\\"));
+		lyx_unescapes.push_back(pair<string, string>("\\\\{", "_x_<"));
+		lyx_unescapes.push_back(pair<string, string>("\\\\}", "_x_>"));
+		lyx_unescapes.push_back(pair<string, string>("\\sim ", "~"));
+		lyx_unescapes.push_back(pair<string, string>("\\sim", "~"));
+
+		regexp_latex_escapes.push_back(pair<string, string>("\\\\", "(?:\\\\\\\\|\\\\backslash|\\\\textbackslash\\{\\})"));
+		regexp_latex_escapes.push_back(pair<string, string>("(<?!\\\\\\\\textbackslash)\\{", "\\\\\\{"));
+		regexp_latex_escapes.push_back(pair<string, string>("(<?!\\\\\\\\textbackslash\\\\\\{)\\}", "\\\\\\}"));
+		regexp_latex_escapes.push_back(pair<string, string>("\\[", "\\{\\[\\}"));
+		regexp_latex_escapes.push_back(pair<string, string>("\\]", "\\{\\]\\}"));
+		regexp_latex_escapes.push_back(pair<string, string>("\\^", "(?:\\^|\\\\textasciicircum\\{\\}|\\\\mathcircumflex)"));
+		regexp_latex_escapes.push_back(pair<string, string>("%", "\\\\\\%"));
+	}
+};
+
+static StaticEscapes escapes;
+
 /// A map of symbols and their escaped equivalent needed within a regex.
 /// @note Beware of order
 Escapes const & get_regexp_escapes()
 {
-	static Escapes escape_map;
-	if (escape_map.empty()) {
-		escape_map.push_back(pair<string, string>("$", "\\$"));
-		escape_map.push_back(pair<string, string>("{", "\\{"));
-		escape_map.push_back(pair<string, string>("}", "\\}"));
-		escape_map.push_back(pair<string, string>("[", "\\["));
-		escape_map.push_back(pair<string, string>("]", "\\]"));
-		escape_map.push_back(pair<string, string>("(", "\\("));
-		escape_map.push_back(pair<string, string>(")", "\\)"));
-		escape_map.push_back(pair<string, string>("+", "\\+"));
-		escape_map.push_back(pair<string, string>("*", "\\*"));
-		escape_map.push_back(pair<string, string>(".", "\\."));
-		escape_map.push_back(pair<string, string>("\\", "(?:\\\\|\\\\backslash)"));
-		escape_map.push_back(pair<string, string>("~", "(?:\\\\textasciitilde|\\\\sim)"));
-		escape_map.push_back(pair<string, string>("^", "(?:\\^|\\\\textasciicircum\\{\\}|\\\\mathcircumflex)"));
-	}
-	return escape_map;
+	return escapes.regexp_escapes;
 }
 
 /// A map of lyx escaped strings and their unescaped equivalent.
 Escapes const & get_lyx_unescapes() {
-	static Escapes escape_map;
-	if (escape_map.empty()) {
-		escape_map.push_back(pair<string, string>("\\%", "%"));
-		escape_map.push_back(pair<string, string>("\\mathcircumflex ", "^"));
-		escape_map.push_back(pair<string, string>("\\mathcircumflex", "^"));
-		escape_map.push_back(pair<string, string>("\\backslash ", "\\"));
-		escape_map.push_back(pair<string, string>("\\backslash", "\\"));
-		escape_map.push_back(pair<string, string>("\\\\{", "_x_<"));
-		escape_map.push_back(pair<string, string>("\\\\}", "_x_>"));
-		escape_map.push_back(pair<string, string>("\\sim ", "~"));
-		escape_map.push_back(pair<string, string>("\\sim", "~"));
-	}
-	return escape_map;
+	return escapes.lyx_unescapes;
 }
 
 /// A map of escapes turning a regexp matching text to one matching latex.
 Escapes const & get_regexp_latex_escapes() {
-	static Escapes escape_map;
-	if (escape_map.empty()) {
-		escape_map.push_back(pair<string, string>("\\\\", "(?:\\\\\\\\|\\\\backslash|\\\\textbackslash\\{\\})"));
-		escape_map.push_back(pair<string, string>("(<?!\\\\\\\\textbackslash)\\{", "\\\\\\{"));
-		escape_map.push_back(pair<string, string>("(<?!\\\\\\\\textbackslash\\\\\\{)\\}", "\\\\\\}"));
-		escape_map.push_back(pair<string, string>("\\[", "\\{\\[\\}"));
-		escape_map.push_back(pair<string, string>("\\]", "\\{\\]\\}"));
-		escape_map.push_back(pair<string, string>("\\^", "(?:\\^|\\\\textasciicircum\\{\\}|\\\\mathcircumflex)"));
-		escape_map.push_back(pair<string, string>("%", "\\\\\\%"));
-	}
-	return escape_map;
+	return escapes.regexp_latex_escapes;
 }
 
 /** @todo Probably the maps need to be migrated to regexps, in order to distinguish if
@@ -616,10 +626,10 @@
 bool regex_replace(string const & s, string & t, string const & searchstr,
 	string const & replacestr)
 {
-	lyx::regex e(searchstr);
+	boost::regex e(searchstr);
 	ostringstream oss;
 	ostream_iterator<char, char> it(oss);
-	lyx::regex_replace(it, s.begin(), s.end(), e, replacestr);
+	boost::regex_replace(it, s.begin(), s.end(), e, replacestr);
 	// tolerate t and s be references to the same variable
 	bool rv = (s != oss.str());
 	t = oss.str();
@@ -714,9 +724,9 @@
 	// normalized string to search
 	string par_as_string;
 	// regular expression to use for searching
-	lyx::regex regexp;
+	boost::regex regexp;
 	// same as regexp, but prefixed with a ".*"
-	lyx::regex regexp2;
+	boost::regex regexp2;
 	// leading format material as string
 	string lead_as_string;
 	// par_as_string after removal of lead_as_string
@@ -865,12 +875,12 @@
 		// If entered regexp must match at begin of searched string buffer
 		string regexp_str = string("\\`") + lead_as_regexp + par_as_string;
 		LYXERR(Debug::FIND, "Setting regexp to : '" << regexp_str << "'");
-		regexp = lyx::regex(regexp_str);
+		regexp = boost::regex(regexp_str);
 
 		// If entered regexp may match wherever in searched string buffer
 		string regexp2_str = string("\\`.*") + lead_as_regexp + ".*" + par_as_string;
 		LYXERR(Debug::FIND, "Setting regexp2 to: '" << regexp2_str << "'");
-		regexp2 = lyx::regex(regexp2_str);
+		regexp2 = boost::regex(regexp2_str);
 	}
 }
 
@@ -1129,16 +1139,16 @@
 
 
 /// Finds forward
-int findForwardAdv(DocIterator & cur, MatchStringAdv & match)
+int findForwardAdv(DocIterator & cur, MatchStringAdv const & match, DocIterator const & cur_end)
 {
 	if (!cur)
 		return 0;
-	while (cur) {
+	while (cur && cur < cur_end) {
 		LYXERR(Debug::FIND, "findForwardAdv() cur: " << cur);
 		int match_len = match(cur, -1, false);
 		LYXERR(Debug::FIND, "match_len: " << match_len);
-		if (match_len) {
-			for (; cur; cur.forwardPos()) {
+		if (match_len && cur < cur_end) {
+			for (; cur && cur < cur_end; cur.forwardPos()) {
 				LYXERR(Debug::FIND, "Advancing cur: " << cur);
 				int match_len = match(cur);
 				LYXERR(Debug::FIND, "match_len: " << match_len);
@@ -1150,7 +1160,7 @@
 						return len;
 				}
 			}
-			if (!cur)
+			if (!cur || cur == cur_end)
 				return 0;
 		}
 		if (cur.pit() < cur.lastpit()) {
@@ -1394,6 +1404,151 @@
 }
 
 
+struct FindAdvHit {
+	DocIterator dit;
+	int match_len;	//< -1 means invalid FindAdvHit contents
+
+
+	FindAdvHit()
+	: match_len(-1) {  }
+
+
+	FindAdvHit(DocIterator const & dit, int match_len)
+	: dit(dit), match_len(match_len) {  }
+};
+
+/// Synchronized vector of hits.
+/// Allows worker threads to push their search results here,
+/// and the parent to wait for the first hit to become available.
+class FindAdvHits {
+	std::map<int, FindAdvHit> hits_;
+	mutable QMutex hits_mtx_;
+	QWaitCondition found_;
+	DocIterator cur_;
+	int first_id_;		///< Id of first par search result to be waited for
+	int last_id_;		///< Id of last par still being searched
+
+	bool exists_(int id) const {
+		std::map<int, FindAdvHit>::const_iterator it = hits_.find(id);
+		return it != hits_.end();
+	}
+
+public:
+	FindAdvHits() {
+		first_id_ = 0;
+		last_id_ = -1;
+	}
+
+
+	void clear(DocIterator const & cur) {
+		QMutexLocker lock(&hits_mtx_);
+		first_id_ = 0;
+		last_id_ = -1;
+		hits_.clear();
+		cur_ = cur;
+	}
+
+
+	int nextRange(DocIterator & cur_beg, DocIterator & cur_end) {
+		QMutexLocker lock(&hits_mtx_);
+		if (!cur_)
+			return -1;
+		cur_beg = cur_;
+		cur_.forwardPar();
+		if (cur_)
+			cur_.pos() = 0;
+		cur_end = cur_;
+		++last_id_;
+		LYXERR(Debug::FIND, "Returning range from cur_beg=" << cur_beg << " to cur_end=" << cur_end << ", last_id_=" << last_id_);
+		return last_id_;
+	}
+
+
+	void insert(int id, FindAdvHit const & hit) {
+		QMutexLocker lock(&hits_mtx_);
+		// @todo Insertion and immediate removal can be avoided, but...
+		hits_[id] = hit;
+		if (id == first_id_) {
+			while (exists_(first_id_) && hits_[first_id_].match_len == 0) {
+				LYXERR(Debug::FIND, "Erasing id=" << first_id_);
+				hits_.erase(first_id_);
+				++first_id_;
+				LYXERR(Debug::FIND, "first_id_=" << first_id_);
+			}
+			LYXERR(Debug::FIND, "first_id_=" << first_id_ << ", last_id_=" << last_id_);
+			/* if ((exists_(first_id_) && hits_[first_id_].match_len > 0) */
+			/*     || first_id_ > last_id_) */
+		}
+		LYXERR(Debug::FIND, "Waking up");
+		found_.wakeOne();
+	}
+
+
+	int waitForFirstHit(DocIterator & dit) {
+		QMutexLocker lock(&hits_mtx_);
+		LYXERR(Debug::FIND, "Searching for first_id_=" << first_id_ << ", last_id_=" << last_id_);
+		std::map<int, FindAdvHit>::const_iterator it = hits_.find(first_id_);
+		while (it == hits_.end() && (last_id_ == -1 || first_id_ <= last_id_)) {
+			LYXERR(Debug::FIND, "waiting for found_");
+			found_.wait(&hits_mtx_);
+			LYXERR(Debug::FIND, "Searching for first_id_=" << first_id_);
+			it = hits_.find(first_id_);
+			LYXERR(Debug::FIND, "Woken up: it==end is " << (it == hits_.end()));
+		}
+		if (it == hits_.end() || it->second.match_len == 0)
+			return 0;
+		FindAdvHit const & hit = it->second;
+		dit = hit.dit;
+		return hit.match_len;
+	}
+};
+
+
+static FindAdvHits hits;
+
+
+class FindAdvThread : public QRunnable {
+	MatchStringAdv const & matchAdv_;
+
+public:
+	FindAdvThread(MatchStringAdv const & matchAdv)
+		: matchAdv_(matchAdv) {  }
+
+	void run() {
+		do {
+			DocIterator dit;
+			DocIterator dit_end;
+			int id = hits.nextRange(dit, dit_end);
+			if (id < 0)
+				break;
+			LYXERR(Debug::FIND, "Calling findForwardAdv() from " << dit << " to " << dit_end << ", id_=" << id);
+			int match_len = findForwardAdv(dit, matchAdv_, dit_end);
+			LYXERR(Debug::FIND, "findForwardAdv() result (id=" << id << "): dit=" << dit << ", match_len=" << match_len);
+			hits.insert(id, FindAdvHit(dit, match_len));
+			/// @todo REVIEW EXIT CONDITION!!!
+			if (match_len > 0)
+				break;
+		} while (true);
+	}
+};
+
+
+int findForwardAdvPar(DocIterator & cur, MatchStringAdv const & match) {
+	QRunnable *p_thread;
+	hits.clear(cur);
+	for (int i = 0; i < QThread::idealThreadCount(); ++i) {
+		// Ownership taken by QThreadPool on start()
+		LYXERR(Debug::FIND, "Starting findadv thread");
+		p_thread = new FindAdvThread(match);
+		QThreadPool::globalInstance()->start(p_thread);
+	}
+	int match_len = hits.waitForFirstHit(cur);
+	LYXERR(Debug::FIND, "Waiting for children - match_len=" << match_len);
+	QThreadPool::globalInstance()->waitForDone();
+	return match_len;
+}
+
+
 /// Perform a FindAdv operation.
 bool findAdv(BufferView * bv, FindAndReplaceOptions const & opt)
 {
@@ -1404,12 +1559,12 @@
 		MatchStringAdv matchAdv(bv->buffer(), opt);
 		findAdvReplace(bv, opt, matchAdv);
 		cur = bv->cursor();
-		if (opt.forward)
-				match_len = findForwardAdv(cur, matchAdv);
-		else
-				match_len = findBackwardsAdv(cur, matchAdv);
+		if (opt.forward) {
+			match_len = findForwardAdvPar(cur, matchAdv);
+		} else
+			match_len = findBackwardsAdv(cur, matchAdv);
 	} catch (...) {
-		// This may only be raised by lyx::regex()
+		// This may only be raised by boost::regex()
 		bv->message(_("Invalid regular expression!"));
 		return false;
 	}

Reply via email to