[Piglit] [PATCH 00/35] Serialize profiles into XML at build time

[Michel Dänzer]

On 2018-05-10 03:08 AM, Matt Turner wrote:
> On Tue, May 8, 2018 at 7:07 AM, Michel Dänzer <michel at daenzer.net> wrote:
>> On 2018-05-07 06:49 PM, Michel Dänzer wrote:
>>> On 2018-05-07 06:44 PM, Dylan Baker wrote:
>>>> Quoting Tomi Sarvela (2018-05-07 01:20:46)
>>>>>
>>>>> piglit/framework$ diff -c profile.py.orig profile.py
>>>>> *** profile.py.orig     2018-05-07 19:11:37.649994643 +0300
>>>>> --- profile.py  2018-05-07 19:11:46.880994608 +0300
>>>>> ***************
>>>>> *** 584,591 ****
>>>>>                # more code, and adding side-effects
>>>>>                test_list = (x for x in test_list if filterby(x))
>>>>>
>>>>> !         pool.imap(lambda pair: test(pair[0], pair[1], profile, pool),
>>>>> !                   test_list, chunksize)
>>>>>
>>>>>        def run_profile(profile, test_list):
>>>>>            """Run an individual profile."""
>>>>> --- 584,591 ----
>>>>>                # more code, and adding side-effects
>>>>>                test_list = (x for x in test_list if filterby(x))
>>>>>
>>>>> !         pool.map(lambda pair: test(pair[0], pair[1], profile, pool),
>>>>> !                  test_list, chunksize)
>>>>>
>>>>>        def run_profile(profile, test_list):
>>>>>            """Run an individual profile."""
>>>>>
>>>>>
>>>>> Tomi
>>>>
>>>> Juan, can you test this patch and see if it resolves your issue as well? I'm not
>>>> sure why this is fixing things, but if it does I'm happy to merge it and deal
>>>> with any performance problems it introduces later.
>>>
>>> FWIW, this patch doesn't fix the gpu profile running a lot fewer tests
>>> now than it did before 9461d92301e72807eba4776a16a05207e3a16477. I'm
>>> also using -x.
>>
>> I just bisected another problem to
>> 9461d92301e72807eba4776a16a05207e3a16477: The xts-render profile doesn't
>> work anymore. Most of the time, it doesn't even start:
>>
>> [000/480]
>> Traceback (most recent call last):
>>   File "./piglit", line 178, in <module>
>>     main()
>>   File "./piglit", line 174, in main
>>     sys.exit(runner(args))
>>   File "/home/daenzer/src/piglit-git/piglit/framework/exceptions.py", line 51, in _inner
>>     func(*args, **kwargs)
>>   File "/home/daenzer/src/piglit-git/piglit/framework/programs/run.py", line 370, in run
>>     backend.finalize({'time_elapsed': time_elapsed.to_json()})
>>   File "/home/daenzer/src/piglit-git/piglit/framework/backends/json.py", line 163, in finalize
>>     assert data['tests']
>> AssertionError
>>
>> Sometimes, it doesn't fail like this, but only runs between 0 and another
>> number < 480 of tests. Very rarely, it manages to run all tests.
>>
>> (I'm using python 3.6 now)
>>
>>
>> Dylan, since a number of issues have been reported to have started with
>> this commit, and you don't seem to have an idea what's wrong with it,
>> can you revert it and anything depending on it for the time being? I'll
>> be happy to test against the issues I've run into when you're ready to
>> try again.
> 
> Do you think that is a good workflow? (Serious question)

Who said anything about "workflow"? It's just damage control. I did say
it would suck.

Would a single change causing multiple regressions be tolerated for so
long if any of the regressions affected the Intel CI?


> Dylan's patches were on the list for three weeks and I think only one
> person (Rafael) tested them. It doesn't make sense to me to
> significantly increase the burden on the person writing the code (by
> reverting all the patches when a problem is found) in exchange for a
> promise to test the patches... which you or anyone else could have
> done during the three weeks Dylan was practically begging for testers.
> 
> It's frustrating for me, just as an observer, to see that not even the
> people who have so loudly complained about the lack of this very
> feature could be bothered to try it out.

I don't know what you mean by "this very feature", but I don't remember
ever "complaining loudly" about the lack of any feature in piglit, so I
guess I'm not one of those people.

Anyway, these patches weren't on my radar; I had no idea they might
cause such issues, and I certainly don't have the bandwidth to even look
at all piglit patches (not that it would make any difference when it
comes to Python), let alone test them. Does that mean I can't report any
issues I run into?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer