diff --git a/cli/command/container/formatter_stats.go b/cli/command/container/formatter_stats.go index a7792f2bc89f..de8e3e98996d 100644 --- a/cli/command/container/formatter_stats.go +++ b/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 { diff --git a/cli/command/container/stats.go b/cli/command/container/stats.go index 1902fac833fe..9d4bfce51bff 100644 --- a/cli/command/container/stats.go +++ b/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 diff --git a/cli/command/container/stats_helpers.go b/cli/command/container/stats_helpers.go index 4f7c746be2cd..66571b530942 100644 --- a/cli/command/container/stats_helpers.go +++ b/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