Skip to content

Always use unstable sort for simple types#14825

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
HertzDevil:perf/unstable-sort
Aug 8, 2024
Merged

Always use unstable sort for simple types#14825
straight-shoota merged 1 commit intocrystal-lang:masterfrom
HertzDevil:perf/unstable-sort

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
@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