Skip to content

Simplify JSON-RPC write path (avoid per-message buffer copy) #128

Description

@krukow

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-chjsonrpc-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

Metadata

Metadata

Assignees

No one assigned

    Labels

    post-gaDeferred to after 1.0.0 GA

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions