[Poppler-bugs] [Bug 50992] library should be thread-safe

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=50992

--- Comment #24 from Albert Astals Cid <tsdgeos at terra.es> 2012-08-05 15:21:43 UTC ---
(In reply to comment #23)
> (In reply to comment #22)
> > Some random quick comments from looking at the code:
> >  * Seems "most" of the "let make it thread safe" is adding the 
> >       xref = (fCopy) ? doc->getXRef()->copy() : doc->getXRef();
> >     stuff, have you measured how slow is copying the xref? Because if it's not
> > "expensive" we may just do it all the time
> 
> I made just one test: I rendered "cud10nebsiaol.pdf" (from bug 52488) with and
> without the patch, it took 1 second more with the patch (without threads,
> nearly half the timee with threads). But that could propably be a measuring
> error.

Ouch, that seems quite a lot, i'll do a test if i have time

> > 
> >  * To make the "thread-safety" better we should remove PDFDoc::getXRef() and
> > force everyone to use Gfx:::getXRef() or ask for an XRef an input, right?
> 
> Yes, but without knowing anything about the consequences

Sure, but having half PDFDoc::getXRef and half Gfx::getXRef is just asking for
trouble in my opinion

> 
> > 
> >  * Should we remove the xref member in Page and use gfx->getXRef() everywhere?
> 
> Same answer
> 
> > 
> >  * The mutex locking seems to be very "rendering" oriented, e.g. if you are
> > locking  XRef::copy, why not lock XRef::add or XRef::setModifiedObject ?
> 
> It is rendering oriented. Until yesterday it even wasn't used, but I got some
> problems in my last tests, probably later solved later in
> FileStream::makeSubStream later, but I have forgot to remove it again and
> therefore also didn't test that.

Well, i understand that having rendering work is great, but if we are going to
make it thread-safe it should be really thread-safe everywhere, i.e. can i ask
for the fonts of the document while it's rendering? That's the typical use case
were okular now blocks

> > 
> >  * Why do you need UniqueFileStream ?
> 
> Probably could done in FileStream itself. But the difference is that
> UniqueFileStream closes it filehandle in the destructor, FileStream doesn't.

Sure, i see the difference in the code, but why is that behaviour difference
needed?

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.