docker stats: assorted fixes and optimizations in rendering

cli/command/container/formatter_stats.go

@@ -119,24 +119,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,
@@ -148,8 +139,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"
 
@@ -287,30 +285,32 @@ 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
+	// renderBuf holds the formatted stats output produced by statsFormatWrite.
+	// It does not include any terminal control sequences.
+	var renderBuf bytes.Buffer
+
+	// frameBuf holds the final terminal frame, including cursor movement and
+	// line-clearing escape sequences, written in a single pass to avoid flicker.
+	var frameBuf bytes.Buffer
 
 	statsCtx := formatter.Context{
-		Output: &statsTextBuffer,
+		Output: &renderBuf,
 		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(renderBuf.Bytes())
 		return nil
 	}
 
@@ -319,34 +319,38 @@ 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 {
+			renderBuf.Reset()
+			frameBuf.Reset()
+			statsList := cStats.snapshot()
+			if len(statsList) == 0 && !showAll {
+				// Clear screen
+				_, _ = io.WriteString(dockerCLI.Out(), "\033[H\033[J")
+				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")
 
 			if err := statsFormatWrite(statsCtx, ccStats, daemonOSType, !options.NoTrunc); err != nil {
 				return err
 			}
 
-			for line := range strings.SplitSeq(statsTextBuffer.String(), "\n") {
+			// Start by moving the cursor to the top-left
+			_, _ = io.WriteString(&frameBuf, "\033[H")
+
+			// TODO(thaJeztah): consider wrapping the writer to inject ANSI (line-clearing) during formatting.
+			// instead of post-processing the results.
+			for line := range bytes.SplitSeq(renderBuf.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")
+				_, _ = frameBuf.Write(line)
+				_, _ = io.WriteString(&frameBuf, "\033[K")
+				_ = frameBuf.WriteByte('\n')
 			}
 			// 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()
-
-			if len(ccStats) == 0 && !showAll {
-				return nil
-			}
+			_, _ = io.WriteString(&frameBuf, "\033[J")
+			_, _ = dockerCLI.Out().Write(frameBuf.Bytes())
 		case err, ok := <-closeChan:
 			if !ok || err == nil || errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
 				// Suppress "unexpected EOF" errors in the CLI so that

cli/command/container/stats_helpers.go

@@ -49,6 +49,22 @@ func (s *stats) isKnownContainer(cid string) (int, bool) {
 	return -1, false
 }
 
+// snapshot returns a point-in-time copy of the tracked container list
+// (the slice of *Stats pointers). The returned slice is safe for use
+// without holding the stats lock, but the underlying Stats values may
+// continue to change concurrently.
+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