Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PERF: assigning in a for loop #49729

Closed
2 of 3 tasks
Saelyos opened this issue Nov 16, 2022 · 6 comments
Closed
2 of 3 tasks

PERF: assigning in a for loop #49729

Saelyos opened this issue Nov 16, 2022 · 6 comments
Labels
Indexing Related to indexing on series/frames, not to indexes themselves Internals Related to non-user accessible pandas implementation Performance Memory or execution speed performance Regression Functionality that used to work in a prior pandas version
Milestone

Comments

@Saelyos
Copy link

Saelyos commented Nov 16, 2022

Pandas version checks

  • I have checked that this issue has not already been reported.

  • 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

INSTALLED VERSIONS

commit : 91111fd
python : 3.9.6.final.0
python-bits : 64
OS : Linux
OS-release : 5.10.0-19-amd64
Version : #1 SMP Debian 5.10.149-2 (2022-10-21)
machine : x86_64
processor :
byteorder : little
LC_ALL : None
LANG : fr_FR.UTF-8
LOCALE : fr_FR.UTF-8

pandas : 1.5.1
numpy : 1.23.3
pytz : 2022.2.1
dateutil : 2.8.2
setuptools : 65.4.0
pip : 22.2.2
Cython : 0.29.32
pytest : 6.2.5
hypothesis : None
sphinx : None
blosc : None
feather : None
xlsxwriter : 3.0.3
lxml.etree : 4.9.1
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : 3.1.2
IPython : 8.5.0
pandas_datareader: None
bs4 : 4.11.1
bottleneck : None
brotli : None
fastparquet : None
fsspec : None
gcsfs : None
matplotlib : 3.6.0
numba : None
numexpr : None
odfpy : None
openpyxl : 3.0.10
pandas_gbq : None
pyarrow : None
pyreadstat : None
pyxlsb : None
s3fs : None
scipy : 1.9.1
snappy : None
sqlalchemy : 1.4.41
tables : None
tabulate : None
xarray : None
xlrd : None
xlwt : None
zstandard : None
tzdata : None

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)
@Saelyos Saelyos added Needs Triage Issue that has not been reviewed by a pandas team member Performance Memory or execution speed performance labels Nov 16, 2022
@phofl
Copy link
Member

phofl commented Nov 16, 2022

Hi, thanks for your report. Caused by #47074 cc @jorisvandenbossche

labelling as regression for now

@phofl phofl added this to the 1.5.3 milestone Nov 16, 2022
@phofl phofl added Indexing Related to indexing on series/frames, not to indexes themselves Regression Functionality that used to work in a prior pandas version Internals Related to non-user accessible pandas implementation labels Nov 16, 2022
@jorisvandenbossche
Copy link
Member

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 (453eaba), 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.

@Saelyos
Copy link
Author

Saelyos commented Nov 17, 2022

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)

@jorisvandenbossche
Copy link
Member

I opened two PRs that each partly address this performance regression: #49771, #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)

@rhshadrach
Copy link
Member

@jorisvandenbossche - is there more to be done here, or would you consider this closed?

@rhshadrach rhshadrach removed the Needs Triage Issue that has not been reviewed by a pandas team member label Dec 15, 2022
@jorisvandenbossche
Copy link
Member

I think with both PRs merged and backported for upcoming 1.5.3, we can consider this as closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Indexing Related to indexing on series/frames, not to indexes themselves Internals Related to non-user accessible pandas implementation Performance Memory or execution speed performance Regression Functionality that used to work in a prior pandas version
Projects
None yet
Development

No branches or pull requests

4 participants