[poppler] RFC: patch: parallel testing

Adam Reichold adamreichold at myopera.com
Sat Dec 29 08:32:02 PST 2012


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

Best regards, Adam.

Am 29.12.2012 17:13, schrieb Carlos Garcia Campos:
> Adam Reichold <adamreichold at myopera.com> 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 <adamreichold at myopera.com> 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
>>>>>>>>>>>> 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
>>>> 
>> From 5e7461569e3085370326203539fa02de968eb2ca Mon Sep 17 00:00:00
>> 2001 From: Adam Reichold <adamreichold at myopera.com> 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 poppler at lists.freedesktop.org 
>> 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-----


More information about the poppler mailing list