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

Merge ordered and unordered comparators #5818

Closed
Akirathan opened this issue Mar 6, 2023 · 5 comments · Fixed by #5845
Closed

Merge ordered and unordered comparators #5818

Akirathan opened this issue Mar 6, 2023 · 5 comments · Fixed by #5845
Assignees
Labels
--breaking Important: a change that will break a public API or user-facing behaviour --low-performance -compiler

Comments

@Akirathan
Copy link
Member

Akirathan commented Mar 6, 2023

Merge ordered and unordered comparators into a single comparator with signature:

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

Since #5778, it is no longer necessary to differentiate between ordered and unordered comparators.

Related discussion:

This change should also improve the performance.

Follow-up of #5771.

@Akirathan Akirathan added --breaking Important: a change that will break a public API or user-facing behaviour -compiler --low-performance labels Mar 6, 2023
@Akirathan Akirathan self-assigned this Mar 6, 2023
@github-project-automation github-project-automation bot moved this to ❓New in Issues Board Mar 6, 2023
@Akirathan Akirathan moved this from ❓New to 📤 Backlog in Issues Board Mar 6, 2023
@JaroslavTulach JaroslavTulach added this to the Beta Release milestone Mar 6, 2023
@Akirathan Akirathan moved this from 📤 Backlog to 🔧 Implementation in Issues Board Mar 7, 2023
@Akirathan Akirathan linked a pull request Mar 7, 2023 that will close this issue
4 tasks
@enso-bot
Copy link

enso-bot bot commented Mar 7, 2023

Pavel Marek reports a new STANDUP for today (2023-03-07):

Progress: Merging ordered and unordered comparators into one. This simplifies the API a lot. Seems like a straightforward change. Locally got tests working. Seems that I have a performance regression, investigating it.. It should be finished by 2023-03-09.

@enso-bot
Copy link

enso-bot bot commented Mar 10, 2023

Pavel Marek reports a new STANDUP for yesterday (2023-03-09):

Progress: Discovered that Meta.is_same_object is very slow and analyzing it. It should be finished by 2023-03-09.

@enso-bot
Copy link

enso-bot bot commented Mar 10, 2023

Pavel Marek reports a new 🔴 DELAY for today (2023-03-10):

Summary: There is 4 days delay in implementation of the Merge ordered and unordered comparators (#5818) task.
It will cause 4 days delay for the delivery of this weekly plan.

Delay Cause: Waiting for the last review. Also, CI is overloaded.

@enso-bot
Copy link

enso-bot bot commented Mar 10, 2023

Pavel Marek reports a new STANDUP for today (2023-03-10):

Progress: Fixed remaining tests. PR is ready to be merged. Benchmarks are still better than before. Written some analysis to #5709 (comment). Integrating review comments. It should be finished by 2023-03-13.

@mergify mergify bot closed this as completed in #5845 Mar 11, 2023
@github-project-automation github-project-automation bot moved this from 🔧 Implementation to 🟢 Accepted in Issues Board Mar 11, 2023
@enso-bot
Copy link

enso-bot bot commented Mar 14, 2023

Pavel Marek reports a new STANDUP for the provided date (2023-03-08):

Progress: Benchmarks on sieve.enso 3x faster, on EqualsBenchmarks 2x faster. Ready to merge, waiting for some reviews. It should be finished by 2023-03-13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--breaking Important: a change that will break a public API or user-facing behaviour --low-performance -compiler
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants