Introduce CursorState to consolidate per-cursor memory tracking#1832
Merged
renanmagagnin merged 1 commit intoMay 15, 2026
Merged
Conversation
Consolidates per-cursor state in the memory limiter into a single CursorState struct, eliminating hot-path DashMap lookups from the backpressure controller and cursor read path. Previously, per-cursor data was split across two DashMaps in MemoryLimiter: one for reservation balances (mem_reserved_per_cursor) and one for active FUSE read ranges (active_reads). Every reserve, try_reserve, set_active_read, and clear_active_read call performed an independent DashMap lookup on the hot path. This change introduces CursorState, a shared struct holding both the reservation counter and active read state, and a CursorHandle wrapper for setting active reads. The BackpressureController and Cursor hold a direct Arc<CursorState>, so reservation and active-read operations no longer require any DashMap lookup. The DashMap is now only accessed on cursor creation, release, and in the on_pool_reserve callback (same as before, but unified into a single map). No behavioral changes. Existing unit tests adapted to the new API. Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
passaro
temporarily deployed
to
PR integration tests
GitHub Actions
Inactive
yerzhan7
added a commit
to yerzhan7/mountpoint-s3
that referenced
this pull request
May 15, 2026
Pulls in awslabs#1832 (CursorState consolidation). The conflict in mountpoint-s3-fs/src/memory/pool.rs was in the delegation block: upstream removed reserve/try_reserve/release_cursor/next_cursor_id/inner_stats/ inner_limiter in favor of a unified `create_cursor`, while this branch had added `mem_limit`/`data_buffer_budget` accessors used by `WriteHandleLimiter`. Resolved by keeping both — upstream's `create_cursor` plus our two accessors — and dropped the now-unused test-only `inner_stats`/`inner_limiter` helpers along with their TODO. Signed-off-by: Yerzhan Mazhkenov <20302932+yerzhan7@users.noreply.github.com>
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
3 participants
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.
Consolidates per-cursor state in the memory limiter into a single
CursorStatestruct, eliminating hot-path lookups from the backpressure controller and cursor read path.Previously, per-cursor data was split across two maps in MemoryLimiter: one for reservation balances and one for active FUSE read ranges. Every
reserve,try_reserve,set_active_read, andclear_active_readcall performed an independent lookup on the hot path. This change introducesCursorState, a shared struct holding both the reservation counter and active read state, and aCursorHandlewrapper for setting active reads. TheBackpressureControllerandCursordirectly hold a reference toCursorState, so reservation and active-read operations no longer require any lookup. The map is now only accessed on cursor creation, release, and in theon_pool_reservecallback (same as before, but unified into a single map).Does this change impact existing behavior?
No behavioral changes. Existing unit tests adapted to the new API.
Does this change need a changelog entry? Does it require a version change?
No.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).