CLN/ERR: str.cat internals by h-vetinari · Pull Request #22725 · pandas-dev/pandas (original) (raw)
@WillAyd @jreback
Unfortunately, I have bad news. I started out in this PR with a very idiomatic solution (see the first couple commits), and it was just too slow.
Here's the ASV for the last commit:
All benchmarks:
before after ratio
[b28cf5aa] [a97fe67e]
<master> <cln_str_cat>
9.38±0ms 9.38±0ms 1.00 strings.Cat.time_cat(0, ',', '-', 0.0)
9.38±0.8ms 10.9±0ms ~1.17 strings.Cat.time_cat(0, ',', '-', 0.001)
10.9±0ms 12.5±0.6ms ~1.14 strings.Cat.time_cat(0, ',', '-', 0.15)
9.38±0.6ms 8.59±0.8ms 0.92 strings.Cat.time_cat(0, ',', None, 0.0)
- 12.5±0.8ms 10.9±0ms 0.87 strings.Cat.time_cat(0, ',', None, 0.001)
- 14.1±0ms 10.9±0ms 0.78 strings.Cat.time_cat(0, ',', None, 0.15)
9.38±0ms 9.38±0.8ms 1.00 strings.Cat.time_cat(0, None, '-', 0.0)
9.38±0ms 9.38±0.8ms 1.00 strings.Cat.time_cat(0, None, '-', 0.001)
10.9±0.8ms 12.5±0.8ms ~1.14 strings.Cat.time_cat(0, None, '-', 0.15)
10.9±2ms 7.81±0.8ms ~0.71 strings.Cat.time_cat(0, None, None, 0.0)
7.81±0ms 10.9±0.8ms ~1.40 strings.Cat.time_cat(0, None, None, 0.001)
10.9±2ms 10.9±0.8ms 1.00 strings.Cat.time_cat(0, None, None, 0.15)
78.1±8ms 93.8±8ms ~1.20 strings.Cat.time_cat(3, ',', '-', 0.0)
+ 62.5±8ms 109±0ms 1.75 strings.Cat.time_cat(3, ',', '-', 0.001)
+ 78.1±8ms 125±8ms 1.60 strings.Cat.time_cat(3, ',', '-', 0.15)
+ 46.9±8ms 93.8±0ms 2.00 strings.Cat.time_cat(3, ',', None, 0.0)
62.5±8ms 109±0ms ~1.75 strings.Cat.time_cat(3, ',', None, 0.001)
+ 46.9±8ms 102±8ms 2.17 strings.Cat.time_cat(3, ',', None, 0.15)
62.5±0ms 78.1±6ms ~1.25 strings.Cat.time_cat(3, None, '-', 0.0)
+ 46.9±8ms 93.8±0ms 2.00 strings.Cat.time_cat(3, None, '-', 0.001)
+ 62.5±8ms 125±6ms 2.00 strings.Cat.time_cat(3, None, '-', 0.15)
+ 62.5±8ms 85.9±8ms 1.38 strings.Cat.time_cat(3, None, None, 0.0)
46.9±6ms 93.8±0ms ~2.00 strings.Cat.time_cat(3, None, None, 0.001)
46.9±0ms 93.8±0ms ~2.00 strings.Cat.time_cat(3, None, None, 0.15)
So especially when others is not None (all the pd.concat and dealing with DataFrames) we lose perf.
As a comparison, here's the ASV before the idiomatic changes @WillAyd requested:
All benchmarks:
before after ratio
[b28cf5aa] [0d3c6d21]
<master> <cln_str_cat~2>
9.38±0ms 9.38±0ms 1.00 strings.Cat.time_cat(0, ',', '-', 0.0)
9.38±0ms 10.9±0ms ~1.17 strings.Cat.time_cat(0, ',', '-', 0.001)
10.9±0ms 12.5±0ms ~1.14 strings.Cat.time_cat(0, ',', '-', 0.15)
9.38±0.6ms 9.38±0ms 1.00 strings.Cat.time_cat(0, ',', None, 0.0)
14.1±0.8ms 10.9±0ms ~0.78 strings.Cat.time_cat(0, ',', None, 0.001)
14.1±0.6ms 11.7±0.8ms ~0.83 strings.Cat.time_cat(0, ',', None, 0.15)
9.38±0ms 9.38±0ms 1.00 strings.Cat.time_cat(0, None, '-', 0.0)
9.38±0.6ms 10.9±0.6ms ~1.17 strings.Cat.time_cat(0, None, '-', 0.001)
10.9±0ms 12.5±0ms ~1.14 strings.Cat.time_cat(0, None, '-', 0.15)
10.9±0ms 7.81±2ms ~0.71 strings.Cat.time_cat(0, None, None, 0.0)
9.38±0ms 10.9±0.8ms ~1.17 strings.Cat.time_cat(0, None, None, 0.001)
10.9±0.8ms 11.7±0.8ms 1.07 strings.Cat.time_cat(0, None, None, 0.15)
78.1±0ms 78.1±8ms 1.00 strings.Cat.time_cat(3, ',', '-', 0.0)
70.3±8ms 78.1±0ms ~1.11 strings.Cat.time_cat(3, ',', '-', 0.001)
93.8±8ms 93.8±0ms 1.00 strings.Cat.time_cat(3, ',', '-', 0.15)
46.9±6ms 78.1±0ms ~1.67 strings.Cat.time_cat(3, ',', None, 0.0)
46.9±8ms 78.1±0ms ~1.67 strings.Cat.time_cat(3, ',', None, 0.001)
46.9±6ms 62.5±8ms ~1.33 strings.Cat.time_cat(3, ',', None, 0.15)
54.7±8ms 54.7±8ms 1.00 strings.Cat.time_cat(3, None, '-', 0.0)
46.9±8ms 62.5±8ms ~1.33 strings.Cat.time_cat(3, None, '-', 0.001)
62.5±0ms 78.1±10ms ~1.25 strings.Cat.time_cat(3, None, '-', 0.15)
62.5±6ms 46.9±8ms ~0.75 strings.Cat.time_cat(3, None, None, 0.0)
46.9±6ms 62.5±0ms ~1.33 strings.Cat.time_cat(3, None, None, 0.001)
46.9±0ms 62.5±6ms ~1.33 strings.Cat.time_cat(3, None, None, 0.15)
This isn't great, but not too bad IMO. Obviously it costs us to uselessly add in sep='' just to catch TypeErrors that should already be caught in the .str-accessor. I have something in mind there as well.
The direct comparison:
HEAD~2 vs. master HEAD vs. master HEAD vs. HEAD~2 HvH2 increase
(3, ',', '-', 0.0) 1.00 1.20 1.20 +20.00%
(3, ',', '-', 0.001) 1.11 1.75 1.58 +57.66%
(3, ',', '-', 0.15) 1.00 1.60 1.60 +60.00%
(3, ',', None, 0.0) 1.67 2.00 1.20 +19.76%
(3, ',', None, 0.001) 1.67 1.75 1.05 +4.79%
(3, ',', None, 0.15) 1.33 2.17 1.63 +63.16%
(3, None, '-', 0.0) 1.00 1.25 1.25 +25.00%
(3, None, '-', 0.001) 1.33 2.00 1.50 +50.38%
(3, None, '-', 0.15) 1.25 2.00 1.60 +60.00%
(3, None, None, 0.0) 0.75 1.38 1.84 +84.00%
(3, None, None, 0.001) 1.33 2.00 1.50 +50.38%
(3, None, None, 0.15) 1.33 2.00 1.50 +50.38%
Finally, as a general warning about the results right after the run with SOME BENCHMARKS CHANGED SIGNIFICANTLY etc.: all benchmarks with a ~ in their ratio are falsely omitted from those results. I've opened airspeed-velocity/asv#752 for that.