Skip to content

Add priority queue interface with a binary heap–based implementation#6873

Merged
simonvonhackewitz merged 63 commits intotokiwa-software:mainfrom
simonvonhackewitz:priority_queue
Apr 30, 2026
Merged

Add priority queue interface with a binary heap–based implementation#6873
simonvonhackewitz merged 63 commits intotokiwa-software:mainfrom
simonvonhackewitz:priority_queue

Conversation

@simonvonhackewitz
Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread modules/base/src/container/Binary_Heap_Queue.fz Outdated
Comment thread modules/base/src/container/Binary_Heap_Queue.fz Outdated
Copy link
Copy Markdown
Member

@michaellilltokiwa michaellilltokiwa left a comment

Choose a reason for hiding this comment

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

Lots of small(er) comments from me, sorry. Overall structure is pretty much what I would expect. We have a mutable queue definition with enqueue/dequeue functionality. Of to a good start!

Comment thread modules/base/src/container/Mutable_Priority_Queue.fz Outdated
Comment thread modules/base/src/container/Mutable_Priority_Queue.fz
Comment thread modules/base/src/container/Mutable_Priority_Queue.fz Outdated

# unordered elements to construct the queue from
#
initial_elements option (Sequence T)) ref : container.Mutable_Priority_Queue T is
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since this constructor is not public anyway, I would probably prefer all initialization to be done outside of the constructor. So it would have elements as an arg instead of initial_elements.
This way the constructor does not not contain any code which may make error handling more easy.

Comment thread modules/base/src/container/Binary_Heap_Queue.fz
Comment thread modules/base/src/container/Binary_Heap_Queue.fz Outdated
Comment thread modules/base/src/container/Binary_Heap_Queue.fz
Comment thread modules/base/src/container/Binary_Heap_Queue.fz Outdated
Comment thread modules/base/src/container/Binary_Heap_Queue.fz
@simonvonhackewitz simonvonhackewitz added enhancement New feature or request base changes related to base module labels Apr 15, 2026
@simonvonhackewitz simonvonhackewitz changed the title WIP: add priority queue using a binary heap internally add priority queue using a binary heap internally Apr 16, 2026
Copy link
Copy Markdown
Member

@michaellilltokiwa michaellilltokiwa left a comment

Choose a reason for hiding this comment

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

I think the histogram is a good start.
What I'm missing is - the probably most important metric - is WCET (worst case execution time). An also some heuristic how the determine if this is within a certain range.

Comment thread benchmarks/priority_queue_benchmark.fz Outdated
Comment thread benchmarks/priority_queue_benchmark.fz Outdated
Comment thread benchmarks/priority_queue_benchmark.fz Outdated
Comment thread benchmarks/priority_queue_benchmark.fz Outdated
Comment thread benchmarks/priority_queue_benchmark.fz Outdated
Copy link
Copy Markdown
Member

@fridis fridis left a comment

Choose a reason for hiding this comment

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

Good work!

I have a number of comments, but mostly some more documentation and some code that I would like to see at other places, so nothing major to commplain.

I have not checked the tests and benchmarks yet, but that should not be blocking.

Some comments can be addressed later, so do not consider them as blocking your documentation etc.

Comment thread modules/base/src/container/Binary_Heap_Queue.fz Outdated
Comment thread modules/base/src/container/Binary_Heap_Queue.fz Outdated
Comment thread modules/base/src/container/Binary_Heap_Queue.fz
Comment thread modules/base/src/container/Binary_Heap_Queue.fz
Comment thread modules/base/src/container/Binary_Heap_Queue.fz Outdated
Comment thread modules/base/src/container/Binary_Heap_Queue.fz Outdated
Comment thread modules/base/src/container/Binary_Heap_Queue.fz
Comment thread modules/base/src/container/Binary_Heap_Queue.fz Outdated
Comment thread modules/base/src/mutate/array.fz
Comment thread modules/base/src/mutate/array.fz
Copy link
Copy Markdown
Member

@fridis fridis left a comment

Choose a reason for hiding this comment

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

The test is already good in the sense that it covers diifferent and important cases.

I made a number of suggestions how this test could be more systematic. Please just pick and chose comments that you would like to include in your thesis and ignore what is too much for now.

Some design decisions (like using a compare function instead of property.orderable, adding min/max variants, etc.) result in an explosion of configurations and make it harder to test this. At a later point in time, I would like you to revisit these decisions.

Comment thread tests/priority_queue/priority_queue_test.fz
Comment thread tests/priority_queue/priority_queue_test.fz Outdated
Comment thread modules/base/src/container/Binary_Heap_Queue.fz
Comment thread modules/base/src/container/Binary_Heap_Queue.fz
Comment thread tests/priority_queue/priority_queue_test.fz
Comment thread tests/priority_queue/priority_queue_test.fz Outdated
Comment thread tests/priority_queue/priority_queue_test.fz
Comment thread tests/priority_queue/priority_queue_test.fz Outdated
Comment thread tests/priority_queue/priority_queue_test.fz
Comment thread tests/priority_queue/priority_queue_test.fz Outdated
Comment thread modules/base/src/container/Mutable_Priority_Queue.fz Outdated
Comment thread modules/base/src/container/Mutable_Priority_Queue.fz Outdated
Comment thread modules/base/src/container/Binary_Heap_Queue.fz Outdated
Comment thread modules/base/src/container/Binary_Heap_Queue.fz Outdated
Comment thread modules/base/src/container/Binary_Heap_Queue.fz
@simonvonhackewitz simonvonhackewitz marked this pull request as ready for review April 30, 2026 13:37
@simonvonhackewitz simonvonhackewitz changed the title add priority queue using a binary heap internally Add a priority queue interface and an implementation based on a binary heap Apr 30, 2026
@simonvonhackewitz simonvonhackewitz changed the title Add a priority queue interface and an implementation based on a binary heap Add priority queue interface with a binary heap–based implementation Apr 30, 2026
Copy link
Copy Markdown
Member

@michaellilltokiwa michaellilltokiwa left a comment

Choose a reason for hiding this comment

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

Good work!

@simonvonhackewitz simonvonhackewitz merged commit 0e5dbf3 into tokiwa-software:main Apr 30, 2026
5 checks passed
@simonvonhackewitz simonvonhackewitz deleted the priority_queue branch April 30, 2026 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

base changes related to base module enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants