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