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

Sort the best traces as we go using insertion sort instead of sorting the whole array each time #1481

Merged
merged 9 commits into from
Aug 23, 2024

Conversation

romac
Copy link
Member

@romac romac commented Aug 15, 2024

See this thread for background information: #1465 (comment)

This might be completely overkill if n-traces is in practice less than a few thousands, but it was fun to write :) No worries if you'd rather not merge it because of that.


  • Tests added for any new code
  • Documentation added for any new functionality
  • Entries added to the respective CHANGELOG.md for any new functionality
  • Feature table on README.md updated for any listed functionality

@romac
Copy link
Member Author

romac commented Aug 15, 2024

Here is a benchmark to compare the two approaches.

It looks like even with only 1k elements and a super cheap comparison function on integers this makes quite the difference:

basic-example

@romac
Copy link
Member Author

romac commented Aug 19, 2024

@bugarela Let me know if you want to move forward with this, and I will add a changelog entry. Otherwise, no problem :)

@bugarela
Copy link
Collaborator

@bugarela Let me know if you want to move forward with this, and I will add a changelog entry. Otherwise, no problem :)

I absolutely do! I haven't reviewed it since it is marked as a draft, but I can do it. Thank you for doing this!

@romac romac marked this pull request as ready for review August 19, 2024 14:01
@romac romac requested a review from bugarela August 19, 2024 14:01
@romac
Copy link
Member Author

romac commented Aug 19, 2024

Ok great, I'll let you take care of the changelog then :)

Copy link
Collaborator

@bugarela bugarela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you so much to taking care of this in such organization ✨

quint/src/util.ts Outdated Show resolved Hide resolved
romac and others added 2 commits August 19, 2024 16:31
@romac romac enabled auto-merge August 22, 2024 12:28
@romac romac disabled auto-merge August 23, 2024 08:21
@romac romac enabled auto-merge August 23, 2024 08:21
@romac romac merged commit e287c4b into main Aug 23, 2024
14 checks passed
@romac romac deleted the romac/trace-insertion-sort branch August 23, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants