CAICAIIs commented on issue #3694: URL: https://github.com/apache/dubbo-go/issues/3694#issuecomment-5393193141
Thanks for the proposal. The direction is reasonable, but I would not take it as one broad change yet. The main risk is concurrency: `StreamingClientConn` allows `Send`, `RequestHeader`, and `CloseRequest` to race with each other, so any replacement for `io.Pipe` needs an explicit write-side snapshot/lock boundary, not only a response-side `Read`/`Close` guard. Otherwise `CloseRequest` can observe a partially written request body. The performance claim also needs a tighter proof. The current unary marshaler still allocates via `codec.Marshal` before anything reaches the transport, so the claimed copy reduction is not yet established. I would want an alloc benchmark, not just a transport rewrite, before calling that part solved. There is also a body lifetime issue to pin down. `net/http.Client.Do` may close request bodies asynchronously after it returns, so buffer reuse needs a precise rule across transport errors, redirects/retries, and context cancel. Without that, pool return can become racy. My suggestion is to split this into two steps: first a unary-only fast path behind a default-off flag that preserves current behavior for gRPC and streaming, then a separate pass for marshal/allocation reduction with race and alloc benchmarks. -- 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]
