chore: replace number-allocator with inline Set-based implementation#2041
chore: replace number-allocator with inline Set-based implementation#2041roli-lpci wants to merge 1 commit intomqttjs:mainfrom
Conversation
Replace the `number-allocator` package (which depends on `js-sdsl` and `debug`) with an 80-line inline implementation using a Set and a low-water-mark cursor. This addresses item 1.1 of the v6 dependency cleanup roadmap (mqttjs#2038). - Removes 3 transitive dependencies (number-allocator, js-sdsl, debug) - All existing tests pass (123 integration + 10 unit) - 19 new dedicated unit tests for the inline NumberAllocator - 1.6x faster in realistic MQTT workloads (70ms vs 113ms for 1M cycles) - Browser-compatible (Set is ES2015) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2041 +/- ##
==========================================
+ Coverage 83.41% 83.68% +0.26%
==========================================
Files 25 26 +1
Lines 1586 1612 +26
Branches 367 372 +5
==========================================
+ Hits 1323 1349 +26
Misses 230 230
Partials 33 33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR removes the external number-allocator dependency by introducing an internal NumberAllocator implementation and updating internal call sites to use it, supporting the v6 dependency cleanup roadmap (#2038).
Changes:
- Added
src/lib/number-allocator.ts(Set + low-water-mark allocator) and new unit tests. - Updated internal imports to reference the new inline allocator.
- Removed
number-allocatorfrompackage.jsonand regeneratedpackage-lock.json.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/number-allocator.ts |
New inline allocator implementation to replace the number-allocator package. |
src/lib/unique-message-id-provider.ts |
Switched to local allocator import; removed unnecessary boolean cast. |
src/lib/topic-alias-send.ts |
Switched to local allocator import. |
test/node/number-allocator.ts |
Added dedicated unit tests for the new allocator behavior. |
package.json |
Removed number-allocator dependency. |
package-lock.json |
Lockfile updated to reflect dependency removal (plus additional upgrades). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (num < this.min || num > this.max || this.used.has(num)) { | ||
| return false | ||
| } | ||
| this.used.add(num) |
There was a problem hiding this comment.
use() does not update lowWaterMark, so the cursor can point at a known-occupied value after calling use(lowWaterMark) (or after marking many values via use()), causing alloc()/firstVacant() to rescan already-used IDs repeatedly. Consider advancing lowWaterMark when num <= lowWaterMark (e.g., increment until a vacant slot) to maintain the intended invariant and keep the sequential-allocation optimization effective.
| this.used.add(num) | |
| this.used.add(num) | |
| // Maintain the invariant that lowWaterMark is the lowest number that might be vacant. | |
| // If we just used a number at or below lowWaterMark, advance lowWaterMark past any | |
| // consecutively used numbers. | |
| if (num <= this.lowWaterMark) { | |
| while (this.lowWaterMark <= this.max && this.used.has(this.lowWaterMark)) { | |
| this.lowWaterMark++ | |
| } | |
| } |
There was a problem hiding this comment.
I think this is a legit change
Summary
Replaces the
number-allocatorpackage (which depends onjs-sdslanddebug) with an 80-line inline implementation using aSetand a low-water-mark cursor.This addresses item 1.1 of the v6 dependency cleanup roadmap (#2038).
number-allocator,js-sdsl,debug)NumberAllocatorSetis ES2015)Changes
src/lib/number-allocator.tssrc/lib/unique-message-id-provider.tsas booleancastsrc/lib/topic-alias-send.tspackage.jsonnumber-allocatordependencypackage-lock.jsontest/node/number-allocator.tsImplementation
The inline
NumberAllocatortracks used numbers in aSet<number>and maintains alowWaterMarkcursor that avoids full scans in the common sequential-allocation case. For MQTT's 16-bit ID range (1–65535), this is more than sufficient.API is identical to the original:
alloc(),use(),free(),clear(),firstVacant().Test plan