[poppler] RFC: upstream optional threading support in pdftoppm for simple testing
Albert Astals Cid
aacid at kde.org
Sat Jun 1 10:58:47 PDT 2013
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