From c75696b9b8498ae03a4ad9527b9b7c8337415456 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Thu, 5 Dec 2024 18:35:27 +0100 Subject: [PATCH] net/http: allocate CloseNotifier channel lazily MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CloseNotifier interface is deprecated. We can defer allocating the backing channel until the first use of CloseNotifier. goos: linux goarch: amd64 pkg: net/http cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz │ before │ after │ │ sec/op │ sec/op vs base │ Server-8 160.8µ ± 2% 160.1µ ± 1% ~ (p=0.353 n=10) CloseNotifier/h1-8 222.1µ ± 4% 226.4µ ± 7% ~ (p=0.143 n=10) geomean 189.0µ 190.4µ +0.75% │ before │ after │ │ B/op │ B/op vs base │ Server-8 2.292Ki ± 0% 2.199Ki ± 0% -4.07% (p=0.000 n=10) CloseNotifier/h1-8 3.224Ki ± 0% 3.241Ki ± 0% +0.51% (p=0.000 n=10) geomean 2.718Ki 2.669Ki -1.80% │ before │ after │ │ allocs/op │ allocs/op vs base │ Server-8 21.00 ± 0% 20.00 ± 0% -4.76% (p=0.000 n=10) CloseNotifier/h1-8 50.00 ± 0% 50.00 ± 0% ~ (p=1.000 n=10) ¹ geomean 32.40 31.62 -2.41% ¹ all samples are equal --- src/net/http/server.go | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/src/net/http/server.go b/src/net/http/server.go index 1e8e1437d26832..fc8f959b8682a2 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -261,7 +261,7 @@ type conn struct { // rwc is the underlying network connection. // This is never wrapped by other types and is the value given out - // to CloseNotifier callers. It is usually of type *net.TCPConn or + // to [Hijacker] callers. It is usually of type *net.TCPConn or // *tls.Conn. rwc net.Conn @@ -486,11 +486,12 @@ type response struct { clenBuf [10]byte statusBuf [3]byte + // lazyCloseNotifyMu protects closeNotifyCh and closeNotifyTriggered. + lazyCloseNotifyMu sync.Mutex // closeNotifyCh is the channel returned by CloseNotify. - // TODO(bradfitz): this is currently (for Go 1.8) always - // non-nil. Make this lazily-created again as it used to be? - closeNotifyCh chan bool - didCloseNotify atomic.Bool // atomic (only false->true winner should send) + closeNotifyCh chan bool + // closeNotifyTriggered tracks prior closeNotify calls. + closeNotifyTriggered bool } func (c *response) SetReadDeadline(deadline time.Time) error { @@ -761,9 +762,8 @@ func (cr *connReader) handleReadError(_ error) { // may be called from multiple goroutines. func (cr *connReader) closeNotify() { - res := cr.conn.curReq.Load() - if res != nil && !res.didCloseNotify.Swap(true) { - res.closeNotifyCh <- true + if res := cr.conn.curReq.Load(); res != nil { + res.closeNotify() } } @@ -1100,7 +1100,6 @@ func (c *conn) readRequest(ctx context.Context) (w *response, err error) { reqBody: req.Body, handlerHeader: make(Header), contentLength: -1, - closeNotifyCh: make(chan bool, 1), // We populate these ahead of time so we're not // reading from req.Header after their Handler starts @@ -2250,12 +2249,32 @@ func (w *response) Hijack() (rwc net.Conn, buf *bufio.ReadWriter, err error) { } func (w *response) CloseNotify() <-chan bool { + w.lazyCloseNotifyMu.Lock() + defer w.lazyCloseNotifyMu.Unlock() if w.handlerDone.Load() { panic("net/http: CloseNotify called after ServeHTTP finished") } + if w.closeNotifyCh == nil { + w.closeNotifyCh = make(chan bool, 1) + if w.closeNotifyTriggered { + w.closeNotifyCh <- true // action prior closeNotify call + } + } return w.closeNotifyCh } +func (w *response) closeNotify() { + w.lazyCloseNotifyMu.Lock() + defer w.lazyCloseNotifyMu.Unlock() + if w.closeNotifyTriggered { + return // already triggered + } + w.closeNotifyTriggered = true + if w.closeNotifyCh != nil { + w.closeNotifyCh <- true + } +} + func registerOnHitEOF(rc io.ReadCloser, fn func()) { switch v := rc.(type) { case *expectContinueReader: