Skip to content
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.

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

Always use unstable sort for simple types #14825

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Jul 22, 2024

When calling #sort! without a block, if two elements have the same binary representations whenever they compare equal using #<=>, then they are indistinguishable from each other and swapping is a no-op. This allows the use of unstable sorting which runs slightly faster and requires no temporary allocations, as opposed to stable sorting which allocates memory linear in the collection size.

Following this criterion, the only reference type that supports unstable sorting is:

class Foo
  include Comparable(self)

  def <=>(other : self) : Int32
    object_id <=> other.object_id
  end
end

Primitive floats do not support it, as the signed zeros compare equal but have opposite sign bits. For simplicity, unions also aren't touched; either they don't have a meaningful, non-null #<=> defined across the variant types, or they break the criterion (e.g. 1_i32 and 1_i8 have different type IDs).

#sort always delegates to #sort!. This does not affect #sort_by! since the projected element type alone doesn't guarantee the original elements themselves can be swapped in the same way.

@straight-shoota
Copy link
Member

straight-shoota commented Jul 23, 2024

I suppose Slice(UInt8) and (by extenstion) StaticArray(UInt8) should also be binary equivalent?

@straight-shoota
Copy link
Member

I'm also wondering if we could generalize this property (e.g. via a module) so we don't need to hard-code a compatibility list in Slice#sort!. Instead, types could declare binary equivalent comparison as an opt-in mechanism, which would open this feature for types from user code.

@HertzDevil
Copy link
Contributor Author

Two Slices may compare equal but differ in @read_only.

StaticArray and Tuple are probably fine if all their element types are, but it would be difficult to have those types conditionally include a module.

@straight-shoota
Copy link
Member

Argh. I wish we had readability of a slice as a type information, not a runtime property.

@straight-shoota straight-shoota added this to the 1.14.0 milestone Jul 25, 2024
@straight-shoota straight-shoota merged commit f1eabf5 into crystal-lang:master Aug 8, 2024
61 checks passed
@HertzDevil HertzDevil deleted the perf/unstable-sort branch August 13, 2024 12:52
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.

3 participants