[Piglit] [PATCH 4/8] framework: Remove micro-optimization with no impact.

[Eric Anholt]

Dylan Baker <baker.dylan.c at gmail.com> writes:

> On Wednesday 18 September 2013 15:00:29 Eric Anholt wrote:
>> We can compute these lists anyway.  Note how we don't really do much
>> work to generate
>> ---
>>  framework/summary.py | 59
>> ++++++++++++++++++++-------------------------------- 1 file changed, 23
>> insertions(+), 36 deletions(-)
>> 
>> diff --git a/framework/summary.py b/framework/summary.py
>> index 656efc0..8f6f5f7 100644
>> --- a/framework/summary.py
>> +++ b/framework/summary.py
>> @@ -397,19 +397,10 @@ class Summary:
>>              # Create a list with all the test names in it
>>              self.tests['all'] = set(self.tests['all']) | set(each.tests)
>> 
>> -    def __generate_lists(self, lists):
>> +    def __generate_lists(self):
>>          """
>>          Private: Generate the lists of changes, problems, regressions,
>> fixes, and skips
>> -
>> -        lists is a list contianing any of the following: changes, problems,
>> -        skips, fixes (which will also generate regressions)
>> -
>> -        This method has different code paths to allow the exclusion of
>> certain -        lists being generated. This is both useful for speeding up
>> HTML -        generation when a page isn't needed (regressions with only
>> one test -        file is provided), and for JUnit and text which only need
>> a limited -        subset of these lists
>>          """
>>          def find_regressions(status):
>>              """
>> @@ -441,31 +432,27 @@ class Summary:
>>                  status_text.append(s)
>>                  status.append(find_regressions(s))
>> 
>> -            if 'changes' in lists:
>> -                # Check and append self.tests['changes']
>> -                # A set cannot contain duplicate entries, so creating a set
>> -                # out the list will reduce it's length to 1 if all entries
>> -                # are the same, meaning it is not a change
>> -                if len(set(status)) > 1:
>> -                    self.tests['changes'].add(test)
>> -
>> -            if 'problems' in lists:
>> -                for i in status_text:
>> -                    if i not in ['pass', 'skip', 'special']:
>> -                        self.tests['problems'].add(test)
>> -
>> -            if 'skipped' in lists:
>> -                if 'skip' in status_text:
>> -                    self.tests['skipped'].add(test)
>> -
>> -            if 'fixes' in lists:
>> -                # Find both fixes and regressions, and append them to the
>> -                # proper lists
>> -                for i in xrange(len(status) - 1):
>> -                    if status[i] < status[i + 1] and status[i] != 0:
>> -                        self.tests['regressions'].add(test)
>> -                    if status[i] > 1 and status[i + 1] == 1:
>> -                        self.tests['fixes'].add(test)
>> +            # Check and append self.tests['changes']
>> +            # A set cannot contain duplicate entries, so creating a set
>> +            # out the list will reduce it's length to 1 if all entries
>> +            # are the same, meaning it is not a change
>> +            if len(set(status)) > 1:
>> +                self.tests['changes'].add(test)
>> +
>> +            for i in status_text:
>> +                if i not in ['pass', 'skip', 'special']:
>> +                    self.tests['problems'].add(test)
>> +
>> +            if 'skip' in status_text:
>> +                self.tests['skipped'].add(test)
>> +
>> +            # Find both fixes and regressions, and append them to the
>> +            # proper lists
>> +            for i in xrange(len(status) - 1):
>> +                if status[i] < status[i + 1] and status[i] != 0:
>> +                    self.tests['regressions'].add(test)
>> +                if status[i] > 1 and status[i + 1] == 1:
>> +                    self.tests['fixes'].add(test)
>> 
>>      def __find_totals(self):
>>          """
>> @@ -560,7 +547,7 @@ class Summary:
>>          else:
>>              pages = ['problems', 'skipped']
>> 
>> -        self.__generate_lists(pages)
>> +        self.__generate_lists()
>> 
>>          # Index.html is a bit of a special case since there is index, all,
>> and # alltests, where the other pages all use the same name. ie,
>
> IIRC there is a pretty major performance difference as more tests are added to 
> the list, but if there isn't then could we just fold this back into the 
> constructor, as it was originally?

I tested with junit and a single results list of a full piglit run,
which should highlight this "optimization" to the maximum extent
possible as far as I can tell.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130918/887d2386/attachment.pgp>