Bound nrepl-completed-requests with a FIFO cap#3898
Merged
Conversation
`nrepl--mark-id-completed' moved request handlers from `nrepl-pending-requests' to `nrepl-completed-requests' with no upper bound on the latter. Each entry retains its handler closure (often hundreds of bytes once you count its lexical environment), so a long- running session accumulated thousands of stale handlers in memory. The completed table exists so late messages -- responses arriving after their request's `done' status -- can still find a callback to dispatch them. In practice that window is sub-second. Add a FIFO cap so the table is bounded to the recently-completed entries: - New defcustom `nrepl-completed-requests-max-size' (default 1000). - New buffer-local `nrepl--completed-requests-order' tracking insertion order. - `nrepl--mark-id-completed' enqueues each id and, when the cap is exceeded, dequeues + removes the oldest. 1000 is enough to cover any plausible late-message window: requests complete in milliseconds, so this caps about a second of activity. Setting the size to 0 bypasses the eviction, restoring the previous unbounded behavior for users who have a reason to keep everything. Three Buttercup specs cover the cap, FIFO eviction, and the 0-disables case.
bbatsov
force-pushed
the
bound-completed-requests
branch
from
a1266f4 to
4b5711f
Compare
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
1 participant
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.
Found during a wider audit of request handling. The longest-standing of the four issues this audit surfaced.
nrepl--mark-id-completedmoves request handlers fromnrepl-pending-requeststonrepl-completed-requestswith no upper bound on the latter. The function carries a;; FIXME: This should go away eventually when we get rid of pending-request hash tablecomment that's been sitting there for years.Each completed entry retains its handler closure (often hundreds of bytes once you count its lexical environment), so a long-running session accumulated thousands of stale handlers in memory.
The completed table exists for a real reason: late messages -- responses arriving after their request's
donestatus -- still need a callback to dispatch them. In practice that window is sub-second.This PR adds a FIFO cap:
nrepl-completed-requests-max-size, default1000.nrepl--completed-requests-ordertracking insertion order.nrepl--mark-id-completedenqueues each id; when the cap is exceeded, dequeues the oldest and removes it from the hash.1000 is enough to cover any plausible late-message window: requests complete in milliseconds, so this caps roughly a second of activity. Setting the size to 0 bypasses eviction, restoring the previous unbounded behavior for users who want it.
Three Buttercup specs cover: the cap is honored, FIFO eviction order is correct, and
0disables eviction.