[Mesa-dev] [PATCH 22/41] glapi: gl_XML.py: convert gl_api.functionIterateByOffset to a generator

[Ilia Mirkin]

On Thu, Mar 31, 2016 at 8:04 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> This implementation is somewhat more efficient than the previous, and is
> less code. It works by replacing one of the list building functions with
> a generator, so that the last list can be avoided. It also decreases the
> size of the list that remains by using sorting instead of a sparse list.
>
> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
> ---
>  src/mapi/glapi/gen/gl_XML.py | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py
> index f96e44a..221bd18 100644
> --- a/src/mapi/glapi/gen/gl_XML.py
> +++ b/src/mapi/glapi/gen/gl_XML.py
> @@ -940,17 +940,15 @@ class gl_api(object):
>      def functionIterateByOffset(self):
>          max_offset = max(f.offset for f in self.functions_by_name.itervalues())
>
> -        temp = [None for i in range(max_offset + 1)]
> +        temp = []
>          for func in self.functions_by_name.itervalues():
>              if func.offset != -1:
> -                temp[func.offset] = func
> +                temp.append((func.offset, func))
>
> -        list = []
> -        for i in range(max_offset + 1):
> -            if temp[i]:
> -                list.append(temp[i])
> -
> -        return iter(list)
> +        for i, (_, func) in enumerate(sorted(temp, key=lambda i: i[0])):
> +            yield func
> +            if i > max_offset:
> +                break

IMHO this is still quite fancy and unnecessarily complicated.

How about

  temp = dict((f.offset, f) for f in self.functions_by_name.itervalues())
  return (temp[offset] for offset in sorted(temp))

That's all that's happening there, right? This lets you not use yield
(which is confusing to most people, as Brian points out), but still
uses a generator to avoid creating that second list, behind the
scenes, while staying understandable to people who understand list
comprehensions (which, hopefully, anyone using python by now does...
they've been around since at least Python 2.2)

[BTW, this is not a complete review of the series... just looking at
this patch because Brian happened to reply to it.]

>
>      def functionIterateAll(self):
>          return self.functions_by_name.itervalues()
> --
> 2.8.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev