threading cleanup: add mutex protection around moronic change to transcode. Add mutex to equiv issue in unac. Rename const strings everywhere to cstr_xx to ease future detection of potentially problematic static variables. Most probably close issue #65
This commit is contained in:
Jean-Francois Dockes
@@ -0,0 +1,106 @@
|
||||
/* Copyright (C) 2004 J.F.Dockes
|
||||
* 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 2 of the License, or
|
||||
* (at your option) 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.,
|
||||
* 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
|
||||
*/
|
||||
|
||||
//
|
||||
// Small test program to evaluate the cost of using mutex locks: calls
|
||||
// to methods doing a small (150 bytes) base64 encoding job + string
|
||||
// manips, with and without locking. The performance cost is
|
||||
// negligible on all machines I tested (around 0.3% to 2% depending on
|
||||
// the system and machine), but not inexistent, you would not want
|
||||
// this in a tight loop.
|
||||
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <unistd.h>
|
||||
|
||||
#include <string>
|
||||
using namespace std;
|
||||
|
||||
#include "ptmutex.h"
|
||||
#include "base64.h"
|
||||
|
||||
static char *thisprog;
|
||||
static char usage [] =
|
||||
"ptmutex [-l] count\n"
|
||||
"\n"
|
||||
;
|
||||
static void
|
||||
Usage(void)
|
||||
{
|
||||
fprintf(stderr, "%s: usage:\n%s", thisprog, usage);
|
||||
exit(1);
|
||||
}
|
||||
|
||||
static int op_flags;
|
||||
#define OPT_MOINS 0x1
|
||||
#define OPT_l 0x2
|
||||
|
||||
static const string convertbuffer =
|
||||
"* The recoll GUI program sometimes crashes when running a query while\
|
||||
the indexing thread is active. Possible workarounds:";
|
||||
|
||||
static PTMutexInit o_lock;
|
||||
void workerlock(string& out)
|
||||
{
|
||||
PTMutexLocker locker(o_lock);
|
||||
base64_encode(convertbuffer, out);
|
||||
}
|
||||
|
||||
void workernolock(string& out)
|
||||
{
|
||||
base64_encode(convertbuffer, out);
|
||||
}
|
||||
|
||||
int main(int argc, char **argv)
|
||||
{
|
||||
int count = 0;
|
||||
thisprog = argv[0];
|
||||
argc--; argv++;
|
||||
|
||||
while (argc > 0 && **argv == '-') {
|
||||
(*argv)++;
|
||||
if (!(**argv))
|
||||
/* Cas du "adb - core" */
|
||||
Usage();
|
||||
while (**argv)
|
||||
switch (*(*argv)++) {
|
||||
case 'l': op_flags |= OPT_l; break;
|
||||
default: Usage(); break;
|
||||
}
|
||||
b1: argc--; argv++;
|
||||
}
|
||||
|
||||
if (argc != 1)
|
||||
Usage();
|
||||
count = atoi(*argv++);argc--;
|
||||
|
||||
if (op_flags & OPT_l) {
|
||||
fprintf(stderr, "Looping %d, locking\n", count);
|
||||
for (int i = 0; i < count; i++) {
|
||||
string s;
|
||||
workerlock(s);
|
||||
}
|
||||
} else {
|
||||
fprintf(stderr, "Looping %d, no locking\n", count);
|
||||
for (int i = 0; i < count; i++) {
|
||||
string s;
|
||||
workernolock(s);
|
||||
}
|
||||
}
|
||||
exit(0);
|
||||
}
|
||||
|
||||
@@ -405,7 +405,7 @@ list(int fd, const string& path, vector<string>* names, flags flags, nspace dom)
|
||||
return true;
|
||||
}
|
||||
|
||||
static const string nullstring("");
|
||||
static const string cstr_nullstring("");
|
||||
|
||||
bool get(const string& path, const string& _name, string *value,
|
||||
flags flags, nspace dom)
|
||||
@@ -414,7 +414,7 @@ bool get(const string& path, const string& _name, string *value,
|
||||
}
|
||||
bool get(int fd, const string& _name, string *value, flags flags, nspace dom)
|
||||
{
|
||||
return get(fd, nullstring, _name, value, flags, dom);
|
||||
return get(fd, cstr_nullstring, _name, value, flags, dom);
|
||||
}
|
||||
bool set(const string& path, const string& _name, const string& value,
|
||||
flags flags, nspace dom)
|
||||
@@ -424,7 +424,7 @@ bool set(const string& path, const string& _name, const string& value,
|
||||
bool set(int fd, const string& _name, const string& value,
|
||||
flags flags, nspace dom)
|
||||
{
|
||||
return set(fd, nullstring, _name, value, flags, dom);
|
||||
return set(fd, cstr_nullstring, _name, value, flags, dom);
|
||||
}
|
||||
bool del(const string& path, const string& _name, flags flags, nspace dom)
|
||||
{
|
||||
@@ -432,7 +432,7 @@ bool del(const string& path, const string& _name, flags flags, nspace dom)
|
||||
}
|
||||
bool del(int fd, const string& _name, flags flags, nspace dom)
|
||||
{
|
||||
return del(fd, nullstring, _name, flags, dom);
|
||||
return del(fd, cstr_nullstring, _name, flags, dom);
|
||||
}
|
||||
bool list(const string& path, vector<string>* names, flags flags, nspace dom)
|
||||
{
|
||||
@@ -440,17 +440,17 @@ bool list(const string& path, vector<string>* names, flags flags, nspace dom)
|
||||
}
|
||||
bool list(int fd, vector<string>* names, flags flags, nspace dom)
|
||||
{
|
||||
return list(fd, nullstring, names, flags, dom);
|
||||
return list(fd, cstr_nullstring, names, flags, dom);
|
||||
}
|
||||
|
||||
static const string userstring("user.");
|
||||
static const string cstr_userstring("user.");
|
||||
bool sysname(nspace dom, const string& pname, string* sname)
|
||||
{
|
||||
if (dom != PXATTR_USER) {
|
||||
errno = EINVAL;
|
||||
return false;
|
||||
}
|
||||
*sname = userstring + pname;
|
||||
*sname = cstr_userstring + pname;
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -460,7 +460,7 @@ bool pxname(nspace dom, const string& sname, string* pname)
|
||||
errno = EINVAL;
|
||||
return false;
|
||||
}
|
||||
*pname = sname.substr(userstring.length());
|
||||
*pname = sname.substr(cstr_userstring.length());
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
@@ -428,7 +428,7 @@ void neutchars(const string &str, string &out, const string& chars)
|
||||
* if reasonably possible. Note: we could also use textsplit, stopping when
|
||||
* we have enough, this would be cleanly utf8-aware but would remove
|
||||
* punctuation */
|
||||
static const string SEPAR = " \t\n\r-:.;,/[]{}";
|
||||
static const string cstr_SEPAR = " \t\n\r-:.;,/[]{}";
|
||||
string truncate_to_word(const string &input, string::size_type maxlen)
|
||||
{
|
||||
string output;
|
||||
@@ -436,7 +436,7 @@ string truncate_to_word(const string &input, string::size_type maxlen)
|
||||
output = input;
|
||||
} else {
|
||||
output = input.substr(0, maxlen);
|
||||
string::size_type space = output.find_last_of(SEPAR);
|
||||
string::size_type space = output.find_last_of(cstr_SEPAR);
|
||||
// Original version only truncated at space if space was found after
|
||||
// maxlen/2. But we HAVE to truncate at space, else we'd need to do
|
||||
// utf8 stuff to avoid truncating at multibyte char. In any case,
|
||||
@@ -676,6 +676,9 @@ static void gettime(int, struct m_timespec *ts)
|
||||
}
|
||||
///// End system interface
|
||||
|
||||
// Note: this not protected against multithread access and not reentrant, but
|
||||
// this is mostly debug code, and it won't crash, just show bad results. Also
|
||||
// the frozen thing is not used that much
|
||||
static m_timespec frozen_tv;
|
||||
void Chrono::refnow()
|
||||
{
|
||||
|
||||
+15
-6
@@ -29,15 +29,21 @@ using std::string;
|
||||
|
||||
#include "transcode.h"
|
||||
#include "debuglog.h"
|
||||
|
||||
#include "ptmutex.h"
|
||||
#ifdef RCL_ICONV_INBUF_CONST
|
||||
#define ICV_P2_TYPE const char**
|
||||
#else
|
||||
#define ICV_P2_TYPE char**
|
||||
#endif
|
||||
|
||||
// We gain approximately 28% exec time for word at a time conversions by
|
||||
// We gain approximately 25% exec time for word at a time conversions by
|
||||
// caching the iconv_open thing.
|
||||
//
|
||||
// We may also lose some concurrency on multiproc because of the
|
||||
// necessary locking, but we only have one processing-intensive
|
||||
// possible thread for now (the indexing one), so this is probably not
|
||||
// an issue (and could be worked around with a slightly more
|
||||
// sohisticated approach).
|
||||
#define ICONV_CACHE_OPEN
|
||||
|
||||
bool transcode(const string &in, string &out, const string &icode,
|
||||
@@ -48,6 +54,8 @@ bool transcode(const string &in, string &out, const string &icode,
|
||||
static iconv_t ic = (iconv_t)-1;
|
||||
static string cachedicode;
|
||||
static string cachedocode;
|
||||
static PTMutexInit o_cachediconv_mutex;
|
||||
PTMutexLocker locker(o_cachediconv_mutex);
|
||||
#else
|
||||
iconv_t ic;
|
||||
#endif
|
||||
@@ -163,13 +171,14 @@ using namespace std;
|
||||
|
||||
// Repeatedly transcode a small string for timing measurements
|
||||
static const string testword("\xc3\xa9\x6c\x69\x6d\x69\x6e\xc3\xa9\xc3\xa0");
|
||||
// Without cache 10e6 reps on macpro -> 1.88 S
|
||||
// With cache -> 1.56
|
||||
// Without cache 10e6 reps on y -> 6.68
|
||||
// With cache -> 4.73
|
||||
// With cache and lock -> 4.9
|
||||
void looptest()
|
||||
{
|
||||
cout << testword << endl;
|
||||
string out;
|
||||
for (int i = 0; i < 1000*1000; i++) {
|
||||
for (int i = 0; i < 10*1000*1000; i++) {
|
||||
if (!transcode(testword, out, "UTF-8", "UTF-16BE")) {
|
||||
cerr << "Transcode failed" << endl;
|
||||
break;
|
||||
@@ -184,7 +193,7 @@ int main(int argc, char **argv)
|
||||
exit(0);
|
||||
#endif
|
||||
if (argc != 5) {
|
||||
cerr << "Usage: trcsguess ifilename icode ofilename ocode" << endl;
|
||||
cerr << "Usage: transcode ifilename icode ofilename ocode" << endl;
|
||||
exit(1);
|
||||
}
|
||||
const string ifilename = argv[1];
|
||||
|
||||
Reference in New Issue
Block a user