Make sizing computation much faster for nested containers

[ogayot]

Hello,

Today, I investigated a performance regression in Subiquity after moving from urwid 2.1.2 to urwid 2.6.10. It quickly turned out that Subiquity is relying too much on widget nesting and that the calls to Pile.sizing started taking forever after #733 got merged.

When nesting Columns, Pile and other containers, calls to widget.sizing() can become very slow, especially in slower architectures. Profiling showed that we are spending a big slice of time in enum.IntFlag dunder methods (i.e, __new__, _get_value, __or__, __and__ and __call__).

   111191    0.014    0.000    0.014    0.000 enum.py:1129(__new__)
   333573    0.039    0.000    0.053    0.000 enum.py:1544(_get_value)
    56592    0.040    0.000    0.086    0.000 enum.py:1551(__or__)
    54599    0.037    0.000    0.080    0.000 enum.py:1562(__and__)
   111191    0.021    0.000    0.035    0.000 enum.py:726(__call__)

Although enum.IntFlag feels cleaner than enum.IntEnum for the job, the performance of enum.IntEnum is much better.

In Subiquity, switching from enum.IntFlag to enum.IntEnum causes calls to Pile.sizing() to be about 10 times faster.

Difference before/after this patch:

1069704 function calls (1056415 primitive calls) in 0.203 seconds
74088 function calls (60799 primitive calls) in 0.027 seconds

(yes, we have too much nesting... we need to fix it)

Benchmark with enum.IntFlag and enum.IntEnum

import cProfile
import enum

class FL(enum.IntFlag):
    A = enum.auto()
    B = enum.auto()
    C = enum.auto()
    D = enum.auto()
    E = enum.auto()

class EN(enum.IntEnum):
    A = 0b1
    B = 0b10
    C = 0b100
    D = 0b1000
    E = 0b10000


with cProfile.Profile() as pr:
    for i in range(100000):
        FL.A | FL.B

pr.print_stats()

with cProfile.Profile() as pr:
    for i in range(100000):
        EN.A | EN.B

pr.print_stats()

         900020 function calls in 0.158 seconds

   Ordered by: standard name

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.000    0.000    0.000    0.000 cProfile.py:121(__exit__)
        3    0.000    0.000    0.000    0.000 enum.py:115(_iter_bits_lsb)
   100000    0.012    0.000    0.012    0.000 enum.py:1152(__new__)
        3    0.000    0.000    0.000    0.000 enum.py:1435(_iter_member_by_value_)
        1    0.000    0.000    0.000    0.000 enum.py:1455(_missing_)
   300000    0.040    0.000    0.055    0.000 enum.py:1585(_get_value)
   100000    0.070    0.000    0.158    0.000 enum.py:1592(__or__)
   100000    0.020    0.000    0.033    0.000 enum.py:695(__call__)
        1    0.000    0.000    0.000    0.000 {built-in method __new__ of type object at 0xa00020}
        1    0.000    0.000    0.000    0.000 {built-in method builtins.hasattr}
   300003    0.015    0.000    0.015    0.000 {built-in method builtins.isinstance}
        2    0.000    0.000    0.000    0.000 {method 'append' of 'list' objects}
        1    0.000    0.000    0.000    0.000 {method 'disable' of '_lsprof.Profiler' objects}
        2    0.000    0.000    0.000    0.000 {method 'get' of 'dict' objects}
        1    0.000    0.000    0.000    0.000 {method 'join' of 'str' objects}
        1    0.000    0.000    0.000    0.000 {method 'setdefault' of 'dict' objects}


         2 function calls in 0.000 seconds

   Ordered by: standard name

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.000    0.000    0.000    0.000 cProfile.py:121(__exit__)
        1    0.000    0.000    0.000    0.000 {method 'disable' of '_lsprof.Profiler' objects}

Checklist

• I've ensured that similar functionality has not already been implemented
• I've ensured that similar functionality has not earlier been proposed and declined
• I've branched off the master branch
• I've merged fresh upstream into my branch recently
• I've ran tox successfully in local environment

[penguinolog]

Super useful change.
please put # fmt: off / # fmt: on around enum - formatters love only 1 space around =.

[ogayot]

Super useful change. please put # fmt: off / # fmt: on around enum - formatters love only 1 space around =.

Implemented thanks!

I had to make further modifications to make the tests pass. While IntFlag | IntFlag results in another IntFlag, IntEnum | IntEnum results in an int. Sadly, I think this makes the code harder to read.

There's a bit of a tradeoff between performance and readability here. I'm open for suggestions to make this better :)

[coveralls]

Pull Request Test Coverage Report for Build 13386614645

Details

• 20 of 20 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 74.78%

---

Totals
Change from base Build 13363725938:	0.0%
Covered Lines:	9264
Relevant Lines:	12436

---

💛 - Coveralls

[penguinolog]

Release 3.0 expected next week