Generated via Copilot on behalf of @krukow
Summary
The JSON-RPC write path in src/github/copilot_sdk/protocol.clj is functionally correct but does more buffering and copying than necessary. This is a post-GA cleanup (deferred from the 1.0.0 GA Phase 3 idiomatic/state review to avoid destabilizing the release candidate) — there is no correctness or resource bug, only an opportunity to simplify and reduce allocations.
Current shape
Outbound messages flow outgoing-ch → jsonrpc-nio-writer thread (start-write-loop!) → write-message!. In write-message! (protocol.clj:95-104) each message is serialized and then copied twice into a freshly-allocated ByteBuffer:
(let [json-str (json/write-str msg)
content-bytes (.getBytes json-str StandardCharsets/UTF_8)
header-bytes (.getBytes header StandardCharsets/UTF_8)
buf (ByteBuffer/allocate (+ (alength header-bytes) (alength content-bytes)))]
(.put buf header-bytes)
(.put buf content-bytes)
(.flip buf)
(while (.hasRemaining buf) (.write channel buf)))
So every outbound message allocates a new combined buffer and copies the header and content bytes into it (on top of the getBytes allocations).
Possible improvements (post-GA, non-breaking)
- Write the header and content buffers in sequence (two
.write calls) instead of allocating a third combined buffer and copying into it, or wrap the existing byte arrays with ByteBuffer/wrap.
- Consider a reusable/thread-local buffer for the writer thread (single-threaded, daemon
jsonrpc-nio-writer), sized to the message with growth, to avoid per-message allocation on the hot path.
- Re-evaluate whether the
outgoing-ch + dedicated writer thread layering is still the simplest design now that the read/notification paths have settled, or whether it can be flattened.
Constraints
- Must preserve correct content-length framing and the existing
disconnect interrupt/join semantics for the writer thread (protocol.clj:578-590).
- Keep all blocking channel I/O off core.async go threads (per AGENTS.md rigor rules).
- Benchmark before/after — this is an optimization, so it needs evidence it actually helps and a regression test that framing is byte-identical.
Labels: post-ga
Summary
The JSON-RPC write path in
src/github/copilot_sdk/protocol.cljis functionally correct but does more buffering and copying than necessary. This is a post-GA cleanup (deferred from the 1.0.0 GA Phase 3 idiomatic/state review to avoid destabilizing the release candidate) — there is no correctness or resource bug, only an opportunity to simplify and reduce allocations.Current shape
Outbound messages flow
outgoing-ch→jsonrpc-nio-writerthread (start-write-loop!) →write-message!. Inwrite-message!(protocol.clj:95-104) each message is serialized and then copied twice into a freshly-allocatedByteBuffer:So every outbound message allocates a new combined buffer and copies the header and content bytes into it (on top of the
getBytesallocations).Possible improvements (post-GA, non-breaking)
.writecalls) instead of allocating a third combined buffer and copying into it, or wrap the existing byte arrays withByteBuffer/wrap.jsonrpc-nio-writer), sized to the message with growth, to avoid per-message allocation on the hot path.outgoing-ch+ dedicated writer thread layering is still the simplest design now that the read/notification paths have settled, or whether it can be flattened.Constraints
disconnectinterrupt/join semantics for the writer thread (protocol.clj:578-590).Labels:
post-ga