Adam Reichold <[email protected]> writes: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Hello Carlos, > > I think the problem also happens with a tty but it is not immediately > obvious: The output will look fine, but e.g. the "FAIL" appended to > the current line might belong to a different test case. Consider the > following chain of events (serialized by the Printer lock) using two > worker threads: > > thread A: self.printer.print_test_start("Test A: ") > thread B: self.printer.print_test_start("Test B: ") > thread A: self.printer.print_test_result_ln("FAIL") > > which as I understand it should result in > > "Test B: FAIL" > > whereas it was test A that actually failed.
Ok, I've pushed a different version. The number of the test is not that important in the end, so I think we could assign the numbers when printing the results to ensure we keep the order. Another issue I've noticed with the threads is that poppler-regtest can't be kille with CTRL + C anymore. > Best regards, Adam. Thanks, > Am 29.12.2012 17:13, schrieb Carlos Garcia Campos: >> Adam Reichold <[email protected]> writes: >> >> Hello Thomas, >> >> It is not a very sophisticated solution, but I think the attached >> patch should ensure useful messages for interleaved output of >> several worker threads. >> >>> I hadn't noticed this problem, and I can only reproduce it when >>> running the tests in verbose mode, does it only happen when >>> stdout is not a tty? >> >> Best regards, Adam. >> >> Am 29.12.2012 10:32, schrieb Thomas Freitag: >>>>> Hi Adam! >>>>> >>>>> I used yesterday the first time Your patch for regtesting: >>>>> Awfully, congratulations. You probably already encountered my >>>>> comment on bug 50992. One minor thing: Unfortunately the >>>>> protocol is not really readable anymore, because each thread >>>>> i.e. first outputs >>>>> >>>>> Testing '/media/thomas/HD-PCTU3/PDF Suite/01_ump_a_2009.pdf' >>>>> using splash backend (9/1137): >>>>> >>>>> and when it finishes i.e. >>>>> >>>>> PASS >>>>> >>>>> Because of the worker threads it's now nearly impossible to >>>>> match the test itself with the result of the test. Can You >>>>> change it in that way that this is outputted together? Should >>>>> be quite easy.... >>>>> >>>>> Cheers, Thomas >>>>> >>>>> Am 06.12.2012 19:02, schrieb Adam Reichold: Hello again, >>>>> >>>>> I reformatted the patch hopefully solving the whitespace >>>>> issues. (It does apply and run for me, but there are still >>>>> some whitespace warnings.) >>>>> >>>>> The use of recursive locks is probably not necessary in >>>>> "TestRun.py" and "TestReferences.py", but since the >>>>> performance impact is probably negligible in this case, I >>>>> thought allowing for recursion would help to keep the code >>>>> more accessible. In "Printer.py" there is at least one >>>>> recursion from "printout_update" into "printout". >>>>> >>>>> Best regards, Adam. >>>>> >>>>> Am 06.12.2012 18:19, schrieb Adam Reichold: >>>>>>>> Hello Carlos, >>>>>>>> >>>>>>>> Am 06.12.2012 18:11, schrieb Carlos Garcia Campos: >>>>>>>>> Adam Reichold <[email protected]> writes: >>>>>>>>>> Hello again, >>>>>>>>>> >>>>>>>>>> As suggested by William Bader, I added the option >>>>>>>>>> to use the logical CPU count as the number of >>>>>>>>>> worker threads automatically. >>>>>>>>> Thanks for the patch, any improvement in the >>>>>>>>> regression test framework is more than welcome, even >>>>>>>>> more if it's to improve the performance. What poppler >>>>>>>>> have you sued to create the patches? They don't apply >>>>>>>>> for me here using poppler from current git master. >>>>>>>> Yes, it is about performance and I am hesitant about >>>>>>>> introducing such complexity into the test driver which >>>>>>>> is supposed to test the correctness of program itself, >>>>>>>> but faster regression testing probably means more >>>>>>>> regression testing... :-) >>>>>>>> >>>>>>>> I am not sure how to correctly format the patch. I >>>>>>>> built on the current master but it seems Git thinks >>>>>>>> that the lines where I just changed the indentation are >>>>>>>> whitespace errors. I am not sure how to correct this as >>>>>>>> changes in whitespace are obviously important for >>>>>>>> Python scripts... I'll try to research this, but any >>>>>>>> advice would be welcome. >>>>>>>> >>>>>>>> Best regards, Adam. >>>>>>>> >>>>>>>>>> Regards, Adam. >>>>>>>>>> >>>>>>>>>> Am 04.12.2012 12:28, schrieb Adam Reichold: >>>>>>>>>>> Hello Thomas, >>>>>>>>>>> >>>>>>>>>>> Am 04.12.2012 08:41, schrieb Thomas Freitag: >>>>>>>>>>>> Am 04.12.2012 07:45, schrieb Adam Reichold: >>>>>>>>>>>> Hello everyone, >>>>>>>>>>>> >>>>>>>>>>>> I currently try to get myself acquainted with >>>>>>>>>>>> Poppler's regression testing framework. Because >>>>>>>>>>>> my system has a rather low single-threaded >>>>>>>>>>>> performance, I tried to implement parallel >>>>>>>>>>>> testing using Python's Queue class. >>>>>>>>>>>> >>>>>>>>>>>> Even though poppler-regtest currently uses two >>>>>>>>>>>> processes per test file, rendering even and >>>>>>>>>>>> odd pages respectively, the test files >>>>>>>>>>>> themselves are still handled sequentially and >>>>>>>>>>>> both process are joined for each test file. >>>>>>>>>>>> This will yield suboptimal system utilization >>>>>>>>>>>> even for a small three-core system like mine. >>>>>>>>>>>> >>>>>>>>>>>> Using the "-t/--threads N" option in the >>>>>>>>>>>> patched poppler-regtest will spawn N worker >>>>>>>>>>>> threads that handle all tests they can get from >>>>>>>>>>>> a single queue for all known tests, allowing to >>>>>>>>>>>> heavily utilize also large system if using a >>>>>>>>>>>> large set of test cases. But even for my >>>>>>>>>>>> three-core system, this brought down the time >>>>>>>>>>>> to create references for the complete test >>>>>>>>>>>> suite using the Splash backend from 4,5 hours >>>>>>>>>>>> to 2,75 hours. >>>>>>>>>>>>> What do You mean with "patched >>>>>>>>>>>>> poppler-regtest"? There was no attachment, or >>>>>>>>>>>>> do I miss something? >>>>>>>>>>> No, this on the process level in the test driver >>>>>>>>>>> and independent of the your multi-threading work. >>>>>>>>>>> There was a patch called "parallel_testing.patch" >>>>>>>>>>> attached, as least my mail client tells me so. >>>>>>>>>>> >>>>>>>>>>> I'll just try again... >>>>>>>>>>> >>>>>>>>>>> Best regards, Adam. >>>>>>>>>>> >>>>>>>>>>>>> If You talk about my multi-threaded testcase, >>>>>>>>>>>>> please be aware, that it is still >>>>>>>>>>>>> experimental, even if quite close. Here the >>>>>>>>>>>>> result of my last regression test last sunday >>>>>>>>>>>>> evening: Total 1133 tests 1114 tests passed >>>>>>>>>>>>> (98.32%) 17 tests failed (1.50%): >>>>>>>>>>>>> /media/thomas/HD-PCTU3/PDF Suite/Algorithmics >>>>>>>>>>>>> - The Spirit of Computing, 3rd Ed.pdf >>>>>>>>>>>>> (splash), /media/thomas/HD-PCTU3/PDF >>>>>>>>>>>>> Suite/Essentials of English Grammar - >>>>>>>>>>>>> www.ielts4u.blogfa.com.pdf ... 2 tests >>>>>>>>>>>>> crashed (0.18%): /media/thomas/HD-PCTU3/PDF >>>>>>>>>>>>> Suite/bug157090.pdf (splash), >>>>>>>>>>>>> /media/thomas/HD-PCTU3/PDF >>>>>>>>>>>>> Suite/sinatr4c.f5.pdf (splash) I'll first >>>>>>>>>>>>> have a look at the crashes next weekend, then >>>>>>>>>>>>> I'll continue with looking at the failed >>>>>>>>>>>>> tests.... Cheers, Thomas >>>>>>>>>>>> >>>>>>>>>>>> IMHO, the necessary changes seem quite small >>>>>>>>>>>> especially since a lot of them are connected to >>>>>>>>>>>> indentation handling. What are your thoughts >>>>>>>>>>>> on the utility and implementation of this? >>>>>>>>>>>> >>>>>>>>>>>> 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 >>>>> >>> From 5e7461569e3085370326203539fa02de968eb2ca Mon Sep 17 00:00:00 >>> 2001 From: Adam Reichold <[email protected]> Date: Sat, 29 >>> Dec 2012 15:17:33 +0100 Subject: [PATCH] print messages that are >>> useful with interleaved output by several worker threads >>> >>> --- regtest/TestRun.py | 18 +++++++++--------- 1 file changed, 9 >>> insertions(+), 9 deletions(-) >>> >>> diff --git a/regtest/TestRun.py b/regtest/TestRun.py index >>> 5564757..750b93c 100644 --- a/regtest/TestRun.py +++ >>> b/regtest/TestRun.py @@ -71,7 +71,7 @@ class TestRun: with >>> self._lock: self._n_tests += 1 >>> >>> - self.printer.print_test_start("Testing '%s' using %s >>> backend (%d/%d): " % (doc_path, backend.get_name(), n_doc, >>> total_docs)) + test_msg = "Testing '%s' using %s backend >>> (%d/%d): " % (doc_path, backend.get_name(), n_doc, total_docs) >>> test_has_md5 = backend.create_refs(doc_path, test_path) >>> >>> if backend.has_stderr(test_path): @@ -81,24 +81,24 @@ class >>> TestRun: if ref_has_md5 and test_has_md5: if >>> backend.compare_checksums(refs_path, test_path, not >>> self.config.keep_results, self.config.create_diffs, >>> self.config.update_refs): # FIXME: remove dir if it's empty? - >>> self.printer.print_test_result("PASS") + >>> self.printer.printout(test_msg + "PASS") with self._lock: >>> self._n_passed += 1 else: - >>> self.printer.print_test_result_ln("FAIL") + >>> self.printer.printout_ln(test_msg + "FAIL") with self._lock: >>> self._failed.append("%s (%s)" % (doc_path, backend.get_name())) >>> return elif test_has_md5: if ref_is_crashed: - >>> self.printer.print_test_result_ln("DOES NOT CRASH") + >>> self.printer.printout_ln(test_msg + "DOES NOT CRASH") elif >>> ref_is_failed: - >>> self.printer.print_test_result_ln("DOES NOT FAIL") + >>> self.printer.printout_ln(test_msg + "DOES NOT FAIL") return >>> >>> test_is_crashed = backend.is_crashed(test_path) if ref_is_crashed >>> and test_is_crashed: - >>> self.printer.print_test_result("PASS (Expected crash)") + >>> self.printer.printout_ln(test_msg + "PASS (Expected crash)") with >>> self._lock: self._n_passed += 1 return @@ -106,19 +106,19 @@ >>> class TestRun: test_is_failed = backend.is_failed(test_path) if >>> ref_is_failed and test_is_failed: # FIXME: compare status errors >>> - self.printer.print_test_result("PASS (Expected fail >>> with status error %d)" % (test_is_failed)) + >>> self.printer.printout(test_msg + "PASS (Expected fail with status >>> error %d)" % (test_is_failed)) with self._lock: self._n_passed += >>> 1 return >>> >>> if test_is_crashed: - >>> self.printer.print_test_result_ln("CRASH") + >>> self.printer.printout_ln(test_msg + "CRASH") with self._lock: >>> self._crashed.append("%s (%s)" % (doc_path, backend.get_name())) >>> return >>> >>> if test_is_failed: - >>> self.printer.print_test_result_ln("FAIL (status error %d)" % >>> (test_is_failed)) + self.printer.printout_ln(test_msg >>> + "FAIL (status error %d)" % (test_is_failed)) with self._lock: >>> self._failed_status_error("%s (%s)" % (doc_path, >>> backend.get_name())) return -- 1.8.0.3 >>> >>> _______________________________________________ poppler mailing >>> list [email protected] >>> http://lists.freedesktop.org/mailman/listinfo/poppler >> > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2.0.19 (GNU/Linux) > > iQEcBAEBAgAGBQJQ3xsCAAoJEPSSjE3STU34CZAIAKvyx1zQDNH5KNx4OfEUSWDy > 8K5SV+J3UcK8HODqbQigzRJ4W7aVc6Hn/TPDXVVbktr7Et4NYmzZaPXIsJ0d2GUR > k7S+XIZ1JbDP/IDzG9iPedMvFL7ZFnTForOS0+DQpDakEDrRMsd6/xhcT2dNDso7 > ssqMG4OW+wGAe4lv5CMNyS1I3dQrggTkOjMS4+yiTPmnkqb+WLA3Ufn9hCjl3AVz > Euyni1z7l3oirajYoU6nf/nk9q/vCcFkHcKKkIFMyjE1VDIStSVLze9NqnyjT5ms > Yy1nAi9mfDWj3/VrdLxmVipeYZHV9+yN4CuAecZPWRnBpy5n9RYDAzaQIE1Io2k= > =lXUz > -----END PGP SIGNATURE----- > _______________________________________________ > poppler mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/poppler -- Carlos Garcia Campos PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462
pgp_B7HNUJX7x.pgp
Description: PGP signature
_______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
