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