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

[Dylan Baker]

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?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130918/bf5daf84/attachment.pgp>