[poppler] RFC: upstream optional threading support in pdftoppm for simple testing

Albert Astals Cid aacid at kde.org
Sun Jun 2 14:30:09 PDT 2013 

El Diumenge, 2 de juny de 2013, a les 12:10:19, Adam Reichold va escriure:
> 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.

Pushed, gave the file a "better" (imho) name.

Cheers,
  Albert

> 
> 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
> >>>>>>>>>>>>> poppler at lists.freedesktop.org
> >>>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>>>>>>>> 
> >>>>>>>>>>> _______________________________________________
> >>>>>>>>>>> poppler mailing list
> >>>>>>>>>>> poppler at lists.freedesktop.org
> >>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>>>>>>> 
> >>>>>>>>>> _______________________________________________
> >>>>>>>>>> poppler mailing list
> >>>>>>>>>> poppler at lists.freedesktop.org
> >>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>>>>>> 
> >>>>>>>>> _______________________________________________
> >>>>>>>>> poppler mailing list
> >>>>>>>>> poppler at lists.freedesktop.org
> >>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>>>>> 
> >>>>>>>> _______________________________________________
> >>>>>>>> poppler mailing list
> >>>>>>>> poppler at lists.freedesktop.org
> >>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>>>> 
> >>>>>>> _______________________________________________
> >>>>>>> poppler mailing list
> >>>>>>> poppler at lists.freedesktop.org
> >>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>>>> 
> >>>>>>> .
> >>>>>> 
> >>>>>> _______________________________________________
> >>>>>> poppler mailing list
> >>>>>> poppler at lists.freedesktop.org
> >>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>> 
> >>>>> _______________________________________________
> >>>>> poppler mailing list
> >>>>> poppler at lists.freedesktop.org
> >>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>>>> 
> >>>>> .
> >>>> 
> >>>> _______________________________________________
> >>>> poppler mailing list
> >>>> poppler at lists.freedesktop.org
> >>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >>> 
> >>> _______________________________________________
> >>> poppler mailing list
> >>> poppler at lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >> 
> >> _______________________________________________
> >> poppler mailing list
> >> poppler at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/poppler

More information about the poppler mailing list