PERF: Slow performance of to_dict (#46470)
State: closed

Improves performance of `to_dict` method for dataframes and series

For `orient=index` and `orient=list` performance has decreased but this is because, prior to this PR, we were not coercing types to native python types, we now are. (I assume we want this, but maybe not?)

```
N: 5,000,000

With objects (3 int cols, 3 float fields, 3 string fields)
dict: old: 16.27s, new: 16.50s
list: old: 1.33s, new: 6.28s
split: old: 20.40s, new: 19.77s
records: old: 27.31s, new: 19.75s
index: old: 17.20s, new: 28.44s
tight: old: 20.95s, new: 18.88s

Without objects (3 int cols, 3 float fields)
dict: old: 7.49s, new: 7.48s
list: old: 1.04s, new: 1.07s
split: old: 12.98s, new: 6.32s
records: old: 17.64s, new: 7.20s
index: old: 14.13s, new: 14.44s
tight: old: 12.87s, new: 6.62s
```

- [x] closes #46470 (Replace xxxx with the Github issue number)
- [x] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [ ] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [x] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.

Labels:
name: IO Data
description: IO issues that don't fit into a more specific label
name: Performance
description: Memory or execution speed performance
name: Stale

Comments:
@rhshadrach there are lots of failures on the builds that seem unrelated to my changes, am I missing something?
@phofl yeah very enough, but as I was writing tests i realised that a few of the other `orient`s weren't doing the coercing to the correct dtypes, so i started to fix them in this PR as my head was in the space and then that method started getting very long. But yeah take your point for sure
> Could you please avoid doing the refactoring and the changes in parallel? This makes it really hard to review

+1. @RogerThomas can you leave as one function for now and refactor in a follow up (or vice-versa)?
@rhshadrach @phofl, sure thing. Just to be sure before I start, do you want me to;

a) Keep the code that I added that handles correct dtype coercing for some orient types and just move the code back into the original method
or
b) Just add the code that speeds up the `to_dict` orient type, move this code back into the original method and leave the other orient types alone?
@RogerThomas - I think it's best to have separate PRs for performance and correctness. Best to get correctness first, then improve performance (in general).
@rhshadrach sorry not sure i follow, the ticket that this is supposed to close is purely a performance issue. Are you saying i should create a pr with the fix for the other orient types and then continue with this pr to address the slow performance of to_dict records?
@RogerThomas - yes, that's correct. If they are independent, then any order is okay. If they are not independent, then best practice is to get correct behavior and then work on performance.
Ok thanks @rhshadrach, do i need to create a separate github issue for the pr to fix the other orient types?
@RogerThomas - I prefer to, but in general, no. E.g. you can specify the PR # in the whatsnew when doing a bugfix instead of an issue #.
Ok thanks @rhshadrach, I'll try and get to both in the next few days
@rhshadrach @phofl i've removed the helper function, i've updated `tight`, `records`, `split`, `list` and `index` orient types to only use `maybe_box_native` when neccessary leading to decent performance improvements which can be seen in the table below which gives the % speed up for each orient type on a 1,000,000 row dataframe with 10 columns over an average of 5 `to_dict` calls for each orient type and # of object columns split

| #Object Cols   |   dict |   list |   split |   records |   index |   tight |
|:---------------|-------:|-------:|--------:|----------:|--------:|--------:|
| 0              |  1.367 | 29.082 |  76.557 |    61.491 |  48.133 |  45.578 |
| 1              |  0.013 | 38.321 |  66.922 |    34.831 |  43.742 |  38.486 |
| 2              |  0.175 | 35.425 |  56.186 |    31.715 |  38.07  |  30.966 |
| 3              |  0.036 | 31.251 |  47.548 |    27.564 |  20.055 |  24.693 |
| 4              | -0.176 | 28.231 |  38.317 |    26.378 |  22.535 |  20.204 |
| 5              | -0.132 | 26.137 |  32.546 |    23.783 |  18.309 |  12.24  |
| 6              |  2.231 | 23.064 |  22.454 |    19.934 |  12.076 |  10.85  |
| 7              | -1.064 | 19.406 |  16.208 |    18.046 |  10.371 |   6.304 |
| 8              | -1.623 | 17.361 |  11.56  |    16.953 |  -2.514 |   4.505 |
| 9              | -0.752 | 13.337 |   5.26  |    12.818 |  -0.294 |  -1.968 |
| 10             | -0.378 |  1.603 |  -0.339 |    -0.871 |   3.154 |  -1.243 |
| mean           | -0.028 | 23.929 |  33.929 |    24.786 |  19.421 |  17.329 |
| min            | -1.623 |  1.603 |  -0.339 |    -0.871 |  -2.514 |  -1.968 |
| max            |  2.231 | 38.321 |  76.557 |    61.491 |  48.133 |  45.578 |

