Skip to content

Conversation

MukjepScarlet
Copy link

toArray is much faster than toTypedArray on JVM. And we don't need a typed array here.

`toArray` is much faster than `toTypedArray` on JVM. And we don't need a typed array here.
@fzhinkin
Copy link
Contributor

There's no public Collection.toArray function, so the stdlib won't compile after applying the suggested change.

Please make sure that code compiles and tests pass (you can execute the coreLibsTest Gradle task for that) before opening a PR.

Performance-related changes, in general, have to be accompanied by benchmarks showing performance difference before and after changes. For changes affecting multiple targets, performance needs to be evaluated for all these targets.

@MukjepScarlet MukjepScarlet reopened this Jun 23, 2025
@MukjepScarlet MukjepScarlet changed the title use toArray instead of toTypedArray in ArrayDeque Use toArray instead of toTypedArray in ArrayDeque Jun 23, 2025
@MukjepScarlet
Copy link
Author

@fzhinkin Hi! I have a problem on this. Because the ArrayDeque is declared in common module, so I can't implement JVM-only functions (like toArray here) in it. Is it possible to make platform-specific implementation for classes declared in common module, while it might not exist on other platforms?

@MukjepScarlet MukjepScarlet marked this pull request as draft July 11, 2025 08:04
@fzhinkin
Copy link
Contributor

@MukjepScarlet, in theory, you can introduce an expect function and actualize it differently depending on a platform. But before doing that, it worth ensuring that all these efforts would actually result in a better performance (by writing and running benchmarks).

@MukjepScarlet
Copy link
Author

Now my target of this PR becomes to create a JVM and on-JVM version of ArrayDeque (as long as this array-related needs that), this can also fix another issue of removeIf.

@MukjepScarlet MukjepScarlet changed the title Use toArray instead of toTypedArray in ArrayDeque [KT-78533] JVM specified implementation of ArrayDeque Aug 21, 2025
@MukjepScarlet
Copy link
Author

MukjepScarlet commented Aug 21, 2025

Sorry I need to clean up the fork... I will start another PR.

@MukjepScarlet
Copy link
Author

here: #5492

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants