feat(collections): add IndexedPriorityQueue — addressable heap with O(log n) decrease-key / remove - #283
Merged
Conversation
…(log n) decrease-key / remove
Adds `IndexedPriorityQueue<TElement, TPriority, THasher>`, an addressable
binary min-heap that keeps an element->heap-slot index (a dogfooded
`CelerityDictionary`) so it can, unlike the BCL `PriorityQueue<,>`, change a
queued element's priority (Update / decrease-key / increase-key) and remove an
arbitrary element in O(log n), and answer Contains / TryGetPriority in O(1).
The documented BCL-beating workload is the priority-relaxation loop of
Dijkstra / Prim / A* and reschedulable event queues: the addressable heap holds
O(distinct elements) where the BCL's only substitute — lazy deletion — grows
the heap by one entry per update. Completes the graph-algorithm primitives with
the recently-shipped DisjointSet (union-find). Min-heap by default; a custom
IComparer<TPriority> gives a max-heap. Each element is a key (appears once).
Full parity rollout in one PR:
- collection: src/Celerity/Collections/IndexedPriorityQueue.cs
- dedicated tests: IndexedPriorityQueueTests, ...EnumerationTests,
...DifferentialTests (seeded min/max-heap differential vs a Dictionary oracle)
- benchmark: IndexedPriorityQueueBenchmark vs PriorityQueue<,>, registered in
Program.cs CoreBenchmarks
- dashboard: web/index.html ship card + COLLECTIONS arrays in
web/dev/bench/{index,detail}.html (PriorityQueue added to BCL_TYPES)
- docs: docs/api/collections.md section + README (list, details, decision table)
- AOT smoke test coverage
- CHANGELOG [Unreleased]
Cross-collection shared set/dict suites do not apply (not a hash-table
set/dict, like Deque / DisjointSet / the sketches). No ROADMAP status to flip —
milestone work is done; this is a post-roadmap tier-(c) enhancement.
Closes #282
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Coverage
Files below 100% line coverage
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces IndexedPriorityQueue<TElement, TPriority, THasher>, an addressable (indexed) binary heap for Celerity.Collections that supports Update (decrease-/increase-key) and arbitrary Remove in O(log n), and complements it with tests, benchmarks, docs, and dashboard wiring.
Changes:
- Add the new
IndexedPriorityQueuecollection implementation (indexed min-heap with element→slot index). - Add comprehensive unit + enumeration + differential tests, plus an AOT smoke-test block.
- Add benchmark + dashboard registration and update README/docs/changelog to document the new type.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web/index.html | Adds the new collection to the “What ships in the box” ship card list. |
| web/dev/bench/index.html | Registers IndexedPriorityQueue in the dashboard collection list and classifies PriorityQueue as a BCL baseline type. |
| web/dev/bench/detail.html | Adds IndexedPriorityQueue to the detail-page collection list and BCL baseline classification set. |
| src/Celerity/Collections/IndexedPriorityQueue.cs | Implements the addressable heap + index map, plus version-checked struct enumeration and capacity management. |
| src/Celerity.Tests/Collections/IndexedPriorityQueueTests.cs | Core behavioral tests (enqueue/dequeue, update, remove, comparer, capacity, null/default element, Dijkstra-shaped scenario). |
| src/Celerity.Tests/Collections/IndexedPriorityQueueEnumerationTests.cs | Enumerator behavior and mutation invalidation coverage (generic/non-generic/LINQ/reset). |
| src/Celerity.Tests/Collections/IndexedPriorityQueueDifferentialTests.cs | Seeded differential test suite vs a dictionary oracle for both min-heap and max-heap comparers. |
| src/Celerity.Benchmarks/Program.cs | Registers the new benchmark in the core benchmark suite. |
| src/Celerity.Benchmarks/IndexedPriorityQueueBenchmark.cs | Adds benchmark comparing IndexedPriorityQueue to BCL PriorityQueue (enqueue + decrease-key via lazy deletion). |
| src/Celerity.AotSmokeTest/Program.cs | Adds a smoke-test block covering enqueue/peek/dequeue, update, remove, lookups, growth, and custom comparer. |
| README.md | Documents the new collection and adds a usage snippet + decision-table row. |
| docs/api/collections.md | Adds a full API docs section with mechanism notes and a runnable Dijkstra example. |
| CHANGELOG.md | Adds an Unreleased entry for IndexedPriorityQueue. |
|
|
||
| ### Added | ||
|
|
||
| - **`IndexedPriorityQueue<TElement, TPriority, THasher>`** in `Celerity.Collections` — an addressable binary min-heap that, unlike the BCL `PriorityQueue<,>`, can change a queued element's priority (`Update` / decrease-key) and remove an arbitrary element in `O(log n)`, and answer `Contains` / `TryGetPriority` in `O(1)`. It keeps the heap at `O(distinct elements)` where the BCL's only substitute — lazy deletion — grows it by one entry per update, so it wins on the priority-relaxation loop of Dijkstra / Prim / A\* and any reschedulable event queue. Each element is a key (appears once); a custom `IComparer<TPriority>` gives a max-heap. Ships with dedicated + differential tests, a benchmark vs `PriorityQueue<,>` (registered in the core suite), dashboard cards, and docs. Not thread-safe. Closes [#282](https://github.com/marius-bughiu/Celerity/issues/282). |
Comment on lines
+214
to
+224
| public bool EnqueueOrUpdate(TElement element, TPriority priority) | ||
| { | ||
| if (_index.TryGetValue(element, out int slot)) | ||
| { | ||
| UpdateAt(slot, priority); | ||
| return false; | ||
| } | ||
|
|
||
| TryEnqueue(element, priority); | ||
| return true; | ||
| } |
Comment on lines
+569
to
+574
| private void Grow() | ||
| { | ||
| int newCapacity = _elements.Length == 0 ? DefaultCapacity : _elements.Length * 2; | ||
| Resize(newCapacity); | ||
| _version++; | ||
| } |
Benchmarks5 regressions Highlights
Collections (304)
Hashers (100)
Same-runner A/B (sharded 6-way): main ( |
- EnqueueOrUpdate no longer double-probes the index: extract InsertNew() so the known-absent path inserts directly instead of re-probing via TryEnqueue. - Grow() guards against int overflow and the Array.MaxLength ceiling (a naive `* 2` wraps negative past ~1B elements); throws at the array ceiling rather than corrupting. - Condense the CHANGELOG entry to the brevity standard (CONTRIBUTING.md). Addresses Copilot review comments on #283. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+73
to
+75
| // Bumped on every structural mutation (enqueue, dequeue, update-that-moves, remove, clear, and any | ||
| // capacity change that reallocates the heap arrays) so active enumerators detect concurrent modification. | ||
| private int _version; |
- Fix the _version field comment: an update always bumps the version (its priority change is observable via GetPriority), not only "update-that-moves". - Drop the redundant _version++ in Grow(): it is only called from InsertNew(), whose own bump already covers the enqueue, so growth no longer double-counts. Addresses Copilot review comments on #283. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
CHANGELOG.md:9
- This CHANGELOG bullet is substantially longer than the contributor guidance allows ("a few sentences at most"), which is also a release-safety concern because release notes are extracted verbatim from this section. Consider tightening it to a single concise, user-facing sentence and leaving implementation/workload detail to the PR body/docs.
- **`IndexedPriorityQueue<TElement, TPriority, THasher>`** in `Celerity.Collections` — an addressable binary min-heap that, unlike the BCL `PriorityQueue<,>`, can change a queued element's priority (`Update` / decrease-key) and remove an arbitrary element in `O(log n)`, and answer `Contains` / `TryGetPriority` in `O(1)`. It wins on the priority-relaxation loop of Dijkstra / Prim / A\* and reschedulable event queues, where the BCL's only substitute — lazy deletion — grows the heap by one entry per update. Each element is a key (appears once); a custom `IComparer<TPriority>` gives a max-heap. Not thread-safe. Closes [#282](https://github.com/marius-bughiu/Celerity/issues/282).
Comment on lines
+125
to
+127
| _index = new CelerityDictionary<TElement, int, THasher>(capacity); | ||
| if (capacity > 0) | ||
| _index.EnsureCapacity(capacity); |
The constructor built the element->slot index with `new CelerityDictionary(capacity)` and then called `EnsureCapacity(capacity)`. The ctor sizes the table by NextPowerOfTwo(capacity), which is too small to hold that many entries under the load factor, so EnsureCapacity reallocated and rehashed — two allocations for a large pre-sized queue. Build it empty and size once via EnsureCapacity instead. Addresses a Copilot review comment on #283. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
CHANGELOG.md:9
- This new CHANGELOG bullet is much longer than the repo guidance (CLAUDE.md says entries should be a few user-facing sentences, since the release workflow uses the section as the GitHub Release body and overly long sections can break releases). Please condense this to a short entry.
- **`IndexedPriorityQueue<TElement, TPriority, THasher>`** in `Celerity.Collections` — an addressable binary min-heap that, unlike the BCL `PriorityQueue<,>`, can change a queued element's priority (`Update` / decrease-key) and remove an arbitrary element in `O(log n)`, and answer `Contains` / `TryGetPriority` in `O(1)`. It wins on the priority-relaxation loop of Dijkstra / Prim / A\* and reschedulable event queues, where the BCL's only substitute — lazy deletion — grows the heap by one entry per update. Each element is a key (appears once); a custom `IComparer<TPriority>` gives a max-heap. Not thread-safe. Closes [#282](https://github.com/marius-bughiu/Celerity/issues/282).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Adds
IndexedPriorityQueue<TElement, TPriority, THasher>— an addressable (indexed) binary min-heap. Unlike the BCLPriorityQueue<TElement, TPriority>, it keeps an element→heap-slot index (a dogfoodedCelerityDictionary<TElement, int, THasher>) so it can:Update/TryUpdate/EnqueueOrUpdate(decrease-key and increase-key) — inO(log n),Remove/Remove(out priority)— inO(log n),Contains/GetPriority/TryGetPriorityinO(1).Documented BCL-beating workload: the priority-relaxation loop of Dijkstra / Prim / A* and reschedulable event queues. The addressable heap stays at
O(distinct elements), whereas the BCL heap's only substitute — lazy deletion (re-enqueue + skip stale entries on the way out) — grows the heap by one entry per update and can't answer "what is X's current priority?". It pairs with the recently-shippedDisjointSet<T>(union-find / Kruskal's MST) to round out the graph-algorithm primitives the BCL omits.Min-heap by default (
Comparer<TPriority>.Default); a customIComparer<TPriority>gives a max-heap or any order. Each element is a key (appears at most once), soEnqueuethrows on a duplicate —TryEnqueue/EnqueueOrUpdatecover the may-exist cases. The out-of-banddefault(TElement)/nullelement is handled for free by the dogfooded index.Closes #282.
Parity rollout (all in this PR)
src/Celerity/Collections/IndexedPriorityQueue.cs(sealed,IReadOnlyCollection<KeyValuePair<TElement, TPriority>>, allocation-free struct enumerator,EnsureCapacity/TrimExcess, version-checked enumeration).IndexedPriorityQueueTests(47 cases: min-order, duplicate/throw contract, decrease-/increase-key, arbitrary remove, custom comparer, capacity, seeding, null element, a Dijkstra scenario),IndexedPriorityQueueEnumerationTests(heap-order enumeration, generic/non-generic, LINQ,Reset, mutation-invalidation for every op, read-doesn't-invalidate), andIndexedPriorityQueueDifferentialTests(seeded min-and max-heap differential: 40 seeds × 800 ops reconciled against aDictionaryoracle after every op, plus a monotonic full-drain check).IndexedPriorityQueueBenchmarkvsPriorityQueue<int, int>onEnqueueand the headlineDecreaseKey(in-place update vs the lazy-deletion emulation), registered inProgram.cs'sCoreBenchmarks.web/index.html;COLLECTIONSentries inweb/dev/bench/index.html(Enqueue/DecreaseKey) andweb/dev/bench/detail.html;PriorityQueueadded toBCL_TYPESso the baseline arm is classified as the BCL reference.docs/api/collections.md(mechanism, method table, choosing-it, a runnable Dijkstra example) + README (Collections list, a details block, a "Choosing a collection" decision-table row).IndexedPriorityQueueblock inCelerity.AotSmokeTest.[Unreleased]bullet, matching the current brevity standard.Facets that genuinely don't apply
AddAndTryAddTests,SetConstructorValidationTests, …) — not a hash-table set/dict; likeDeque/DisjointSet/ the sketches it has no rows in those files.done. This is a post-roadmap tier-(c) enhancement in the graph-algorithms lane (distinct from the recent standalone-collection additions).Test plan
dotnet build(net8.0 / net9.0 / net10.0) — 0 errors, 0 new warnings.dotnet testfull suite on net10.0 — 4263 passed, 0 failed (includes the new 47 dedicated + enumeration + differential cases).all checks passed) with the new coverage.main, the benchmark workflow republishesdata.jstogh-pagesand the dashboard surfaces the newIndexedPriorityQueuecard.Note: a pre-existing latent drift —
DisjointSetBenchmarkuses[Params] int ElementCount, which the dashboard's(ItemCount: N)regex does not parse, so its card renders no data. This PR usesItemCountto render correctly; the DisjointSet fix is out of scope and flagged separately.