PERF: regression in DataFrame reduction ops performance

[jorisvandenbossche]

From https://pandas.pydata.org/speed/pandas/#stat_ops.FrameOps.time_op?Cython=0.29.21&Cython=0.29.16&p-op='sum'&p-dtype='int'&p-axis=0&commits=3a043f2d-4c03d07b&x-axis-scale=date

Reproducer:

values = np.random.randn(100000, 4)  
df = pd.DataFrame(values).astype("int")
%timeit df.sum()

increased with a factor 2 to 3 x somewhere the last days.

[ukarroum]

Would like to try working on that if possible.

[ukarroum]

take

[jbrockmendel]

#35881 is a candidate, though thats a much bigger impact than id expect

[jorisvandenbossche]

Checking out that commit vs the one before, seems to confirm that #35881 is indeed the cause of the slowdown

[jorisvandenbossche]

I did a quick profile, and commented on #35881 about some observations

[jorisvandenbossche]

@ukarroum especially #35881 (comment) is something you could test

[ukarroum]

Can confirm that by using self._iter_column_arrays() instead of self.dtypes we have significant performance improvement :

With self.dtypes :

In [8]: values = np.random.randn(100000, 4)   
   ...: df = pd.DataFrame(values).astype("int") 
   ...: %timeit df.sum() 
714 µs ± 21 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

With self._iter_column_arrays()

In [4]: values = np.random.randn(100000, 4)   
   ...: df = pd.DataFrame(values).astype("int") 
   ...: %timeit df.sum() 
477 µs ± 8.7 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

Gonna submit a PR soon.

[jorisvandenbossche]

Reopening since this dtype change only fixed part of the regression. See the updated plot at https://pandas.pydata.org/speed/pandas/#stat_ops.FrameOps.time_op?Cython=0.29.21&p-op='sum'&p-dtype='int'&p-axis=0&commits=3a043f2d-4c03d07b&x-axis-scale=date

[ukarroum]

Gonna do some profiling and open a PR asap.

[ukarroum]

The origin of the remaining regression is changing this condition :

if numeric_only is not None:

312 µs ± 2.74 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

to :

if numeric_only is not None or (
            numeric_only is None
            and axis == 0
            and not any_object
            and not self._mgr.any_extension_types
        ):

474 µs ± 2.29 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

in commit 7d257c6

EDIT : by removing the code not accessible from the used reproducer (check 1st comment), we have the minimal code :

df = self

ignore_failures = numeric_only is None

res, indexer = df._mgr.reduce(blk_func, ignore_failures=ignore_failures)
out = df._constructor(res).iloc[0]

return out

Changes in the above commit don't seem to add significant perfs regression to _mgr.reduce.

[ukarroum]

replacing out = df._constructor(res).iloc[0] with an 0(1) instruction (hardcoded serie) has almost no impact on time execution.

df._mgr.reduce(blk_func, ignore_failures=ignore_failures) is the line adding the 150 microsec.

EDIT : using try: func(values) before the above condition seems to solve the problem and passes the tests.

but i m not confident to do a PR (yet) since i don't fully understand this code and i'm afraid it may break some use cases that may need to use the condition block even if the try won't fail.

[jbrockmendel]

We don't particularly need ignore_failures in BlockManager.reduce right now, but it will be needed when we fix the many reduction bugs caused by calling .values further down in DataFrame._reduce. If the perf impact is that big, it can be reverted in the interim i guess.

[ukarroum]

@jbrockmendel : Actually we don't need to revert the whole change, we only need to revert the following condition :

if numeric_only is not None or (
            numeric_only is None
            and axis == 0
            and not any_object
            and not self._mgr.any_extension_types
        ):

to :

if numeric_only is not None:

This will solve the regression issue

EDIT : Do you think i can safely do so ?

[jbrockmendel]

Do you think i can safely do so ?

In the short term, yes, since we are not really relying on this. The upcoming bugfixes will rely on it. So its really a question of if we want to revert then unrevert later.