Re: [gentoo-portage-dev] [PATCH 8/8] eshowkw: Always group Prefix keywords last

[Michał Górny]

Dnia 26 stycznia 2018 06:45:25 CET, Zac Medico <zmedico@g.o> napisał(a):
>On 01/23/2018 03:25 PM, Alec Warner wrote:
>>
>>
>> On Tue, Jan 23, 2018 at 4:48 PM, Michał Górny <mgorny@g.o
>> <mailto:mgorny@g.o>> wrote:
>>
>>     Always group all Prefix keywords after other types of keywords.
>This
>>     not only ensures that fbsd sorts first but more importantly
>stabilizes
>>     the LHS output between regular and -P variant -- that is, -P
>always adds
>>     additional keywords at the end.
>>     ---
>>      pym/gentoolkit/eshowkw/keywords_header.py | 16 ++++++++++------
>>      1 file changed, 10 insertions(+), 6 deletions(-)
>>
>>     diff --git a/pym/gentoolkit/eshowkw/keywords_header.py
>>     b/pym/gentoolkit/eshowkw/keywords_header.py
>>     index 41b8ba4..1b64bfd 100644
>>     --- a/pym/gentoolkit/eshowkw/keywords_header.py
>>     +++ b/pym/gentoolkit/eshowkw/keywords_header.py
>>     @@ -142,12 +142,16 @@ class keywords_header:
>>                                             break
>>
>>                     # sort by, in order (to match Bugzilla):
>>     -               # 1. arch, then ~arch
>>     -               # 2. profile stability
>>     -               # 3. short keywords, then long (prefix, fbsd)
>>     -               # 4. keyword name in reverse component order
>>     -               normal.sort(key=lambda kw: (kw in
>>     self.__TESTING_KW_ARCHS,
>>     -                       levels.get(kw, 99), kw.count('-'),
>>     list(reversed(kw.split('-')))))
>>     +               # 1. non-prefix, then prefix (stable output
>between
>>     -P and not)
>>     +               # 2. arch, then ~arch
>>     +               # 3. profile stability
>>     +               # 4. short keywords, then long (prefix, fbsd)
>>     +               # 5. keyword name in reverse component order
>>     +               normal.sort(key=lambda kw: (self.__isPrefix(kw),
>>     +                       kw in self.__TESTING_KW_ARCHS,
>>     +                       levels.get(kw, 99),
>>     +                       kw.count('-'),
>>     +                       list(reversed(kw.split('-')))))
>>
>>
>> I'm a bit sad about this lambda because its ended up a bit long.
>>
>> What are your thoughts on splitting it out?
>
>The fact that it's a lambda doesn't bother me so much as the
>inefficiency of regenerating the key on every call. I've found this
>cute
>little memodict decorator that will optimize it nicely:
>
>http://code.activestate.com/recipes/578231-probably-the-fastest-memoization-decorator-in-the-/
>
>def memodict(f):
>    class memodict(dict):
>        def __missing__(self, key):
>            ret = self[key] = f(key)
>            return ret
>    return memodict().__getitem__

Do you really think saving a few microseconds is worth the added complexity? Because if you do, then I'd rather turn keyword into a class and cache everything properly rather than adding ugly hacks to optimize one call site.

-- 
Best regards,
Michał Górny (by phone)