Pandas version checks
-
[X] I have checked that this issue has not already been reported.
-
[X] I have confirmed this issue exists on the latest version of pandas.
-
[ ] I have confirmed this issue exists on the main branch of pandas.
Reproducible Example
I'm experiencing a slowdown compared to prior versions when assigning a DataFrame in a for loop :
import pandas as pd
def foo(df):
for idx in df.index:
df.at[idx, "bar"] = 3
df = pd.DataFrame(range(10000))
df["bar"] = 0
%timeit foo(df)
# pandas 1.5.1 : 506 ms ± 20.8 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
My use case is more complex and the loop can't be vectorized, that's why I'm using a for loop instead.
Installed Versions
Prior Performance
%timeit foo(df)
# pandas 1.4.4 : 67 ms ± 8.92 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)
Comment From: phofl
Hi, thanks for your report. Caused by #47074 cc @jorisvandenbossche
labelling as regression for now
Comment From: jorisvandenbossche
Taking a look.
We might not be able to get the performance fully down to what it was before, it was maybe also a bit "too fast". To illustrate, using __setitem__ on a Series is also slower than doing that directly on the underlying array:
In [10]: s = pd.Series(range(10000))
In [11]: %timeit s[0] = 2
7.67 µs ± 92.9 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)
In [12]: %timeit s._values[0] = 2
269 ns ± 1.29 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
This is expected (we do more than what numpy does, plus we have some overhead), while of course we want to limit this slowdown compared to numpy as much as possible.
But so previously, at was using SingleBlockManager.setitem_inplace, which was directly setting the values on the underlying numpy array. While now we go through the more generic Block.setitem, just like we do for Series.__setitem__.
This feels more correct do do (since we also do that for .loc etc), but from a quick look at a perf profile, there is certainly some overhead that can be reduced.
There was also some discussion on the original issue about setitem_inplace vs setitem: https://github.com/pandas-dev/pandas/pull/47074/files#r878902106. Given the performance impact, we could also look in keeping some "inplace" fastpath to test first as we did before. I actually had a commit in that PR that did this (https://github.com/pandas-dev/pandas/commit/453eaba2f6272f89f6ebe33173384bafdcfa9038), but removed in the end again arguing that it was not worth it (but at the time only considered behaviour, not performance). I can check if that commit would improve the reproducer here.
Comment From: Saelyos
Hi, thanks for the investigation and the explanations.
After a bit of digging I found a slowdown even more important in some cases. I had a bit of trouble to make a reproducible example because from what I've understood it has do with the internal blocks so I needed a large DataFrame.
Reproducible Example
import pandas as pd
def foo(df):
for idx in df.index:
df.at[idx, "bar"] = ""
df = pd.DataFrame(range(100000))
df["bar"] = ""
df["a"] = "aaaaaaaaa"
df["b"] = "aaaaaaaaa"
df["c"] = "aaaaaaaaa"
df["d"] = "aaaaaaaaa"
df["e"] = "aaaaaaaaa"
df["f"] = "aaaaaaaaa"
%timeit foo(df)
# pandas 1.5.1: 4.73 s ± 238 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
df = df.copy()
%timeit foo(df)
# pandas 1.5.1: 36.1 s ± 620 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
Prior Performance
%timeit foo(df)
# pandas 1.4.4: 519 ms ± 13.5 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
df = df.copy()
%timeit foo(df)
# pandas 1.4.4: 516 ms ± 18.8 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
Comment From: jorisvandenbossche
I opened two PRs that each partly address this performance regression: https://github.com/pandas-dev/pandas/pull/49771, https://github.com/pandas-dev/pandas/pull/49772
Also your second example is considerably better with those two PRs:
In [5]: %timeit foo(df)
800 ms ± 44.2 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
In [6]: df = df.copy()
In [7]: %timeit foo(df)
822 ms ± 44.7 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
Comment From: rhshadrach
@jorisvandenbossche - is there more to be done here, or would you consider this closed?
Comment From: jorisvandenbossche
I think with both PRs merged and backported for upcoming 1.5.3, we can consider this as closed.