Merge ordered and unordered comparators

[Akirathan]

Pull Request Description

Merge ordered and unordered comparators into a single one.

Important Notes

Comparator is now required to have only compare method:

type Comparator
  comapre : T -> T -> (Ordering|Nothing)
  hash : T -> Integer

Checklist

Please include the following checklist in your PR:

• The documentation has been updated if necessary.
• All code conforms to the
  Scala,
  Java,
  and
  Rust
  style guides.
• All code has been tested:
  • Unit tests have been written where possible.
  • If GUI codebase was changed: Enso GUI was tested when built using BOTH
    ./run ide build and ./run ide watch.

[Akirathan]

Benchmarks improved - sieve.enso 3x faster and EqualsBenchmarks are roughly 2x faster

[radeusgd]

minor nitpick but I'm a bit surprised we do the case-of check instead of just delegating to Default_Comparator.compare which could be the place that will then call Comparable.equals_builtin self that?

[Akirathan]

Default_Comparator.compare x y has to handle three cases - x==y, x<y or x>y - which is implemented with three nested case expressions. If x and y are neither, i.e., are not equal, all of these three cases have to be evaluated. So this is, once again, because of the performance - it is just a shortcut.

[radeusgd]

OK.

I'm really surprised we have these 3 builtins. IMO one builtin compare x y returning -1, 0, +1 or null could work just as well or maybe even better.

Like, if you are comparing two Text values that have 1M characters of length (an exaggerated example but well...), assume the difference is at second to last character. The equals check will have to iterate the whole string and find the diff at the end, to determine they are not equal. But are they greater or smaller? Now we have to iterate the whole string once again...

Instead with compare approach we'd do this at one time.
I think even for integers/floats the JVM should be able to boil down a == + < into a single three-way compare instruction (although [citation needed]).

[Akirathan]

We don't have 3 builtins for comparisons, we have only 2 - EqualsNode (Comparable.equals_builtin) and LessThanNode (Comparable.less_than_builtin). Merging their functionality into one is inappropriate because many types that can be compared with ==, cannot be compared with < - Just compare the number of specializations we have in EqualsNode and compare it to the number of specializations in LessThanNode. Moreover, LessThanNode now has a different return value, as it can return Nothing for values that are incomparable.

The example with a string with 1M characters is, indeed, exaggerated. Besides, merging EqualsNode and LessThanNode in one single node might bring us some performance improvement. But comparing it to other low hanging fruits, like problems with nested case expressions, it would be negligible.

[radeusgd]

Looks good.

I will appreciate updating the docs along the lines of what was suggested. Especially the incomparable/partial-ordering concept is a bit subtle, let's try to explain it carefully. Remembering that we used to have ordered/unordered comparators where all elements behaved in one way or another. Now we have partial-orderings by default so we can have mixed behaviour, depending on what pair of values of a given type is compared.