Skip to content

perf(port): replace systeminformation with native TCP bind probe - #63

Open
ubadineke wants to merge 1 commit into
LimeChain:mainfrom
ubadineke:perf/port-check-bind-probe
Open

perf(port): replace systeminformation with native TCP bind probe#63
ubadineke wants to merge 1 commit into
LimeChain:mainfrom
ubadineke:perf/port-check-bind-probe

Conversation

@ubadineke

Copy link
Copy Markdown
Contributor

Summary

isPortOpen called si.networkConnections(), which shells out to lsof/netstat and scans every connection on the machine just to check one port's state. Replaced with a native bind probe (net.createServer().listen()) — same "check state, don't connect" safety property, roughly 1/700th–1/4000th the cost, and drops the systeminformation dependency, which existed for this one call.

This isn't just a during-a-session optimization — StateMonitor polls it every 1-2s the whole time the Gimlet pane is visible, not only while actively debugging.

Why not net.connect()?

A connect-based probe was actually the original implementation here, and it was deliberately removed (7fea373) — but for a narrower reason than "avoid net": it was being used to monitor whether an already-connected debugger session was still alive, which a fresh connect can't do. anza-xyz/sbpf's gdbstub accepts exactly one client and drops its listener right after, so any later connect attempt gets refused whether the real session is healthy or dead. netstat's ESTABLISHED-state check fixed that correctly.

When si.networkConnections() later replaced the plain netstat call (2488386), it was for an unrelated reason (cross-platform parsing), not because of the connect-probe bug above — but it kept the same "don't use net for port checks" habit anyway. That habit doesn't actually apply to the check this PR touches: isPortOpen only checks LISTEN state before a session starts, and a bind probe is safe for it the same way netstat/si are — it never calls .accept() on the target, just without the subprocess. Verified against a fixture mimicking the single-accept gdbstub: 0/10 probes reached its accept handler.

Benchmarks

30 iterations per case, against the actual isPortOpen in this PR vs. si.networkConnections() reconstructed from the removed implementation:

Scenario This PR (net bind-probe) Before (si.networkConnections)
Port closed 0.19ms mean (p95: 1.32ms) 86.58ms mean (p95: 95.02ms)
Port open 0.02ms mean (p95: 0.05ms) 89.21ms mean (p95: 99.51ms)
30 sequential ticks (StateMonitor-realistic) 0.6ms total 2562.7ms total — ~4267x

@@ -65,13 +65,12 @@ class PortManager {
}

async isPortOpen(port) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This method is used to wait for a fresh gdbstub listener while the current LLDB connection may still be established on the port. A bind probe can return EADDRINUSE for the established connection even when no listener exists, which will resolve the outer loop to start another debug session immediately.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Didn't consider that, I was mainly focused on the optimization aspect, didn't take note that the same check served for fresh listeners.

Thanks, I guess you can close 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