Should we cache some small properties?

[max-sixty]

I was doing some profiling on isel, and see there are some properties that (I think) never change, but are called frequently. Should we cache these on their object?

Pandas uses cache_readonly for these cases.

Here's a case: we call LazilyOuterIndexedArray.shape frequently when doing a simple indexing operation. Each call takes ~150µs. An attribute lookup on a python object takes ~50ns (i.e. 3000x faster). IIUC the result on that property should never change.

I don't think this is the solution to performance issues, and there's some additional complexity.
Could they be easy & small wins, though?

[shoyer]

I think this would be totally fine to add. A variant on cache_readonly appears in Python 3.8 standard library as functools.cached_property. I think we could backport that pretty easily:
https://github.com/python/cpython/blob/d360346640e19231032b072216195484fa2450b4/Lib/functools.py#L924

[max-sixty]

Great, I didn't know about that, thanks

It does remind me, though, that I'm not sure it's possible given we're using __slots__, at least without adding an entry to __slots__...
https://github.com/python/cpython/blob/d360346640e19231032b072216195484fa2450b4/Lib/functools.py#L954-L958

[crusaderky]

By reading the implementation of cachedproperty, it needs a __dict__. It should be straightforward to write a variant that uses slots though.

[max-sixty]

By reading the implementation of cachedproperty, it needs a __dict__. It should be straightforward to write a variant that uses slots though.

Is there a way of doing that which doesn't nullify the benefits of slots? i.e. if we add a _cache_dict to the class, is that the same overhead as having a __dict__?

[crusaderky]

@max-sixty afraid so. But as I said it should be straightforward to use a variant that instead of self.__dict__[self.name] uses getattr/setattr.

[crusaderky]

I prefer pandas.utils.cache_readonly to the Python 3.8 cachedproperty. Besides not needing a dict, it is also written in cython, which should make a substantial difference.

max-sixty could you post your benchmark where you measure 150us? I tried caching that property with @cache_readonly and I only get a boost of 7us.

import xarray
ds = xarray.Dataset({'d': ('x', [1, 2]), 'x': [10, 20]})
ds
ds.to_netcdf("foo.nc")
ds.close()
ds = xarray.open_dataset("foo.nc")

%timeit ds.isel(x=[0])

With @property: 187us
With @cache_readonly: 180us

[crusaderky]

%%prun -s cumulative
for _ in range(10000):
    ds.isel(x=[0])

Output (uncached):

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
    10000    0.148    0.000    3.317    0.000 dataset.py:1854(isel)
    [...]
    60000    0.092    0.000    0.122    0.000 indexing.py:535(shape)