david-mollitor-db opened a new pull request, #58893: URL: https://github.com/apache/spark/pull/58893
### What changes were proposed in this pull request? `RequestMessage.serialize` (`core/.../rpc/netty/NettyRpcEnv.scala`) is called for every outgoing Netty RPC (`send` / `ask`). It allocated its output buffer with the no-arg `new ByteBufferOutputStream()`, which starts at the JDK `ByteArrayOutputStream` default of **32 bytes** and grows by doubling (reallocate + `Arrays.copyOf`) as bytes are written. This pre-sizes that buffer to **512 bytes**: ```scala // The RpcAddress preamble plus a typical control message exceeds the 32-byte default, so // start at 512 to hold the common messages without repeated grow-and-copy on this hot path. val bos = new ByteBufferOutputStream(512) ``` ### Why are the changes needed? Every RPC first writes a fixed preamble through an *unbuffered* `DataOutputStream` -- two `RpcAddress`es (each `boolean + UTF host + int port`) and the endpoint name -- and then the Java-serialized message body. Measuring the real serialized sizes (through the actual `JavaSerializer`, byte-identical to `serialize`): | Message | Bytes | reallocations from 32 | from 512 | |---|---:|:--:|:--:| | preamble only (sender set / null) | 60 / 43 | | | | `HeartbeatResponse(false)` | 136 | 3 | 0 | | `UpdateBlockInfo` | 205 | 3 | 0 | | `ReviveOffers` | 247 | 3 | 0 | | `GetLocations` | 263 | 4 | 0 | | `BlockManagerHeartbeat` | 291 | 4 | 0 | | `Heartbeat` (empty accumulators) | 637 | 5 | 1 | | `StatusUpdate` (empty data) | 1773 | 6 | 2 | The preamble alone (43-60 bytes) already exceeds the 32-byte default, so every RPC pays at least three reallocations (32 -> 64 -> 128 -> 256) before the buffer holds even the smallest message. Common control messages cluster around 130-300 bytes; 512 holds them with headroom and takes them from 3-4 reallocations to zero. The buffer is short-lived (handed to the outbox, written to the wire, then released), so the modest over-allocation is cheap, and `ByteBufferOutputStream.toByteBuffer` returns a buffer that aliases the backing array -- so buffer *reuse* is not safe here and pre-sizing is the applicable optimization. The two larger messages (`Heartbeat` with real accumulator updates, `StatusUpdate` with a result payload) exceed any reasonable default and resize regardless, so no larger value is warranted. This reduces steady allocation churn and GC on a hot control-plane path. It is not a throughput change and no benchmark claim is made. ### Does this PR introduce _any_ user-facing change? No. Only the initial capacity of an internal serialization buffer changes; the serialized bytes and all behavior are identical. ### How was this patch tested? The existing `NettyRpcEnvSuite` "RequestMessage serialization" test -- the serialize/deserialize round-trip that exercises this method -- passes. This is a behavior-preserving change, so no new tests were added. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Isaac This pull request and its description were written by Isaac. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
