alamb commented on PR #13133:
URL: https://github.com/apache/datafusion/pull/13133#issuecomment-2448286389
Here is the same benchmark run against a copy of main.
```
++ critcmp main alamb_main_copy
group alamb_main_copy
main
----- ---------------
----
merge sorted f64 1.00 4.8±0.02ms ?
?/sec 1.00 4.8±0.02ms ? ?/sec
merge sorted i64 1.00 4.4±0.02ms ?
?/sec 1.00 4.4±0.04ms ? ?/sec
merge sorted mixed dictionary tuple 1.01 12.3±0.10ms ?
?/sec 1.00 12.2±0.06ms ? ?/sec
merge sorted mixed tuple 1.00 11.7±0.11ms ?
?/sec 1.00 11.6±0.04ms ? ?/sec
merge sorted utf8 dictionary 1.00 4.2±0.02ms ?
?/sec 1.00 4.2±0.02ms ? ?/sec
merge sorted utf8 dictionary tuple 1.00 6.0±0.05ms ?
?/sec 1.00 6.1±0.20ms ? ?/sec
merge sorted utf8 high cardinality 1.00 6.8±0.02ms ?
?/sec 1.00 6.8±0.06ms ? ?/sec
merge sorted utf8 low cardinality 1.00 4.3±0.02ms ?
?/sec 1.00 4.3±0.03ms ? ?/sec
merge sorted utf8 tuple 1.00 12.5±0.07ms ?
?/sec 1.00 12.5±0.09ms ? ?/sec
sort f64 1.00 3.4±0.02ms ?
?/sec 1.00 3.4±0.02ms ? ?/sec
sort i64 1.00 2.9±0.01ms ?
?/sec 1.00 2.9±0.01ms ? ?/sec
sort merge f64 1.00 4.8±0.04ms ?
?/sec 1.01 4.8±0.03ms ? ?/sec
sort merge i64 1.00 4.5±0.02ms ?
?/sec 1.00 4.5±0.02ms ? ?/sec
sort merge mixed dictionary tuple 1.00 12.2±0.08ms ?
?/sec 1.00 12.2±0.09ms ? ?/sec
sort merge mixed tuple 1.00 11.8±0.07ms ?
?/sec 1.00 11.8±0.08ms ? ?/sec
sort merge utf8 dictionary 1.00 3.9±0.02ms ?
?/sec 1.00 3.9±0.02ms ? ?/sec
sort merge utf8 dictionary tuple 1.00 5.9±0.04ms ?
?/sec 1.00 5.9±0.04ms ? ?/sec
sort merge utf8 high cardinality 1.00 7.0±0.03ms ?
?/sec 1.01 7.0±0.03ms ? ?/sec
sort merge utf8 low cardinality 1.00 4.4±0.03ms ?
?/sec 1.01 4.5±0.03ms ? ?/sec
sort merge utf8 tuple 1.00 13.0±0.09ms ?
?/sec 1.00 13.0±0.11ms ? ?/sec
sort mixed dictionary tuple 1.00 18.4±0.19ms ?
?/sec 1.00 18.3±0.15ms ? ?/sec
sort mixed tuple 1.00 15.2±0.12ms ?
?/sec 1.00 15.1±0.10ms ? ?/sec
sort partitioned f64 1.04 220.4±525.12µs ?
?/sec 1.00 211.5±418.36µs ? ?/sec
sort partitioned i64 1.00 191.4±368.75µs ?
?/sec 1.13 217.1±556.72µs ? ?/sec
sort partitioned mixed dictionary tuple 1.00 682.3±247.57µs ?
?/sec 1.05 718.9±404.40µs ? ?/sec
sort partitioned mixed tuple 1.03 563.3±220.61µs ?
?/sec 1.00 547.2±164.45µs ? ?/sec
sort partitioned utf8 dictionary 1.00 190.4±261.52µs ?
?/sec 1.10 209.2±414.25µs ? ?/sec
sort partitioned utf8 dictionary tuple 1.00 575.2±171.84µs ?
?/sec 1.08 618.7±257.01µs ? ?/sec
sort partitioned utf8 high cardinality 1.01 386.7±177.46µs ?
?/sec 1.00 383.1±137.49µs ? ?/sec
sort partitioned utf8 low cardinality 1.00 338.7±215.73µs ?
?/sec 1.13 381.9±485.53µs ? ?/sec
sort partitioned utf8 tuple 1.02 889.2±209.16µs ?
?/sec 1.00 870.1±206.92µs ? ?/sec
sort utf8 dictionary 1.00 1092.9±14.69µs ?
?/sec 1.00 1091.9±21.58µs ? ?/sec
sort utf8 dictionary tuple 1.00 11.3±0.07ms ?
?/sec 1.00 11.3±0.07ms ? ?/sec
sort utf8 high cardinality 1.00 10.1±0.06ms ?
?/sec 1.00 10.1±0.06ms ? ?/sec
sort utf8 low cardinality 1.00 7.3±0.05ms ?
?/sec 1.00 7.2±0.03ms ? ?/sec
sort utf8 tuple 1.00 16.1±0.15ms ?
?/sec 1.00 16.0±0.14ms ? ?/sec
```
This looks a bit more stable.
Given that this change is to the inner loop of SortPreservingMerge it
wouldn't surprise me if it got somewhat slower. That being said I think we
could also merge this PR and improve performance as a follow on
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
