PERF: Remove unnecessary copies in sorting functions (#33917)

[mproszewska]

• closes Performance regression in timeseries.SortIndex.time_sort_index #33917
• tests passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

This PR fixes pereformance regression after commit dec736f3f by removing unnecessary copies if key=None in sorting functions.

setup = """
import pandas as pd
import numpy as np
N = 10 ** 5
idx = pd.date_range(start="1/1/2000", periods=N, freq="s")
s = pd.Series(np.random.randn(N), index=idx)
"""

import timeit
timeit.timeit("s.sort_index()",setup=setup, number=100000)

# master
# 41.245621485984884
# now
# 14.826273362035863

[pep8speaks]

Hello @mproszewska! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-06-03 01:25:38 UTC

[jreback]

can you add an asv which hits this. I think this regression is in 1.1 so no whatsnew is needed.

[jreback]

also in tests where this is hit, can you assert that values is not modified

[mproszewska]

also in tests where this is hit, can you assert that values is not modified

This is used in function that sorts series by index, so I believe that values=index will be modified

[TomAugspurger]

I don't think we need this. SortIndex.time_sort_index already covers this code path.

[mproszewska]

Right, I haven't checked timeseries before adding asv

[TomAugspurger]

Can you add a release note stating that we fixed the performance regression in sort_index?

[mproszewska]

Can you add a release note stating that we fixed the performance regression in sort_index?

Should I add note even though regression was in 1.1?

[jreback]

Can you add a release note stating that we fixed the performance regression in sort_index?

Should I add note even though regression was in 1.1?

no this is fine (just add the asvs)

[mproszewska]

Can you add a release note stating that we fixed the performance regression in sort_index?

Should I add note even though regression was in 1.1?

no this is fine (just add the asvs)

Isn't timeseries.SortIndex.time_sort_index enough?

[jreback]

what does it show pls post a run

[mproszewska]

     <perf>           <master>  
+        158±50μs         367±30μs     2.32  timeseries.SortIndex.time_sort_index(True)
+      3.84±0.4ms       5.94±0.5ms     1.55  timeseries.DatetimeAccessor.time_dt_accessor_normalize(None)
-      3.31±0.1ms       2.47±0.3ms     0.75  timeseries.ResampleDataFrame.time_method('max')

[jreback]

looks fine

can u merge master and ping on green

[mproszewska]

looks fine

can u merge master and ping on green

ping

[jreback]

thanks @mproszewska