Skip to content

Optimize sorted() for primitive-type arrays #5419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Optimize sorted() for primitive-type arrays #5419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions libraries/stdlib/common/src/generated/_Arrays.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6283,49 +6283,49 @@ public fun <T : Comparable<T>> Array<out T>.sorted(): List<T> {
* Returns a list of all elements sorted according to their natural sort order.
*/
public fun ByteArray.sorted(): List<Byte> {
return toTypedArray().apply { sort() }.asList()
return sortedArray().asList()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, asList() function was invoked on a Array<T>, now it is invoked on a corresponding primitive array.
The change is subtle, but it results in different underlying types of the resulting list. Further analysis is required to determine how this change may affect users. For example, some codebases may rely on the fact that the resulting list is effectively mutable and explicitly cast it to MutableList<T>. Changed underlying type may also affect call sites where a list produced by the sorted() call is used. This need to be investigated further.
Besides these potential issues, there is a less ephemeral one: PrimitiveArray.asList() returns a wrapper around the primitive array (for example,

kotlin/libraries/stdlib/jvm/src/generated/_ArraysJvm.kt

Line 167 in bf92d5d

public actual fun IntArray.asList(): List<Int> {
). As a result, access to individual elements requires boxing, which results in worse performance in general. It's not clear if performance gained by optimizing sorting will always offset performance regression in element-access scenarios.

As a middle ground, sorted may convert sorted primitive array into a typed array first and then invoke asList on it (sortedArray().toTypedArray().asList()). Such an implementation outperforms the existing one (by saving time on sorting itself), but returns exactly the same list type as before.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides these potential issues, there is a less ephemeral one: PrimitiveArray.asList() returns a wrapper around the primitive array ... As a result, access to individual elements requires boxing, which results in worse performance in general.

The benchmark results (https://github.com/pubiqq/benchmarks-kmp/tree/list-get-benchmarks) do not allow us to state this unequivocally, in most cases accessing a primitive from PrimitiveArray.asList() is faster than from PrimitiveArray.toTypedArray().asList().

It should also be noted that toTypedArray() boxes primitives too. Yes, it does this preemptively, but it does this for all elements, and this is not always better than using a list that converts primitives on demand.

As a middle ground, sorted may convert sorted primitive array into a typed array first and then invoke asList on it (sortedArray().toTypedArray().asList()). Such an implementation outperforms the existing one (by saving time on sorting itself), but returns exactly the same list type as before.

Such an implementation also performs two array copy operations instead of one. I'm not sure it's worth it.

}

/**
* Returns a list of all elements sorted according to their natural sort order.
*/
public fun ShortArray.sorted(): List<Short> {
return toTypedArray().apply { sort() }.asList()
return sortedArray().asList()
}

/**
* Returns a list of all elements sorted according to their natural sort order.
*/
public fun IntArray.sorted(): List<Int> {
return toTypedArray().apply { sort() }.asList()
return sortedArray().asList()
}

/**
* Returns a list of all elements sorted according to their natural sort order.
*/
public fun LongArray.sorted(): List<Long> {
return toTypedArray().apply { sort() }.asList()
return sortedArray().asList()
}

/**
* Returns a list of all elements sorted according to their natural sort order.
*/
public fun FloatArray.sorted(): List<Float> {
return toTypedArray().apply { sort() }.asList()
return sortedArray().asList()
}

/**
* Returns a list of all elements sorted according to their natural sort order.
*/
public fun DoubleArray.sorted(): List<Double> {
return toTypedArray().apply { sort() }.asList()
return sortedArray().asList()
}

/**
* Returns a list of all elements sorted according to their natural sort order.
*/
public fun CharArray.sorted(): List<Char> {
return toTypedArray().apply { sort() }.asList()
return sortedArray().asList()
}

/**
Expand Down
7 changes: 1 addition & 6 deletions libraries/tools/kotlin-stdlib-gen/src/templates/Ordering.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,17 +200,12 @@ object Ordering : TemplateGroupBase() {
return toMutableList().apply { sort() }
"""
}
body(ArraysOfPrimitives) {
"""
return toTypedArray().apply { sort() }.asList()
"""
}
body(ArraysOfUnsigned) {
"""
return copyOf().apply { sort() }.asList()
"""
}
body(ArraysOfObjects) {
body(ArraysOfPrimitives, ArraysOfObjects) {
"""
return sortedArray().asList()
"""
Expand Down