PERF-#5778: Avoid extra materialization at range-based reshuffling

[dchigarev]

What do these changes do?

This PR removes unnecessary conversion to row partitions while reshuffling partitions.

Changes of our ASV benchmark:

MODIN_TEST_DATASET_SIZE="Big" asv continuous origin/master issue_5778 --launch-method=spawn -b TimeSortValues --no-only-changed -a repeat=5

All benchmarks:

       before           after         ratio
     [8d3db2b4]       [b50ee4e2]
     <issue_5778~1>       <issue_5778>
-       7.52±0.1s       1.15±0.04s     0.15  benchmarks.TimeSortValues.time_sort_values([1000000, 10], 1, True)
-       7.74±0.4s       1.17±0.03s     0.15  benchmarks.TimeSortValues.time_sort_values([1000000, 10], 2, True)

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.

• first commit message and PR title follow format outlined here

  NOTE: If you edit the PR title to match this format, you need to add another commit (even if it's empty) or amend your last commit for the CI job that checks the PR title to pick up the new PR title.

• passes flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
• passes black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
• signed commit with git commit -s
• Resolves Do not build row-partitions for a final function during reshuffling #5778
• tests are passing
• module layout described at docs/development/architecture.rst is up-to-date

[dchigarev]

Each partition from split_row_partitions is already a row partition at this point since we did the conversion before:

modin/modin/core/dataframe/pandas/partitioning/partition_manager.py

Lines 1584 to 1590 in cdedd71

	# Convert our list of block partitions to row partitions. We need to create full-axis
	# row partitions since we need to send the whole partition to the split step as otherwise
	# we wouldn't know how to split the block partitions that don't contain the shuffling key.
	row_partitions = [
	partition.force_materialization().list_of_block_partitions[0]
	for partition in cls.row_partitions(partitions)
	]

There's no need to do double-wrapping.

[RehanSD]

Makes sense!

[anmyachev]

Looks impressive! @RehanSD please also take a look.

[RehanSD]

LGTM! Left a couple of quick questions!

[RehanSD]

Makes sense!

[RehanSD]

Do we want to make this lazy? Since split_row_partitions is in effect the properly partitioned dataframe, we can transform to col partitions, and then add_to_apply_calls the sort instead, and defer metadata materialization till it's needed?

[RehanSD]

This could be a future PR though!

[dchigarev]

We can definitely do deferred meta-data materialization until really needed, created an issue for this #5808.

Regarding the lazy functions submission using add_to_apply_calls: I remember we had a performance regression quite ago when we switched to lazy execution. Since then we reverted the changes in #2471 and never tried to apply them again, so there is probably some careful evaluation that has to be done before making this change again, created an issue for this #5809

[dchigarev]

So, can we merge this one?

[anmyachev]

So, can we merge this one?

@dchigarev we can, but Rehan has an unanswered comment.

[dchigarev]

So, can we merge this one?

@dchigarev we can, but Rehan has an unanswered comment.

@anmyachev I've opened the required issues, can we merge the PR now?