Skip to content

Conversation

@Andrei1998
Copy link
Contributor

@Andrei1998 Andrei1998 commented Sep 12, 2025

This PR adds PriorityQueue support to the core library, implementing two variants:

  1. Binary heap-based implementation using List (under src/PriorityQueue.mo).
  2. Wrapper over Set (under bench/utils/PriorityQueueSet.mo and intended for benchmarking/testing purposes only).

Resolves #391.

Tests

The PR contains comprehensive unit tests, including cross-testing the two implementations against each other to ensure correctness, as well as more complex test scenarios such as long randomized sequences of operations and exhaustive testing of all possible small sequences to catch edge cases.

Benchmarks

The PR contains extensive benchmarks comparing the performance of both implementations (on Nat values). See table below.

Note: an earlier commit in this PR considered simpler but less efficient ways of implementing push and pop for the heap-based implementation. These were removed in favor of more efficient implementations based on the gap technique to avoid needless copies. Concretely, commit 664d092 distinguished between:

  • push (removed) and pushBetter (now renamed push).
  • pop (removed) and popBetter (now renamed pop).

The table below is for the state of the code at this commit.


Instructions

A) PriorityQueue A2) PriorityQueue better push A3) PriorityQueue better push, better pop B) PriorityQueueSet
1.) 100000 operations (push:pop = 1:1) 1_078_836_395 871_391_356 598_903_656 522_729_799
2.) 100000 operations (push:pop = 2:1) 1_365_419_609 1_081_007_251 744_336_786 809_693_353
3.) 100000 operations (push:pop = 10:1) 598_673_615 461_615_949 359_307_559 873_181_417
4.) 100000 operations (only push) 271_131_449 193_823_567 193_823_107 886_825_181
5.) 50000 pushes, then 50000 pops 1_347_356_950 1_308_569_002 778_032_800 961_777_046
6.) 50000 pushes, then 25000 "pop;push"es 902_261_756 813_912_597 530_870_481 922_137_746

Heap (@alexandru-uta noted that these seem broken at the moment, likely due to independent reasons):

A) PriorityQueue A2) PriorityQueue better push A3) PriorityQueue better push, better pop B) PriorityQueueSet
1.) 100000 operations (push:pop = 1:1) 308 B 272 B 272 B 272 B
2.) 100000 operations (push:pop = 2:1) 308 B 308 B 272 B 272 B
3.) 100000 operations (push:pop = 10:1) 272 B 272 B 272 B 272 B
4.) 100000 operations (only push) 272 B 272 B 272 B 272 B
5.) 50000 pushes, then 50000 pops 308 B 308 B 272 B 272 B
6.) 50000 pushes, then 25000 "pop;push"es 272 B 272 B 272 B 272 B

Garbage Collection

A) PriorityQueue A2) PriorityQueue better push A3) PriorityQueue better push, better pop B) PriorityQueueSet
1.) 100000 operations (push:pop = 1:1) 10.23 MiB 15.03 MiB 15.03 MiB 17.43 MiB
2.) 100000 operations (push:pop = 2:1) 12.91 MiB 19.73 MiB 19.73 MiB 19.32 MiB
3.) 100000 operations (push:pop = 10:1) 4.22 MiB 8.67 MiB 8.67 MiB 12.64 MiB
4.) 100000 operations (only push) 406.75 KiB 3.87 MiB 3.87 MiB 9.96 MiB
5.) 50000 pushes, then 50000 pops 20.29 MiB 22.03 MiB 22.03 MiB 26.2 MiB
6.) 50000 pushes, then 25000 "pop;push"es 11.05 MiB 14.22 MiB 14.22 MiB 18.44 MiB

@Andrei1998 Andrei1998 requested a review from a team as a code owner September 12, 2025 19:13
Copy link
Collaborator

@rvanasa rvanasa left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! Added a few comments.

@rvanasa
Copy link
Collaborator

rvanasa commented Sep 18, 2025

Should be ready to merge after updating the *Better functions. Thanks again!

@Andrei1998 Andrei1998 changed the title feat: Add PriorityQueue with heap and Set wrapper implementations feat: Add PriorityQueue Sep 30, 2025
@rvanasa rvanasa merged commit 95dcf60 into dfinity:main Sep 30, 2025
13 of 15 checks passed
P4P5T123 pushed a commit that referenced this pull request Oct 9, 2025
* Partial implementation.

* Fix bug and add peek.

* Started with tests, and more functions.

* Towards more tests, adding SetPriorityQueue.

* Completing first implementation, with tests.

* Add comments to PriorityQueue.mo

* Move set implementation and add benchmark.

* Better holes.

* Extra benchmark.

* More tests.

* .

* Comments.

* Optimization.

* Comments for SetWrapper.

* Move SetWrapper.

* Run format.

* .

* npm run validate

* Fix imports in docstrings.

* Add Changelog entry.

* Fix Changelog entry.

* Remove inefficient push and pop operations (and benchmarks for them).

* Move PriorityQueueSet.mo

* Fix validation.

* Update Changelog.
christoph-dfinity added a commit that referenced this pull request Oct 13, 2025
* chore: updates matchers package to mops release (#394)

* makes comparison arguments to Map functions implicit

* use implicits in tests for Map

* make comparison arguments to Set functions implicit

* update Set tests to use implicits

* remove local bindings, now that we can disambiguate Nat and Int

* make equal arguments implicit

* rename internal Map and Set size to size_ to avoid clash with contextual dot

* Implicit pattern across entire `core` package (#398)

* First pass

* Another pass

* Fix stragglers

* Fix more stragglers

* Apply suggestions from code review

Co-authored-by: Christoph <christoph.hegemann@dfinity.org>
Co-authored-by: Claudio Russo <claudio@dfinity.org>

* Apply suggestions from code review

Co-authored-by: Claudio Russo <claudio@dfinity.org>

* Various fixes

* Apply suggestions from code review

Co-authored-by: Claudio Russo <claudio@dfinity.org>

* Fix

* Update API lockfile

---------

Co-authored-by: Christoph <christoph.hegemann@dfinity.org>
Co-authored-by: Claudio Russo <claudio@dfinity.org>

* rename .size on Queue

* add TODO comment

* I was using a stale compiler

* updates the toolchain to a pre-release

* updated mops to pull down draft branch and adjust tests (#399)

* feat: Add PriorityQueue (#392)

* Partial implementation.

* Fix bug and add peek.

* Started with tests, and more functions.

* Towards more tests, adding SetPriorityQueue.

* Completing first implementation, with tests.

* Add comments to PriorityQueue.mo

* Move set implementation and add benchmark.

* Better holes.

* Extra benchmark.

* More tests.

* .

* Comments.

* Optimization.

* Comments for SetWrapper.

* Move SetWrapper.

* Run format.

* .

* npm run validate

* Fix imports in docstrings.

* Add Changelog entry.

* Fix Changelog entry.

* Remove inefficient push and pop operations (and benchmarks for them).

* Move PriorityQueueSet.mo

* Fix validation.

* Update Changelog.

* Update implicits for RealTimeQueue

* Update implicits for PriorityQueue

* Update API lockfile

---------

Co-authored-by: Ryan Vandersmith <ryan.vandersmith@dfinity.org>
Co-authored-by: Claudio Russo <claudio@dfinity.org>
Co-authored-by: Andrei Constantinescu <andrei.constantinescu@dfinity.org>
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.

Feature request: PriorityQueue data structure

2 participants