cmd/go/internal/vcweb: cache hg servers

Cuts TestScript/reuse_hg from 73s to 47s.
(Python startup is slow! What's left is all Python too!)

Change-Id: Ia7124d4819286b3820355e4f427ffcfdc125491b
Reviewed-on: https://go-review.googlesource.com/c/go/+/718501
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>

diff --git a/src/cmd/go/internal/vcweb/hg.go b/src/cmd/go/internal/vcweb/hg.go
index e07cd3c8750357dcbc8ae216d2cd6a5321292052..fb77d1a2fccdac4430ecca63bfa42adafb3930de 100644 (file)
--- a/src/cmd/go/internal/vcweb/hg.go
+++ b/src/cmd/go/internal/vcweb/hg.go
@@ -25,6 +25,13 @@ type hgHandler struct {
        once      sync.Once
        hgPath    string
        hgPathErr error
+
+       mu     sync.Mutex
+       wg     sync.WaitGroup
+       ctx    context.Context
+       cancel func()
+       cmds   []*exec.Cmd
+       url    map[string]*url.URL
 }
 
 func (h *hgHandler) Available() bool {
@@ -34,6 +41,30 @@ func (h *hgHandler) Available() bool {
        return h.hgPathErr == nil
 }
 
+func (h *hgHandler) Close() error {
+       h.mu.Lock()
+       defer h.mu.Unlock()
+
+       if h.cancel == nil {
+               return nil
+       }
+
+       h.cancel()
+       for _, cmd := range h.cmds {
+               h.wg.Add(1)
+               go func() {
+                       cmd.Wait()
+                       h.wg.Done()
+               }()
+       }
+       h.wg.Wait()
+       h.url = nil
+       h.cmds = nil
+       h.ctx = nil
+       h.cancel = nil
+       return nil
+}
+
 func (h *hgHandler) Handler(dir string, env []string, logger *log.Logger) (http.Handler, error) {
        if !h.Available() {
                return nil, ServerNotInstalledError{name: "hg"}
@@ -50,10 +81,25 @@ func (h *hgHandler) Handler(dir string, env []string, logger *log.Logger) (http.
                // if "hg" works at all then "hg serve" works too, and we'll execute that as
                // a subprocess, using a reverse proxy to forward the request and response.
 
-               ctx, cancel := context.WithCancel(req.Context())
-               defer cancel()
+               h.mu.Lock()
+
+               if h.ctx == nil {
+                       h.ctx, h.cancel = context.WithCancel(context.Background())
+               }
 
-               cmd := exec.CommandContext(ctx, h.hgPath, "serve", "--port", "0", "--address", "localhost", "--accesslog", os.DevNull, "--name", "vcweb", "--print-url")
+               // Cache the hg server subprocess globally, because hg is too slow
+               // to start a new one for each request. There are under a dozen different
+               // repos we serve, so leaving a dozen processes around is not a big deal.
+               u := h.url[dir]
+               if u != nil {
+                       h.mu.Unlock()
+                       logger.Printf("proxying hg request to %s", u)
+                       httputil.NewSingleHostReverseProxy(u).ServeHTTP(w, req)
+                       return
+               }
+
+               logger.Printf("starting hg serve for %s", dir)
+               cmd := exec.CommandContext(h.ctx, h.hgPath, "serve", "--port", "0", "--address", "localhost", "--accesslog", os.DevNull, "--name", "vcweb", "--print-url")
                cmd.Dir = dir
                cmd.Env = append(slices.Clip(env), "PWD="+dir)
 
@@ -74,39 +120,32 @@ func (h *hgHandler) Handler(dir string, env []string, logger *log.Logger) (http.
 
                stdout, err := cmd.StdoutPipe()
                if err != nil {
+                       h.mu.Unlock()
                        http.Error(w, err.Error(), http.StatusInternalServerError)
                        return
                }
 
                if err := cmd.Start(); err != nil {
+                       h.mu.Unlock()
                        http.Error(w, err.Error(), http.StatusInternalServerError)
                        return
                }
-               var wg sync.WaitGroup
-               defer func() {
-                       cancel()
-                       err := cmd.Wait()
-                       if out := strings.TrimSuffix(stderr.String(), "interrupted!\n"); out != "" {
-                               logger.Printf("%v: %v\n%s", cmd, err, out)
-                       } else {
-                               logger.Printf("%v", cmd)
-                       }
-                       wg.Wait()
-               }()
 
                r := bufio.NewReader(stdout)
                line, err := r.ReadString('\n')
                if err != nil {
+                       h.mu.Unlock()
+                       http.Error(w, err.Error(), http.StatusInternalServerError)
                        return
                }
                // We have read what should be the server URL. 'hg serve' shouldn't need to
                // write anything else to stdout, but it's not a big deal if it does anyway.
                // Keep the stdout pipe open so that 'hg serve' won't get a SIGPIPE, but
                // actively discard its output so that it won't hang on a blocking write.
-               wg.Add(1)
+               h.wg.Add(1)
                go func() {
                        io.Copy(io.Discard, r)
-                       wg.Done()
+                       h.wg.Done()
                }()
 
                // On some systems,
@@ -116,12 +155,21 @@ func (h *hgHandler) Handler(dir string, env []string, logger *log.Logger) (http.
                line = strings.ReplaceAll(line, "//1.0.0.127.in-addr.arpa", "//127.0.0.1")
                line = strings.ReplaceAll(line, "//1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa", "//[::1]")
 
-               u, err := url.Parse(strings.TrimSpace(line))
+               u, err = url.Parse(strings.TrimSpace(line))
                if err != nil {
+                       h.mu.Unlock()
                        logger.Printf("%v: %v", cmd, err)
                        http.Error(w, err.Error(), http.StatusBadGateway)
                        return
                }
+
+               if h.url == nil {
+                       h.url = make(map[string]*url.URL)
+               }
+               h.url[dir] = u
+               h.cmds = append(h.cmds, cmd)
+               h.mu.Unlock()
+
                logger.Printf("proxying hg request to %s", u)
                httputil.NewSingleHostReverseProxy(u).ServeHTTP(w, req)
        })