From a076badb8b33f1ecdc5d46f0a3701f10c0579f73 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Tue, 22 May 2018 16:26:30 -0700
Subject: [PATCH] ContainerTop: speed up

Current ContainerTop (a.k.a. docker top) implementation uses "ps"
to get the info about *all* running processes, then parses it, then
filters the results to only contain PIDs used by the container.
Collecting data only to throw most of it away is inefficient,
especially on a system running many containers (or processes).
For example, "docker top" on a container with a single process
can take up to 0.5 seconds to execute (on a mostly idle system)
which is noticeably slow.

Since the containers PIDs are known beforehand, let's use ps's
"-q" option to provide it with a list of PIDs we want info about.

The problem with this approach is, some ps options can't be used
with "-q" (the only one I'm aware of is "f" ("forest view") but
there might be more). As the list of such options is not known,
in case ps fails, it is executed again without "q" (retaining
the old behavior).

Next, the data produced by "ps" is filtered in the same way as before.
The difference here is, in case "-q" worked, the list is much shorter.

I ran some benchmarks on my laptop, with about 8000 "sleep" processes
running to amplify the savings.

The improvement in "docker top" execution times is 5x to 10x (roughly
0.05s vs 0.5s).

The improvement in ContainerTop() execution time is up to 100x
(roughly 3ms vs 300ms).

I haven't measured the memory or the CPU time savings, guess those
are not that critical.

NOTE that busybox ps does not implement -q so the fallback is always
used, but AFAIK it is not usable anyway and Docker expects a normal
ps to be on the system (say the list of fields produced by
"busybox ps -ef" differs from normal "ps -ef" etc.).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 daemon/top_unix.go | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/daemon/top_unix.go b/daemon/top_unix.go
index 28cfb370d8..7854d2376f 100644
--- a/daemon/top_unix.go
+++ b/daemon/top_unix.go
@@ -113,6 +113,20 @@ func parsePSOutput(output []byte, procs []uint32) (*container.ContainerTopOKBody
 	return procList, nil
 }
 
+// psPidsArg converts a slice of PIDs to a string consisting
+// of comma-separated list of PIDs prepended by "-q".
+// For example, psPidsArg([]uint32{1,2,3}) returns "-q1,2,3".
+func psPidsArg(pids []uint32) string {
+	b := []byte{'-', 'q'}
+	for i, p := range pids {
+		b = strconv.AppendUint(b, uint64(p), 10)
+		if i < len(pids)-1 {
+			b = append(b, ',')
+		}
+	}
+	return string(b)
+}
+
 // ContainerTop lists the processes running inside of the given
 // container by calling ps with the given args, or with the flags
 // "-ef" if no args are given.  An error is returned if the container
@@ -145,9 +159,16 @@ func (daemon *Daemon) ContainerTop(name string, psArgs string) (*container.Conta
 		return nil, err
 	}
 
-	output, err := exec.Command("ps", strings.Split(psArgs, " ")...).Output()
+	args := strings.Split(psArgs, " ")
+	pids := psPidsArg(procs)
+	output, err := exec.Command("ps", append(args, pids)...).Output()
 	if err != nil {
-		return nil, fmt.Errorf("Error running ps: %v", err)
+		// some ps options (such as f) can't be used together with q,
+		// so retry without it
+		output, err = exec.Command("ps", args...).Output()
+		if err != nil {
+			return nil, fmt.Errorf("Error running ps: %v", err)
+		}
 	}
 	procList, err := parsePSOutput(output, procs)
 	if err != nil {