How to interpret this table:
For example, we see a 35.425 % speed up for a 1 million row, 10 column dataframe averaged over 5 ``to_dict(orient="list")`` runs where 2 of the columns are object types and the other 8 are float columns.
@rhshadrach would it be possible to set up a call or some pair programming session for the asv stuff? I have a few questions and I think the delay of correspondance is going to take a while to resolve this
@RogerThomas - yes, we can do that, but I won't be available until next week. In the meantime, perhaps you'd be able to ask on our gitter: https://gitter.im/pandas-dev/community. This is a good format for things that don't belong in issues/PRs/mailing list, in particular troubleshooting setup or issues.
This pull request is stale because it has been open for thirty days with no activity. Please [update](https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#updating-your-pull-request) and respond to this comment if you're still interested in working on this.
@RogerThomas - are you interested in continuing this? I think it's all set (edit: one request on type-hints above and now below), just need to merge main. If not, I'd be happy to push it over the finish line.

> Can you also run asvs for this. From within asv_bench:
> 
> asv continuous -f 1.1 upstream/main HEAD -b ^frame_methods.ToDict

After merging main, I ran asvs on this branch:

```
      before           after         ratio
     [5e3d0ed0]       [3a463a97]
     <perf-gh-46470-slow-performance-of-to-dict^2>       <perf-gh-46470-slow-performance-of-to-dict>
+     3.05±0.02μs       69.3±0.1μs    22.73  frame_methods.ToDict.time_to_dict_ints('series')
+      3.06±0.1μs       69.1±0.8μs    22.60  frame_methods.ToDict.time_to_dict_datetimelike('series')
-      36.9±0.2ms       27.7±0.1ms     0.75  frame_methods.ToDict.time_to_dict_datetimelike('index')
-      15.6±0.5ms       11.3±0.5ms     0.73  frame_methods.ToDict.time_to_dict_ints('index')
-        33.4±1ms       22.5±0.1ms     0.67  frame_methods.ToDict.time_to_dict_datetimelike('dict')
-      28.8±0.3ms      19.2±0.08ms     0.67  frame_methods.ToDict.time_to_dict_datetimelike('list')
-      31.7±0.8ms       20.5±0.1ms     0.65  frame_methods.ToDict.time_to_dict_datetimelike('split')
-     10.4±0.08ms       6.52±0.1ms     0.63  frame_methods.ToDict.time_to_dict_ints('dict')
-      38.2±0.9ms       23.1±0.2ms     0.61  frame_methods.ToDict.time_to_dict_datetimelike('records')
-      16.7±0.3ms       6.29±0.1ms     0.38  frame_methods.ToDict.time_to_dict_ints('records')
-      9.66±0.8ms       3.48±0.2ms     0.36  frame_methods.ToDict.time_to_dict_ints('split')
-     4.72±0.04ms         716±20μs     0.15  frame_methods.ToDict.time_to_dict_ints('list')
```

The increase in the "series" case is overhead; this op takes 3us / 60us (main / feat) regardless of the datasize.

Also, I can still help you with running the ASVs if you'd like. Please reach out to me on [gitter](https://gitter.im/pandas-dev/community) and we can set something up.
@rhshadrach sorry yes i am, been super busy recently, will get to it monday, thanks 
@rhshadrach I've made that change for returning quickly, I think I've already addressed the other change request also
@rhshadrach no mypy issues, I think everything is ready
@jreback just to be clear, you want me to move the entire contents of `to_dict` as well as `_create_data_for_split_and_tight_to_dict` from `pandas/core/frame.py` to a new file `pandas/io/conversion.py`?

Also, as I said [here](https://github.com/pandas-dev/pandas/pull/46487/files#diff-a5257444a1b322d619680fc77361cc6ea11ef36b363b4bb2289fdef0f41feb70), the tests I've added in this PR in conjuction with the tests I added in another PR should be more than sufficient. Also, since this is a performance refactor and not a functionality change, surely we can/should have confidence in the existing tests?
@jreback would you have time to discuss the above?
@jreback @rhshadrach any chance we could pick this up, the project I'm working on could really do with the optimisations
@RogerThomas - certainly; can you resolve conflict and will look.
@rhshadrach done
ASVs look great!

```
       before           after         ratio
     [a7da45d8]       [aa9863d0]
-      28.2±0.4ms       20.8±0.5ms     0.74  frame_methods.ToDict.time_to_dict_datetimelike('index')
-     20.0±0.03ms      14.5±0.03ms     0.72  frame_methods.ToDict.time_to_dict_datetimelike('list')
-      23.8±0.6ms      17.1±0.09ms     0.72  frame_methods.ToDict.time_to_dict_datetimelike('dict')
-     11.8±0.06ms       8.24±0.2ms     0.70  frame_methods.ToDict.time_to_dict_ints('index')
-      23.1±0.7ms       15.0±0.4ms     0.65  frame_methods.ToDict.time_to_dict_datetimelike('split')
-      28.5±0.5ms       17.5±0.3ms     0.61  frame_methods.ToDict.time_to_dict_datetimelike('records')
-     7.98±0.01ms      4.37±0.03ms     0.55  frame_methods.ToDict.time_to_dict_ints('dict')
-      7.05±0.1ms      2.55±0.04ms     0.36  frame_methods.ToDict.time_to_dict_ints('split')
-      12.9±0.1ms       4.51±0.1ms     0.35  frame_methods.ToDict.time_to_dict_ints('records')
-     3.74±0.04ms        552±0.9μs     0.15  frame_methods.ToDict.time_to_dict_ints('list')
```
Thanks @rhshadrach!! Just to be clear then, is the last blocker moving the entire to_dict method to pandas.io?
moving to pandas.in is a follow up 

are there sufficient tests here?
> are there sufficient tests here?

As far as I can tell, yes. I commented on this in https://github.com/pandas-dev/pandas/pull/46487#pullrequestreview-1169503371. Are you looking for something more specific?
@jreback @rhshadrach the tests I added [here](https://github.com/pandas-dev/pandas/pull/46752/files), I believe, give us decent coverage across a range of dtypes
@jreback - friendly ping.
@rhshadrach anything I can do to help speed this up, I'd really like to get it in and am afraid it's going to go stale and fall by the wayside.