Hello Albert,
The test case was part of bug #59927 "make qt4 frontend thread-safe"
[1]. I took that code and added a wee bit of configurability and a lock
that should serialize the parts depending on annotation life cycle.
Best regards, Adam.
P.S.: I could also rework this into a single source file if you prefer that.
[1] https://bugs.freedesktop.org/show_bug.cgi?id=59927
Am 01.06.2013 19:58, schrieb Albert Astals Cid:
> El Dimecres, 10 d'abril de 2013, a les 16:56:04, Adam Reichold va escriure:
>> Hell Albert,
>>
>> Am 09.04.2013 00:51, schrieb Albert Astals Cid:
>>> El Dilluns, 8 d'abril de 2013, a les 06:53:10, Thomas Freitag va escriure:
>>>> Am 08.04.2013 00:42, schrieb Albert Astals Cid:
>>>>> El Diumenge, 7 d'abril de 2013, a les 22:07:46, Thomas Freitag va
>>>
>>> escriure:
>>>>>> Am 07.04.2013 21:12, schrieb Albert Astals Cid:
>>>>>>> El Diumenge, 7 d'abril de 2013, a les 16:31:52, Adam Reichold va
>>>
>>> escriure:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> Am 07.04.2013 16:13, schrieb Albert Astals Cid:
>>>>>>>>> El Dissabte, 6 d'abril de 2013, a les 17:43:54, Adam Reichold va
>>>>>
>>>>> escriure:
>>>>>>>>>> Hello,
>>>>>>>>>>
>>>>>>>>>> Am 06.04.2013 17:14, schrieb Albert Astals Cid:
>>>>>>>>>>> El Divendres, 5 d'abril de 2013, a les 21:43:28, Adam Reichold va
>>>>>>>>>
>>>>>>>>> escriure:
>>>>>>>>>>>> Hello again,
>>>>>>>>>>>>
>>>>>>>>>>>> I was a bit in a rush at the first try. Sorry for that, I tidied
>>>>>>>>>>>> it
>>>>>>>>>>>> up
>>>>>>>>>>>> slightly.
>>>>>>>>>>>
>>>>>>>>>>> Maybe we should rename from UTILS_USE_THREAD to UTILS_USE_PTHREAD
>>>>>>>>>>> ?
>>>>>>>>>>>
>>>>>>>>>>> Or add a comment somewhere that we only support pthreads for now
>>>>>>>>>>> somewhere?
>>>>>>>>>>
>>>>>>>>>> I would be fine with both.
>>>>>>>>>>
>>>>>>>>>> Actually, since this is mostly meant for testing, I would be fine
>>>>>>>>>> with
>>>>>>>>>> not making it accessible via autotools or CMake at all, i.e. just
>>>>>>>>>> add
>>>>>>>>>> the definition to 'config.h' manually when and if we need it.
>>>>>>>>>
>>>>>>>>> Makes sense to me, code-wise what's the difference between this and
>>>>>>>>> the
>>>>>>>>> code Thomas posted in the threading bug? Do you think this is
>>>>>>>>> simpler/easier to understand?
>>>>>>>>
>>>>>>>> Yes, the difference is that I left out the Windows-specific part and
>>>>>>>> tried to keep it as simple as possible. For example, I think
>>>>>>>> synchronizing on the job queue is simpler than synchronizing on the
>>>>>>>> thread state. But of course, my implementation is not very efficient
>>>>>>>> in
>>>>>>>> terms of performance, just sufficient for testing.
>>>>>>>
>>>>>>> Thomas would you be OK if we merge this patchset or you'd prefer yours
>>>>>>> (more complex?) to be in?
>>>>>>
>>>>>> Oh, I never thought to get a question like this. To answer it, I need
>>>>>> to
>>>>>> to go a little bit more far afield: When I started to implement it,
>>>>>> first of all I need a test case. Therefore I made that hack to pdftoppm
>>>>>> to use multiple threads under Windows (still my favorite programming
>>>>>> platform, I'm too old to change my habits), the pthread version I made
>>>>>> much later to run it over the PDF suite and so that You can test it,
>>>>>> too. But it was never made to publish it.
>>>>>> I think that a multi
>>>>>> threading version of pdftoppm is not really necessary for the
>>>>>> community.
>>>>>
>>>>> If we replace community by final user, agreed.
>>>>>
>>>>>> BUT: because we now support multi threading, we need a test case in
>>>>>> general. I haven't review Adam's patch in detail, but if You think it
>>>>>> is
>>>>>> sufficient as test case (and I don't think, that anyone need a Windows
>>>>>> test case), I can live with it. If You otherwise mean that we need a
>>>>>> more general support also on Windows, I'll spend one or two weekends to
>>>>>> revise my solution.
>>>>>
>>>>> Honestly *I* don't need Windows support, but that's not *my* project but
>>>>> a
>>>>> community one and you seem to need it so we need it.
>>>>>
>>>>> What I didn't originally like about your patch is that it was a bit too
>>>>> much intrusive for my taste, and to be honest Adam's a bit too.
>>>>
>>>> The advantage of a "pdftoppm" test case is that we can use it in the
>>>> regression test without any manipulation and can compare the multi
>>>> threading results with the "normal" results. And with the actual state
>>>> we still have different results
>>>> 1. with damaged PDF where the "normal" pdftoppm produces output for
>>>> pages which can not be rendered by copying the last producable page.
>>>> 2. with the type 3 fonts where the caching produces "random" result.
>>>> 3. with non embedded fonts where the decision which external font should
>>>> be used is sometimes different.
>>>> But to solve this even I don't need a windows solution. I did need it
>>>> only in the past to set breakpoints to have an easier look at variables,
>>>> memory and call stack.
>>>>
>>>>> So that got me thinking into the fact that we don't really need much of
>>>>> the
>>>>> pdftoppm code, we just need to create a SplashOutputDev and feed it into
>>>>> a
>>>>> few threads.
>>>>>
>>>>> So what do you think of "separate" non pdftoppm code in the test/
>>>>> folder?
>>>>>
>>>>> This way we can even have a thread_pthread_test.cpp and a
>>>>> thread_windows_test.cpp, etc. without "polluting" the regular pdftoppm
>>>>> code.
>>>>
>>>> What do You think about Adam's qt testcase? It has the advantage of not
>>>> only test the rendering, even if it need some enhancements, because You
>>>> still cannot modify the same annotation from different threads at the
>>>> same time.
>>>
>>> I do like it, and being Qt has the benefit of being multiplatform already,
>>> but it has the problem of not being pdftoppm and thus not a drop-in
>>> replacement.
>>>
>>> Oh the decisions one has to take :D
>>>
>>> Ok, so what does this like?
>>>
>>> Take Adam's "qt test" and add it to qt/tests to have a "multiplatform
>>> stressing tool"
>>>
>>> Take Adam's "non introsuive pthread additions to pdftoppm" to have a "drop
>>> in replacement for the test suite"
>>>
>>> This way we get both sides happy?
>>>
>>> Yes?
>>
>> Sorry for being so late to answer, I was occupied at work. And yes, this
>> would make me happy. :-)
>
> Adam I've commited the pdftoppm-pthread code but i can't find the qt4 code,
> it
> was in some bug right? Do you remember where?
>
> Cheers,
> Albert
>
>>
>> Best regards, Adam.
>>
>>> Albert
>>>
>>>> Cheers,
>>>> Thomas
>>>>
>>>>> Thoughts?
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Albert
>>>>>>
>>>>>> So I band the ball back to You.
>>>>>> Cheers,
>>>>>> Thomas
>>>>>>
>>>>>>> Cheers,
>>>>>>>
>>>>>>> Albert
>>>>>>>>
>>>>>>>> Best regards, Adam.
>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>>
>>>>>>>>> Albert
>>>>>>>>>>
>>>>>>>>>> Best regards, Adam.
>>>>>>>>>>
>>>>>>>>>>> Besides that it looks ok-ish in a quick look.
>>>>>>>>>>>
>>>>>>>>>>> Anyone has a comment?
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>>
>>>>>>>>>>> Albert
>>>>>>>>>>>>
>>>>>>>>>>>> Best regards, Adam.
>>>>>>>>>>>>
>>>>>>>>>>>> Am 05.04.2013 19:27, schrieb Adam Reichold:
>>>>>>>>>>>>> Hello everyone,
>>>>>>>>>>>>>
>>>>>>>>>>>>> To make it easier for us to test changes w.r.t. to threading, I
>>>>>>>>>>>>> would
>>>>>>>>>>>>> propose to commit a simple implementation of threading in
>>>>>>>>>>>>> 'pdftoppm'
>>>>>>>>>>>>> to
>>>>>>>>>>>>> master.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The attached patch contains a very simple implementation that is
>>>>>>>>>>>>> not
>>>>>>>>>>>>> focused on maximal performance but should suffice to stress the
>>>>>>>>>>>>> locking
>>>>>>>>>>>>> inside Poppler's core. I opted to implement only the POSIX
>>>>>>>>>>>>> approach
>>>>>>>>>>>>> since I suppose POSIX systems are where most of us test and the
>>>>>>>>>>>>> code
>>>>>>>>>>>>> is
>>>>>>>>>>>>> hopefully simple and short enough not become a maintenance
>>>>>>>>>>>>> burden.
>>>>>>>>>>>>>
>>>>>>>>>>>>> What do you think?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Best regards, Adam.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>> poppler mailing list
>>>>>>>>>>>>> [email protected]
>>>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> poppler mailing list
>>>>>>>>>>> [email protected]
>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>>>>>>>
>>>>>>>>>> _______________________________________________
>>>>>>>>>> poppler mailing list
>>>>>>>>>> [email protected]
>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> poppler mailing list
>>>>>>>>> [email protected]
>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> poppler mailing list
>>>>>>>> [email protected]
>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> poppler mailing list
>>>>>>> [email protected]
>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>>>>
>>>>>>> .
>>>>>>
>>>>>> _______________________________________________
>>>>>> poppler mailing list
>>>>>> [email protected]
>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>>
>>>>> _______________________________________________
>>>>> poppler mailing list
>>>>> [email protected]
>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>>
>>>>> .
>>>>
>>>> _______________________________________________
>>>> poppler mailing list
>>>> [email protected]
>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>
>>> _______________________________________________
>>> poppler mailing list
>>> [email protected]
>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>
>> _______________________________________________
>> poppler mailing list
>> [email protected]
>> http://lists.freedesktop.org/mailman/listinfo/poppler
>
#include "sillythreads.h"
#include <unistd.h>
#include <QDateTime>
#include <QDebug>
#include <QImage>
#include <QMutex>
#include <QScopedPointer>
#include <poppler-qt4.h>
#include <poppler-form.h>
static Poppler::Page* loadPage(Poppler::Document* document, int index)
{
Poppler::Page* page = document->page(index);
if(page == 0)
{
qDebug() << "!Document::page";
exit(EXIT_FAILURE);
}
return page;
}
static Poppler::Page* loadRandomPage(Poppler::Document* document)
{
return loadPage(document, qrand() % document->numPages());
}
SillyThread::SillyThread(Poppler::Document* document, QObject* parent) : QThread(parent),
m_document(document),
m_pages()
{
m_pages.reserve(m_document->numPages());
for(int index = 0; index < m_document->numPages(); ++index)
{
m_pages.append(loadPage(m_document, index));
}
}
void SillyThread::run()
{
forever
{
foreach(Poppler::Page* page, m_pages)
{
QImage image = page->renderToImage();
if(image.isNull())
{
qDebug() << "!Page::renderToImage";
::exit(EXIT_FAILURE);
}
}
}
}
CrazyThread::CrazyThread(Poppler::Document* document, QMutex* annotationMutex, QObject* parent) : QThread(parent),
m_document(document),
m_annotationMutex(annotationMutex)
{
}
void CrazyThread::run()
{
typedef QScopedPointer< Poppler::Page > PagePointer;
qsrand(static_cast< uint >(currentThreadId()));
forever
{
if(qrand() % 2 == 0)
{
qDebug() << "search...";
PagePointer page(loadRandomPage(m_document));
page->search("c", Poppler::Page::CaseInsensitive);
page->search("r", Poppler::Page::CaseSensitive);
page->search("a", Poppler::Page::CaseInsensitive);
page->search("z", Poppler::Page::CaseSensitive);
page->search("y", Poppler::Page::CaseInsensitive);
}
if(qrand() % 2 == 0)
{
qDebug() << "links...";
PagePointer page(loadRandomPage(m_document));
QList< Poppler::Link* > links = page->links();
qDeleteAll(links);
}
if(qrand() % 2 == 0)
{
qDebug() << "form fields...";
PagePointer page(loadRandomPage(m_document));
QList< Poppler::FormField* > formFields = page->formFields();
qDeleteAll(formFields);
}
if(qrand() % 2 == 0)
{
qDebug() << "thumbnail...";
PagePointer page(loadRandomPage(m_document));
page->thumbnail();
}
if(qrand() % 2 == 0)
{
qDebug() << "text...";
PagePointer page(loadRandomPage(m_document));
page->text(QRectF(QPointF(), page->pageSizeF()));
}
if(qrand() % 2 == 0)
{
QMutexLocker mutexLocker(m_annotationMutex);
qDebug() << "add annotation...";
PagePointer page(loadRandomPage(m_document));
Poppler::Annotation* annotation = 0;
switch(qrand() % 3)
{
default:
case 0:
annotation = new Poppler::TextAnnotation(qrand() % 2 == 0 ? Poppler::TextAnnotation::Linked : Poppler::TextAnnotation::InPlace);
break;
case 1:
annotation = new Poppler::HighlightAnnotation();
break;
case 2:
annotation = new Poppler::InkAnnotation();
break;
}
annotation->setBoundary(QRectF(0.0, 0.0, 0.5, 0.5));
annotation->setContents("crazy");
page->addAnnotation(annotation);
delete annotation;
}
if(qrand() % 2 == 0)
{
QMutexLocker mutexLocker(m_annotationMutex);
for(int index = 0; index < m_document->numPages(); ++index)
{
PagePointer page(loadPage(m_document, index));
QList< Poppler::Annotation* > annotations = page->annotations();
if(!annotations.isEmpty())
{
qDebug() << "modify annotation...";
annotations.at(qrand() % annotations.size())->setBoundary(QRectF(0.5, 0.5, 0.25, 0.25));
annotations.at(qrand() % annotations.size())->setAuthor("foo");
annotations.at(qrand() % annotations.size())->setContents("bar");
annotations.at(qrand() % annotations.size())->setCreationDate(QDateTime::currentDateTime());
annotations.at(qrand() % annotations.size())->setModificationDate(QDateTime::currentDateTime());
}
qDeleteAll(annotations);
if(!annotations.isEmpty())
{
break;
}
}
}
if(qrand() % 2 == 0)
{
QMutexLocker mutexLocker(m_annotationMutex);
for(int index = 0; index < m_document->numPages(); ++index)
{
PagePointer page(loadPage(m_document, index));
QList< Poppler::Annotation* > annotations = page->annotations();
if(!annotations.isEmpty())
{
qDebug() << "remove annotation...";
page->removeAnnotation(annotations.takeAt(qrand() % annotations.size()));
}
qDeleteAll(annotations);
if(!annotations.isEmpty())
{
break;
}
}
}
if(qrand() % 2 == 0)
{
qDebug() << "fonts...";
m_document->fonts();
}
}
}
int main(int argc, char** argv)
{
if(argc < 3)
{
qDebug() << "usage: sillythreads duration sillyCount crazyCount file(s)";
return EXIT_FAILURE;
}
int duration = atoi(argv[1]);
int sillyCount = atoi(argv[2]);
int crazyCount = atoi(argv[3]);
for(int argi = 4; argi < argc; ++argi)
{
Poppler::Document* document = Poppler::Document::load(QString::fromLocal8Bit(argv[argi]));
if(document == 0)
{
qDebug() << "!Document::load";
continue;
}
for(int i = 0; i < sillyCount; ++i) {
(new SillyThread(document))->start();
}
QMutex* annotationMutex = new QMutex();
for(int i = 0; i < crazyCount; ++i)
{
(new CrazyThread(document, annotationMutex))->start();
}
}
sleep(duration);
return EXIT_SUCCESS;
}
#ifndef SILLYTHREADS_H
#define SILLYTHREADS_H
#include <QThread>
#include <QVector>
class QMutex;
namespace Poppler
{
class Annotation;
class Document;
class Page;
}
class SillyThread : public QThread
{
Q_OBJECT
public:
SillyThread(Poppler::Document* document, QObject* parent = 0);
void run();
private:
Poppler::Document* m_document;
QVector< Poppler::Page* > m_pages;
};
class CrazyThread : public QThread
{
Q_OBJECT
public:
CrazyThread(Poppler::Document* document, QMutex* annotationMutex, QObject* parent = 0);
void run();
private:
Poppler::Document* m_document;
QMutex* m_annotationMutex;
};
#endif // SILLYTHREADS_H
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
