ext/standard: Optimize sort() and rsort() for packed arrays - #23585
Draft
LamentXU123 wants to merge 2 commits into
Draft
ext/standard: Optimize sort() and rsort() for packed arrays#23585LamentXU123 wants to merge 2 commits into
LamentXU123 wants to merge 2 commits into
Conversation
LamentXU123
force-pushed
the
opt-sort
branch
from
September 5, 2026 18:27
c8a0784 to
f4dc3c4
Compare
LamentXU123
force-pushed
the
opt-sort
branch
from
September 6, 2026 15:22
f4dc3c4 to
e43223d
Compare
Contributor
|
Nice to see faster sorting. In PHPStan profiles we see slow sorting in https://github.com/phpstan/phpstan-src/blob/2.3.x/src/Type/TypeCombinator.php in most profiles (which is not sorting on pure integers though) |
LamentXU123
marked this pull request as draft
September 6, 2026 16:32
Member
Author
|
TBH, I don't think this is a completed PR. The idea is good, but we can't just implement this only for integers. Ideally, we can implement a sort completely supporting any zvals to be used by packed arrays. I will be working on that idea in the coming days. So this is now drafted, sorry to ping you so early. I will request reviews when I think the implementaion is done. Feel free to comment about the idea itself :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After the last trivial one-element optimization I get to be familiar with how sort works in Zend. So, now, except the array is empty or only has one element, we convert arrays to mixed storage before sorting, and convert them back after we sort them.
The reason we are doing this is because the internal sorting functions is using Buckets, so we can preserve the keys when sorting the values. Packed arrays (most cases we are sorting them instead of unpacked and mixed arrays), are sequence zvals, and don't have customized keys.
This let me thinking. Can we just implement a sort internally which completely works with zvals and not Buckets? So we don't need to convert them to Bucket and convert them back again when sorting packed arrays that don't have external keys at all?
I have a idea to implement a sort works with integer arrays entirely depending on zvals. There it is. I set a threshold to use this new internal function instead of the original one when the array to be sorted is large (>= 64 elements), packed and contains fully int. This is because sort functions with fully int arrays is easy to implement (I don't need to consider user's fallback function etc.) and since we are now not doing creating new Buckets and destroying them the benefits mainly comes from larger arrays, both in the aspect of time and memory spaces.
I ran benchmarks on a 64-elements, fully int array. Also, somehow this is also way more faster when we are sorting on a already sorted array, which is like a bonus.
Benchmark:
sort()rsort()sort()And this is the results with a 1,000,000 element array FYI.
sort()rsort()sort()