PERF: Speeds up creation of Period, PeriodArray, with Offset freq

[TomAugspurger]

master:

In [2]: freq = pd.tseries.offsets.Day()
   ...:
   ...: %timeit pd.Period("2001", freq=freq)

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

In [3]: %timeit pd.Period._maybe_convert_freq(freq)
   ...:
64.7 µs ± 382 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)

branch:

In [2]: freq = pd.tseries.offsets.Day()
   ...:
   ...: %timeit pd.Period("2001", freq=freq)

158 µs ± 2.87 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

In [3]: %timeit pd.Period._maybe_convert_freq(freq)
193 ns ± 4.3 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

While looking at the profile plot in snakeviz, it seems like a lot of time in
Period._maybe_convert_freq was spent importing modules. _maybe_convert_freq
calls offsets.to_offset, which imports a Python function inside the method.

Does Cython not handle this well?

[TomAugspurger]

cc @jbrockmendel

[codecov]

Codecov Report

Merging #23589 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #23589      +/-   ##
==========================================
- Coverage   92.25%   92.25%   -0.01%     
==========================================
  Files         161      161              
  Lines       51237    51260      +23     
==========================================
+ Hits        47269    47290      +21     
- Misses       3968     3970       +2

Flag	Coverage Δ
#multiple	90.63% <ø> (ø)	⬆️
#single	42.33% <ø> (+0.03%)	⬆️

Impacted Files	Coverage Δ
pandas/core/arrays/datetimes.py	98.42% <0%> (-0.43%)	⬇️
pandas/core/arrays/period.py	98.08% <0%> (-0.01%)	⬇️
pandas/core/dtypes/generic.py	100% <0%> (ø)	⬆️
pandas/core/arrays/datetimelike.py	95.92% <0%> (+0.02%)	⬆️
pandas/core/arrays/timedeltas.py	93.78% <0%> (+0.03%)	⬆️
pandas/util/testing.py	86.78% <0%> (+0.14%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce62a5c...795a7d1. Read the comment docs.

[jorisvandenbossche]

This already improves the slowdown (using the example code of the plotting benchmark), like from 5.5 s to 2 s, but it is still a lot slower than on 0.23 (100 ms)

Issues related to previous time this happened: #11831, #12909, #12903)

[TomAugspurger]

master

· Discovering benchmarks
· Running 3 total benchmarks (1 commits * 1 environments * 3 benchmarks)
[  0.00%] ·· Benchmarking existing-py_Users_taugspurger_Envs_pandas-dev_bin_python3
[ 16.67%] ··· plotting.TimeseriesPlotting.time_plot_irregular                                                                                        265±0ms
[ 33.33%] ··· plotting.TimeseriesPlotting.time_plot_regular                                                                                          4.94±0s
[ 50.00%] ··· plotting.TimeseriesPlotting.time_plot_regular_compat                                                                                   204±0ms

branch:

· Discovering benchmarks
· Running 3 total benchmarks (1 commits * 1 environments * 3 benchmarks)
[  0.00%] ·· Benchmarking existing-py_Users_taugspurger_Envs_pandas-dev_bin_python3
[ 16.67%] ··· plotting.TimeseriesPlotting.time_plot_irregular                                                                                        204±0ms
[ 33.33%] ··· plotting.TimeseriesPlotting.time_plot_regular                                                                                          1.90±0s
[ 50.00%] ··· plotting.TimeseriesPlotting.time_plot_regular_compat                                                                                   197±0ms

[jreback]

thanks!

[jorisvandenbossche]

To be clear, although this PR improved performance a bit, the perf regression is not yet fixed

[jbrockmendel]

but it is still a lot slower than on 0.23

IIRC the version of to_offset in tslibs.offsets was implemented largely for aesthetic purposes, to have the runtime import in just one place. If there is a perf hit, let's absolutely revert it.

The long-term solution may just be to move to_offset up to cython. The PITA is that requires moving most of tseries.offsets up to cython.

[jorisvandenbossche]

Not saying that the remaining performance hit is necessarily due to to_offset (didn't look enough in detail for that), just that there are still performance problems in general with Periods.

For the plotting, it seems to be coming from Period.asfreq, which uses to_offset amongst other things