Skip to content

Add ID recycling utility#686

Open
CvvT wants to merge 2 commits intomainfrom
weiteng/idpool_wip
Open

Add ID recycling utility#686
CvvT wants to merge 2 commits intomainfrom
weiteng/idpool_wip

Conversation

@CvvT
Copy link
Contributor

@CvvT CvvT commented Feb 25, 2026

As mentioned in #684, asked agent to move the bitmap-based ID generator to LiteBox to avoid duplication.

@CvvT CvvT requested a review from sangho2 February 25, 2026 18:41
@CvvT CvvT marked this pull request as ready for review February 25, 2026 18:43
@github-actions
Copy link

🤖 SemverChecks 🤖 No breaking API changes detected

Note: this does not mean API is unchanged, or even that there are no breaking changes; simply, none of the detections triggered.

Copy link
Contributor

@sangho2 sangho2 left a comment

Choose a reason for hiding this comment

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

Looks good to me! minor comments.

fn grow(&mut self) -> Option<u32> {
let new_id = self.max_ids;
// Guard: bitmap.len() * 64 must fit in u32.
if self.bitmap.len() >= u32::MAX as usize / 64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

In the worst case, the size of this bitmap would be 512 MB. In practice, we might want to have a hybrid approach: growable with mem cap.

// Safety of truncation: the guard above ensures bitmap.len() * 64 fits in u32.
#[allow(clippy::cast_possible_truncation)]
{
self.max_ids = (self.bitmap.len() as u32) * 64;
Copy link
Contributor

Choose a reason for hiding this comment

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

self.max_ids = self.max_ids + 64?

let start = if self.hint >= cap - 1 {
0
} else {
self.hint + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: overflow?


/// Mark an ID as free so it can be reused.
///
/// IDs beyond the current capacity and already-free IDs are silently
Copy link
Contributor

Choose a reason for hiding this comment

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

sounds like a bug. debug_assert might be helpful.


impl SessionIdPool {
/// Maximum recyclable session ID tracked by the bitmap.
const MAX_RECYCLABLE_SESSION_ID: u32 = 65535;
Copy link
Contributor

Choose a reason for hiding this comment

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

With this new IdPool, 65536 might work now.

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