docker stats: assorted fixes and optimizations in rendering

cli/command/container/formatter_stats.go

@@ -118,24 +118,15 @@ func NewStats(container string) *Stats {
 
 // statsFormatWrite renders the context for a list of containers statistics
 func statsFormatWrite(ctx formatter.Context, stats []StatsEntry, osType string, trunc bool) error {
-	render := func(format func(subContext formatter.SubContext) error) error {
-		for _, cstats := range stats {
-			statsCtx := &statsContext{
-				s:     cstats,
-				os:    osType,
-				trunc: trunc,
-			}
-			if err := format(statsCtx); err != nil {
-				return err
-			}
-		}
-		return nil
-	}
+	// TODO(thaJeztah): this should be taken from the (first) StatsEntry instead.
+	// also, assuming all stats are for the same platform (and basing the
+	// column headers on that) won't allow aggregated results, which could
+	// be mixed platform.
 	memUsage := memUseHeader
 	if osType == winOSType {
 		memUsage = winMemUseHeader
 	}
-	statsCtx := statsContext{}
+	statsCtx := statsContext{os: osType}
 	statsCtx.Header = formatter.SubHeaderContext{
 		"Container": containerHeader,
 		"Name":      formatter.NameHeader,
@@ -147,8 +138,18 @@ func statsFormatWrite(ctx formatter.Context, stats []StatsEntry, osType string,
 		"BlockIO":   blockIOHeader,
 		"PIDs":      pidsHeader,
 	}
-	statsCtx.os = osType
-	return ctx.Write(&statsCtx, render)
+	return ctx.Write(&statsCtx, func(format func(subContext formatter.SubContext) error) error {
+		for _, cstats := range stats {
+			if err := format(&statsContext{
+				s:     cstats,
+				os:    osType,
+				trunc: trunc,
+			}); err != nil {
+				return err
+			}
+		}
+		return nil
+	})
 }
 
 type statsContext struct {

cli/command/container/stats.go

@@ -7,9 +7,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
-	"fmt"
 	"io"
-	"strings"
 	"sync"
 	"time"
 
@@ -289,28 +287,25 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions)
 
 	// Buffer to store formatted stats text.
 	// Once formatted, it will be printed in one write to avoid screen flickering.
-	var statsTextBuffer bytes.Buffer
-
+	var buf bytes.Buffer
 	statsCtx := formatter.Context{
-		Output: &statsTextBuffer,
+		Output: &buf,
 		Format: NewStatsFormat(format, daemonOSType),
 	}
 
 	if options.NoStream {
-		cStats.mu.RLock()
-		ccStats := make([]StatsEntry, 0, len(cStats.cs))
-		for _, c := range cStats.cs {
-			ccStats = append(ccStats, c.GetStatistics())
-		}
-		cStats.mu.RUnlock()
-
-		if len(ccStats) == 0 {
+		statsList := cStats.snapshot()
+		if len(statsList) == 0 {
 			return nil
 		}
+		ccStats := make([]StatsEntry, 0, len(statsList))
+		for _, c := range statsList {
+			ccStats = append(ccStats, c.GetStatistics())
+		}
 		if err := statsFormatWrite(statsCtx, ccStats, daemonOSType, !options.NoTrunc); err != nil {
 			return err
 		}
-		_, _ = fmt.Fprint(dockerCLI.Out(), statsTextBuffer.String())
+		_, _ = dockerCLI.Out().Write(buf.Bytes())
 		return nil
 	}
 
@@ -319,30 +314,35 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions)
 	for {
 		select {
 		case <-ticker.C:
-			cStats.mu.RLock()
-			ccStats := make([]StatsEntry, 0, len(cStats.cs))
-			for _, c := range cStats.cs {
+			statsList := cStats.snapshot()
+			if len(statsList) == 0 && !showAll {
+				return nil
+			}
+			ccStats := make([]StatsEntry, 0, len(statsList))
+			for _, c := range statsList {
 				ccStats = append(ccStats, c.GetStatistics())
 			}
-			cStats.mu.RUnlock()
 
 			// Start by moving the cursor to the top-left
-			_, _ = fmt.Fprint(&statsTextBuffer, "\033[H")
+			_, _ = io.WriteString(&buf, "\033[H")
 
 			if err := statsFormatWrite(statsCtx, ccStats, daemonOSType, !options.NoTrunc); err != nil {
 				return err
 			}
 
-			for line := range strings.SplitSeq(statsTextBuffer.String(), "\n") {
+			// TODO(thaJeztah): update statsFormatWrite to directly produce the right format
+			// instead of post-processing the results.
+			for line := range bytes.SplitSeq(buf.Bytes(), []byte{'\n'}) {
 				// In case the new text is shorter than the one we are writing over,
 				// we'll append the "erase line" escape sequence to clear the remaining text.
-				_, _ = fmt.Fprintln(&statsTextBuffer, line, "\033[K")
+				_, _ = buf.Write(line)
+				_, _ = io.WriteString(&buf, "\033[K")
 			}
-			// We might have fewer containers than before, so let's clear the remaining text
-			_, _ = fmt.Fprint(&statsTextBuffer, "\033[J")
 
-			_, _ = fmt.Fprint(dockerCLI.Out(), statsTextBuffer.String())
-			statsTextBuffer.Reset()
+			// We might have fewer containers than before, so let's clear the remaining text
+			_, _ = io.WriteString(&buf, "\033[J")
+			_, _ = dockerCLI.Out().Write(buf.Bytes())
+			buf.Reset()
 
 			if len(ccStats) == 0 && !showAll {
 				return nil

cli/command/container/stats_helpers.go

@@ -49,6 +49,20 @@ func (s *stats) isKnownContainer(cid string) (int, bool) {
 	return -1, false
 }
 
+// snapshot returns a point-in-time copy of stats for the tracked containers.
+// The returned slice is safe for use without holding the stats lock.
+func (s *stats) snapshot() []*Stats {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	if len(s.cs) == 0 {
+		return nil
+	}
+	// https://github.com/golang/go/issues/53643
+	cp := make([]*Stats, len(s.cs))
+	copy(cp, s.cs)
+	return cp
+}
+
 func collect(ctx context.Context, s *Stats, cli client.ContainerAPIClient, streamStats bool, waitFirst *sync.WaitGroup) { //nolint:gocyclo
 	var getFirst bool