From 366b3d3f62ddb9686b8296ba81dacfb8a283855a Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Mon, 10 May 2021 10:38:19 -0700
Subject: [PATCH] ipn{,/ipnserver}: delay JSON marshaling of ipn.Notifies

If nobody is connected to the IPN bus, don't burn CPU & waste
allocations (causing more GC) by encoding netmaps for nobody.

This will notably help hello.ipn.dev.

Updates tailscale/corp#1773

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 ipn/ipnserver/server.go | 47 +++++++++++++++++++++++++++++++++++------
 ipn/message.go          | 17 +++++----------
 ipn/message_test.go     |  7 +++++-
 3 files changed, 51 insertions(+), 20 deletions(-)

diff --git a/ipn/ipnserver/server.go b/ipn/ipnserver/server.go
index 0b323dba7..228d9f1cd 100644
--- a/ipn/ipnserver/server.go
+++ b/ipn/ipnserver/server.go
@@ -6,7 +6,9 @@ package ipnserver
 
 import (
 	"bufio"
+	"bytes"
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -251,8 +253,7 @@ func (s *server) serveConn(ctx context.Context, c net.Conn, logf logger.Logf) {
 			return
 		}
 		defer c.Close()
-		serverToClient := func(b []byte) { ipn.WriteMsg(c, b) }
-		bs := ipn.NewBackendServer(logf, nil, serverToClient)
+		bs := ipn.NewBackendServer(logf, nil, jsonNotifier(c, s.logf))
 		_, occupied := err.(inUseOtherUserError)
 		if occupied {
 			bs.SendInUseOtherUserErrorMessage(err.Error())
@@ -567,7 +568,9 @@ func (s *server) setServerModeUserLocked() {
 	}
 }
 
-func (s *server) writeToClients(b []byte) {
+var jsonEscapedZero = []byte(`\u0000`)
+
+func (s *server) writeToClients(n ipn.Notify) {
 	inServerMode := s.b.InServerMode()
 
 	s.mu.Lock()
@@ -584,8 +587,17 @@ func (s *server) writeToClients(b []byte) {
 		}
 	}
 
-	for c := range s.clients {
-		ipn.WriteMsg(c, b)
+	if len(s.clients) == 0 {
+		// Common case (at least on busy servers): nobody
+		// connected (no GUI, etc), so return before
+		// serializing JSON.
+		return
+	}
+
+	if b, ok := marshalNotify(n, s.logf); ok {
+		for c := range s.clients {
+			ipn.WriteMsg(c, b)
+		}
 	}
 }
 
@@ -671,8 +683,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (
 			errMsg := err.Error()
 			go func() {
 				defer c.Close()
-				serverToClient := func(b []byte) { ipn.WriteMsg(c, b) }
-				bs := ipn.NewBackendServer(logf, nil, serverToClient)
+				bs := ipn.NewBackendServer(logf, nil, jsonNotifier(c, logf))
 				bs.SendErrorMessage(errMsg)
 				time.Sleep(time.Second)
 			}()
@@ -962,3 +973,25 @@ func peerPid(entries []netstat.Entry, la, ra netaddr.IPPort) int {
 	}
 	return 0
 }
+
+// jsonNotifier returns a notify-writer func that writes ipn.Notify
+// messages to w.
+func jsonNotifier(w io.Writer, logf logger.Logf) func(ipn.Notify) {
+	return func(n ipn.Notify) {
+		if b, ok := marshalNotify(n, logf); ok {
+			ipn.WriteMsg(w, b)
+		}
+	}
+}
+
+func marshalNotify(n ipn.Notify, logf logger.Logf) (b []byte, ok bool) {
+	b, err := json.Marshal(n)
+	if err != nil {
+		logf("ipnserver: [unexpected] error serializing JSON: %v", err)
+		return nil, false
+	}
+	if bytes.Contains(b, jsonEscapedZero) {
+		logf("[unexpected] zero byte in BackendServer.send notify message: %q", b)
+	}
+	return b, true
+}
diff --git a/ipn/message.go b/ipn/message.go
index 7f8d2be21..d8b7d1396 100644
--- a/ipn/message.go
+++ b/ipn/message.go
@@ -88,9 +88,9 @@ type Command struct {
 
 type BackendServer struct {
 	logf          logger.Logf
-	b             Backend              // the Backend we are serving up
-	sendNotifyMsg func(jsonMsg []byte) // send a notification message
-	GotQuit       bool                 // a Quit command was received
+	b             Backend      // the Backend we are serving up
+	sendNotifyMsg func(Notify) // send a notification message
+	GotQuit       bool         // a Quit command was received
 }
 
 // NewBackendServer creates a new BackendServer using b.
@@ -98,7 +98,7 @@ type BackendServer struct {
 // If sendNotifyMsg is non-nil, it additionally sets the Backend's
 // notification callback to call the func with ipn.Notify messages in
 // JSON form. If nil, it does not change the notification callback.
-func NewBackendServer(logf logger.Logf, b Backend, sendNotifyMsg func(b []byte)) *BackendServer {
+func NewBackendServer(logf logger.Logf, b Backend, sendNotifyMsg func(Notify)) *BackendServer {
 	bs := &BackendServer{
 		logf:          logf,
 		b:             b,
@@ -115,14 +115,7 @@ func (bs *BackendServer) send(n Notify) {
 		return
 	}
 	n.Version = version.Long
-	b, err := json.Marshal(n)
-	if err != nil {
-		log.Fatalf("Failed json.Marshal(notify): %v\n%#v", err, n)
-	}
-	if bytes.Contains(b, jsonEscapedZero) {
-		log.Printf("[unexpected] zero byte in BackendServer.send notify message: %q", b)
-	}
-	bs.sendNotifyMsg(b)
+	bs.sendNotifyMsg(n)
 }
 
 func (bs *BackendServer) SendErrorMessage(msg string) {
diff --git a/ipn/message_test.go b/ipn/message_test.go
index 3e2f1b8d3..79c4a76b6 100644
--- a/ipn/message_test.go
+++ b/ipn/message_test.go
@@ -7,6 +7,7 @@ package ipn
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"testing"
 	"time"
 
@@ -74,7 +75,11 @@ func TestClientServer(t *testing.T) {
 			bc.GotNotifyMsg(b)
 		}
 	}()
-	serverToClient := func(b []byte) {
+	serverToClient := func(n Notify) {
+		b, err := json.Marshal(n)
+		if err != nil {
+			panic(err.Error())
+		}
 		serverToClientCh <- append([]byte{}, b...)
 	}
 	clientToServer := func(b []byte) {