fix: reduce CPU usage from PTY URL parsing and file watchers#566
Open
EricBlanquer wants to merge 1 commit intositeboon:mainfrom
Open
fix: reduce CPU usage from PTY URL parsing and file watchers#566EricBlanquer wants to merge 1 commit intositeboon:mainfrom
EricBlanquer wants to merge 1 commit intositeboon:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughIncreased file-watcher debounce and reduced chokidar scan depth; tightened shell URL parse buffer and added a per-shell throttled URL detection flow that immediately handles auto-open cases, deduplicates/normalizes URLs, and resolves the active WebSocket from Changes
Sequence Diagram(s)sequenceDiagram
participant Shell as Shell Process
participant Detector as URL Detector
participant Timer as Throttle Timer
participant Map as ptySessionsMap
participant WS as WebSocket (client)
Shell->>Detector: output chunk
Detector->>Detector: append to urlDetectionBuffer
alt shouldAutoOpenUrlFromOutput == true
Detector->>Detector: runUrlDetection(forceAutoOpen=true)
else not auto-open
Detector->>Timer: start/coalesce urlParseTimer (2000ms)
Timer-->>Detector: timer fires
Detector->>Detector: runUrlDetection(forceAutoOpen=false)
end
Detector->>Detector: normalize & dedupe URLs (prefer longest)
Detector->>Map: lookup session by ptySessionKey
Map-->>Detector: session (may be undefined)
alt session exists & WebSocket.OPEN
Detector->>WS: send `auth_url` (autoOpen true/false)
else no open socket
Detector->>Detector: skip send
end
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/index.js`:
- Around line 1863-1889: The code currently adds only the URL string to
announcedAuthUrls so calling emitAuthUrl(url, true) after emitAuthUrl(url,
false) is skipped; change the logic so autoOpen can override a prior
non-autoOpen announcement: update announcedAuthUrls to track per-URL whether
autoOpen has been sent (e.g., map URL -> boolean) and modify emitAuthUrl to send
if the URL is new OR if autoOpen is true and the stored value is false, then set
the stored value to true after sending; reference emitAuthUrl,
announcedAuthUrls, dedupedDetectedUrls and forceAutoOpen when locating the
change.
EricBlanquer
force-pushed
the
fix/cpu-leak-watchers-url-parsing
branch
from
46fb3ac to
a81eb1d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/index.js`:
- Around line 1894-1907: The throttle timer is not cleared when
shouldAutoOpenUrlFromOutput(cleanChunk) causes runUrlDetection(true) to run
immediately, so any existing urlParseTimer can still fire later; update the
branch where runUrlDetection(true) is called to clear the existing timer
(clearTimeout(urlParseTimer); urlParseTimer = null) and reset urlParsePending as
appropriate so the pending callback won't run again after URL_PARSE_THROTTLE_MS.
Modify the logic around shouldAutoOpenUrlFromOutput, runUrlDetection,
urlParseTimer, urlParsePending (and related emitAuthUrl behavior) to ensure the
pending timer is cancelled before immediate execution.
- Throttle extractUrlsFromText() to run at most every 2s instead of on every PTY data chunk (O(n²) regex on 32KB buffer was called hundreds of times per second during active sessions) - Remove chokidar awaitWriteFinish polling (stat every 50ms on ~2000 files), reduce watch depth from 10 to 3 - Increase watcher debounce from 300ms to 2s to reduce getProjects() scan frequency - Reduce URL detection buffer from 32KB to 8KB Before: 94% CPU / 574MB after ~8h. After: ~12% CPU / 100MB stable.
EricBlanquer
force-pushed
the
fix/cpu-leak-watchers-url-parsing
branch
from
a81eb1d to
ac904c5
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/index.js (1)
1976-1993: Consider clearingurlParseTimeron WebSocket close.When the shell client disconnects,
urlParseTimermay still be pending. While the timer callback safely skips sending (due tows.readyStatecheck), clearing it explicitly would be slightly cleaner.🔧 Optional cleanup
ws.on('close', () => { console.log('🔌 Shell client disconnected'); + + if (urlParseTimer) { + clearTimeout(urlParseTimer); + urlParseTimer = null; + } if (ptySessionKey) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/index.js` around lines 1976 - 1993, On WebSocket close, clear any pending urlParseTimer to avoid leaving a dangling timer: inside the ws.on('close' ...) handler (the block that references ptySessionKey, ptySessionsMap, session.timeoutId and PTY_SESSION_TIMEOUT) check if urlParseTimer is set, call clearTimeout(urlParseTimer) and set urlParseTimer = null so the timer is explicitly cancelled when the shell client disconnects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/index.js`:
- Around line 1976-1993: On WebSocket close, clear any pending urlParseTimer to
avoid leaving a dangling timer: inside the ws.on('close' ...) handler (the block
that references ptySessionKey, ptySessionsMap, session.timeoutId and
PTY_SESSION_TIMEOUT) check if urlParseTimer is set, call
clearTimeout(urlParseTimer) and set urlParseTimer = null so the timer is
explicitly cancelled when the shell client disconnects.
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.
Summary
onDatahandler:extractUrlsFromText()(O(n²) regex on large buffer) was called on every data chunk — now throttled to once every 2s, with immediate execution preserved for auto-open triggersawaitWriteFinishpolling: wasstat()-ing every 50ms on ~2000 watched files; reducedepthfrom 10 to 3getProjects()scan frequencyRoot cause
After running for several hours,
node server/index.jsreached 94% CPU / 574MB RAM. Three compounding causes:extractUrlsFromText()ran a nested O(n²) loop + regex on a 32KB buffer on every PTY chunk (hundreds/sec during active Claude sessions)awaitWriteFinish: { pollInterval: 50 }polled ~2000 files every 50msgetProjects()(heavy directory scan) triggered every 300ms by file changesAfter fix: ~12% CPU / 100MB stable after 5 min, trending to idle.
Summary by CodeRabbit