Use functools.cached_property instead of django.utils.functional.cached_property.

[Thomas Grainger]

functools gains cached_property in Python 3.8: ​https://bugs.python.org/issue21145

django.utils.functional.cached_property should import it and deprecate its own implementation

[Simon Charette]

FWIW functools.cached_property adds a RLock acquisition overhead for thread safety which is not something needed by most internal Django's usage and third party ones from my personal experience.

If we were to proceed with this patch we should measure the impact of this thread safety mechanism first and also document it.

[Claude Paroz]

I don't think we should do anything until Python 3.8 becomes the minimal Python supported by Django.

[Mariusz Felisiak]

I agree with Claude, we can reconsider this ticket when Python 3.8 becomes the minimal Python supported by Django.

[David Smith]

We're nearing the point where Python 3.8 will become the minimum supported version. I've therefore started to look at this ticket as I think "someday" has now arrived, and hopefully we can make a decision one way or the other with this.

The first thing I've done is to look at the performance of the two functions and have put together a script ​here. Running it with isolated CPUs is giving repeatable results that show very little (if anything) between the two functions. This benchmark was run with Python 3.9.

There were previous comments about the impact of the RLock mechanism, this is only used the first time when the ​value is set. I suspect this is why I'm seeing little performance difference between the two. (Hopefully, I've understood the context of these comments correctly)

If no one has any objections at this stage, I'll prepare a patch in due course.

python bench_cached_property.py
.....................
Django Cache: Mean +- std dev: 1.07 us +- 0.05 us
.....................
Python Cache: Mean +- std dev: 1.08 us +- 0.05 us

[Adam Johnson]

LGTM, thanks for the benchmarking David

[David Smith]

An issue has been opened against Python's cached_property ​https://bugs.python.org/issue43468

Thanks to Antti Haapala for highlighting this.

[Rainer Koirikivi]

I created a quick-and-dirty version of the benchmark with IO and multithreading to highlight the performance issues: ​https://gist.github.com/koirikivi/c58d30fce18ac1f0d65f06bfa4f93743

[Mateusz Leszko]

Replying to Simon Charette:

FWIW functools.cached_property adds a RLock acquisition overhead for thread safety which is not something needed by most internal Django's usage and third party ones from my personal experience.

If we were to proceed with this patch we should measure the impact of this thread safety mechanism first and also document it.

Hi, I am trying to solve this issue, but is't it true that: if functools.cached_property uses RLock, then we don't need to worry about it, cause its python3 implementation and we can trust it?

[Adam Johnson]

then we don't need to worry about it, cause its python3 implementation and we can trust it?

It would be a performance regression. Especially see the above linked issue which implies massive degradation with multiple threads/processes.

[Thomas Grainger]

I think development of a fixed cached_property has stalled? It looks like it might even be removed from CPython.

​https://mobile.twitter.com/raymondh/status/1431016905276633088

I think this ticket should be wontfix, and there should be a docs in Django about why you MUST not use @functools.cached_property and always use the django one

[Filip Sedlák]

One thing to note is that the @functools.cached_property supports a use case that Django's doesn't. If you define a subclass with plain @property, the caching doesn't happen because the @property is a "data descriptor" so it gets precedence before the cached value. It's still not clear to me why super().count also avoids the cached value in self.__dict__ but it clearly does.

In [8]: class A:
   ...:     @django.utils.functional.cached_property
   ...:     def count(self):
   ...:         print('Computing... A.count')
   ...:         return 15
   ...: 
   ...: 
   ...: class B(A):
   ...:     @property
   ...:     def count(self):
   ...:         print('B.count')
   ...:         return super().count
   ...: 
   ...: 

In [9]: b = B()

In [10]: b.count
B.count
Computing... A.count                          <===== expected
Out[10]: 15

In [11]: b.count
B.count
Computing... A.count                          <===== should not happen
Out[11]: 15

The difference with functools is that functools.cached_property still explicitly looks into instance.__dict__ if it's called.

It's a corner case but I expected super().count to be cached. It hit us when overriding Paginator.count. The lack of caching led to four count queries. Small impact, easy workaround, but surprising behaviour.

[Akshet Pandey]

The locking behavior for functool.cached_property was fixed in 3.12. See: ​https://github.com/python/cpython/issues/87634#issuecomment-1467140709
Someday can now be when minimum version is python 3.12 for django.