AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
938 stars 332 forks source link

Cleanup backports since we are now Python 3.x+ only #1634

Closed BryceGattis closed 6 months ago

JeanChristopheMorinPerso commented 7 months ago

Can we really replace our lrucache with the built-in one? The one we have has a comment that says that it's modified to make it more performant.

predat commented 7 months ago

Can we really replace our lrucache with the built-in one? The one we have has a comment that says that it's modified to make it more performant.

As far as I know, lru_cache has had a C implementation since at least python 3.7. I've done a few tests of my own, and functools.lru_cache seems to be at least 3x faster than rez.backport.lru_cache.lru_cache with python 3.11.

predat commented 7 months ago

Do you think this is a good place to also tackle enum ?

JeanChristopheMorinPerso commented 7 months ago

Can we really replace our lrucache with the built-in one? The one we have has a comment that says that it's modified to make it more performant.

As far as I know, lru_cache has had a C implementation since at least python 3.7. I've done a few tests of my own, and functools.lru_cache seems to be at least 3x faster than rez.backport.lru_cache.lru_cache with python 3.11.

Can you share the code you used to run your benchmark? We can also run rez's own benchmark (because yes we have benchmarks).

Do you think this is a good place to also tackle enum @predat enum should go in another PR IMO.

predat commented 7 months ago

Can you share the code you used to run your benchmark? We can also run rez's own benchmark (because yes we have benchmarks).

@lru_cache(maxsize=1000)
def fib(n):
    return 1 if n in (0, 1) else fib(n - 1) + fib(n - 2)

with from rez.backport.lru_cache import lru_cache:

python -m timeit -s 'from fib import fib' 'fib(495)'
500000 loops, best of 5: 860 nsec per loop

with from functools import lru_cache:

python -m timeit -s 'from fib import fib' 'fib(495)'
5000000 loops, best of 5: 53.9 nsec per loop
BryceGattis commented 6 months ago

Restored rez/backport/ordereddict.py so it can still be used the src/rez/vendor/pyparsing/pyparsing.py vendor file. 👍

JeanChristopheMorinPerso commented 6 months ago

@BryceGattis you can remove rez/backport/ordereddict.py since it's not used. The code in rez.vendor.pyparsing.pyparsing was not adjusted by us, see https://github.com/pyparsing/pyparsing/blob/adf8dd00b736ba1934914e1db78528af02662b65/pyparsing.py#L133-L139.

BryceGattis commented 6 months ago

@JeanChristopheMorinPerso Notes addressed 👍