perf sched map: Apply cpu color when there's an activity

I'd like to see the name of tasks with perf sched map, but it only shows name of new tasks and then use short names after all. This is not good for long running tasks since it's hard for users to track the short names. This patch makes it show the names (except the idle task) when -v option is used. Probably we may make it as default behavior.

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/builtin-sched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/builtin-sched.c +1 -1

index 78006e991d91..97d6cbf486bb 100644
@@ -1493,7 +1493,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 		goto out;
 
 	color_fprintf(stdout, color, "  %12.6f secs ", (double)timestamp / NSEC_PER_SEC);
-	if (new_shortname) {
+	if (new_shortname || (verbose && sched_in->tid)) {
 		const char *pid_color = color;
 
 		if (thread__has_color(sched_in))

Signature · 1 line

2.10.0

tools/perf/builtin-script.c +3 -1

+
 		if (nanosecs)
 			printf("%5lu.%09llu: ", secs, nsecs);
 		else
-			printf("%5lu.%06lu: ", secs, usecs);
+			printf("%12s: ", timestamp_in_usec(buf, sz,
+							   sample->time));
 	}
 }

tools/perf/util/util.c +0 -0

index 85c56800f17a..aa3e778989ce 100644

Joonwoo reported that there's a mismatch between timestamps in script and sched commands. This was because of difference in printing the timestamp. Factor out the code and share it so that they can be in sync. Also I found that sched map has similar problem, fix it too.

Reported-by Joonwoo Park <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/builtin-sched.c  | 9 ++++++---
 tools/perf/builtin-script.c | 9 ++++++---
 tools/perf/util/util.c      | 9 +++++++++
 tools/perf/util/util.h      | 3 +++
 4 files changed, 24 insertions(+), 6 deletions(-)

tools/perf/builtin-sched.c +1 -0

index 97d6cbf486bb..c88d64ae997e 100644
@@ -1191,6 +1191,7 @@ static void output_lat_thread(struct perf_sched *sched, struct work_atoms *work_
 	int i;
 	int ret;
 	u64 avg;
+	char buf[32];
 
 	if (!work_list->nb_atoms)
 		return;

tools/perf/builtin-sched.c +3 -1

 }
 
 static int pid_cmp(struct work_atoms *l, struct work_atoms *r)

@@ -1402,6 +1403,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 	int cpus_nr;
 	bool new_cpu = false;
 	const char *color = PERF_COLOR_NORMAL;
+	char buf[32];
 
 	BUG_ON(this_cpu >= MAX_CPUS || this_cpu < 0);
 

@@ -1492,7 +1494,8 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 	if (sched->map.cpus && !cpu_map__has(sched->map.cpus, this_cpu))
 		goto out;
 
-	color_fprintf(stdout, color, "  %12.6f secs ", (double)timestamp / NSEC_PER_SEC);
+	color_fprintf(stdout, color, "  %12s secs ",
+		      timestamp_in_usec(buf, sizeof(buf), timestamp));
 	if (new_shortname || (verbose && sched_in->tid)) {
 		const char *pid_color = color;
 

tools/perf/builtin-script.c +6 -3

index 7228d141a789..c848c74bdc90 100644
@@ -437,7 +437,6 @@ static void print_sample_start(struct perf_sample *sample,
 {
 	struct perf_event_attr *attr = &evsel->attr;
 	unsigned long secs;
-	unsigned long usecs;
 	unsigned long long nsecs;
 
 	if (PRINT_FIELD(COMM)) {

@@ -464,14 +463,18 @@ static void print_sample_start(struct perf_sample *sample,
 	}
 
 	if (PRINT_FIELD(TIME)) {
+		char buf[32];
+		size_t sz = sizeof(buf);
+
 		nsecs = sample->time;
 		secs = nsecs / NSEC_PER_SEC;
 		nsecs -= secs * NSEC_PER_SEC;
-		usecs = nsecs / NSEC_PER_USEC;
+
 		if (nanosecs)
 			printf("%5lu.%09llu: ", secs, nsecs);
 		else
-			printf("%5lu.%06lu: ", secs, usecs);
+			printf("%12s: ", timestamp_in_usec(buf, sz,
+							   sample->time));
 	}
 }
 

tools/perf/util/util.c +0 -0

index 85c56800f17a..aa3e778989ce 100644

tools/perf/util/util.c +8 -0

+{
+	u64  sec = timestamp / NSEC_PER_SEC;
+	u64 usec = (timestamp % NSEC_PER_SEC) / NSEC_PER_USEC;
+
+	scnprintf(buf, sz, "%"PRIu64".%06"PRIu64, sec, usec);
+	return buf;
+}
+
 unsigned long parse_tag_value(const char *str, struct parse_tag *tags)
 {
 	struct parse_tag *i = tags;

tools/perf/util/util.h +3 -0

index 71b6992f1d98..ece974f1c538 100644
@@ -362,4 +362,7 @@ extern int sched_getcpu(void);
 #endif
 
 int is_printable_array(char *p, unsigned int len);
+
+char *timestamp_in_usec(char *buf, size_t sz, u64 timestamp);
+
 #endif /* GIT_COMPAT_UTIL_H */

Applying cpu color always doesn't help readability IMHO. Instead it might be better to applying the color when there's an activity on those CPUs.

Cc Jiri Olsa <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/builtin-sched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/builtin-sched.c +1 -1

index f5503ca22e1c..78006e991d91 100644
@@ -1479,7 +1479,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 			cpu_color = COLOR_CPUS;
 
 		if (cpu != this_cpu)
-			color_fprintf(stdout, cpu_color, " ");
+			color_fprintf(stdout, color, " ");
 		else
 			color_fprintf(stdout, cpu_color, "*");
 

Signature · 1 line

2.10.0

Applying cpu color always doesn't help readability IMHO. Instead it might be better to applying the color when there's an activity on those CPUs.

Cc Jiri Olsa <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/builtin-sched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/builtin-sched.c +1 -1

index f5503ca22e1c..78006e991d91 100644
@@ -1479,7 +1479,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 			cpu_color = COLOR_CPUS;
 
 		if (cpu != this_cpu)
-			color_fprintf(stdout, cpu_color, " ");
+			color_fprintf(stdout, color, " ");
 		else
 			color_fprintf(stdout, cpu_color, "*");
 

Signature · 1 line

2.10.0

Em Mon, Oct 24, 2016 at 11:02:43AM +0900, Namhyung Kim escreveu:

Quoted 3 lines

Applying cpu color always doesn't help readability IMHO. Instead it might be better to applying the color when there's an activity on those CPUs.

thanks, applied the three patches.