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. 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. 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
