[PATCH 0/6] libtraceeval histogram: Updates

[PATCH 0/6] libtraceeval histogram: Updates

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

This patch set is based on top of:

 https://lore.kernel.org/all/20230808161204.5704-1-stevie.6strings@gmail.com/

I added a sample program task-eval which is one of the tools that will be
using this library. The first patch adds task-eval but that is still using
the old API (defined in trace-analysis.c).

The next patches modify the new API to fit with the use case of task-eval.
One is to use "pointer" as I'm not sure exactly the usecase of the dynamic
structure.

The cmp and release callbacks are changed to be more generic, and they get
called if they simply exist for a given type. I can imagine wanting a
release function for event the most mundane types (like number_32).

The cmp was also updated to pass in the traceeval descriptor, as I found
that I needed access to it while doing a compare (although, I rewrote the
code a bit where that use case isn't in the tool anymore).

Some fixes were made to the query.

The last patch updates the task-eval to use the new API. It added stubs that
are needed for the display portion. These can be implemented later.

Note, when running the new task-eval, it crashes immediately, so there's
lots of bugs to still fix in the existing histogram code.

Happy programming!

Steven Rostedt (Google) (6):
  libtraceeval: Add sample task-eval program
  libtraceeval hist: Add pointer and const string types
  libtraceeval histogram: Have cmp and release functions be generic
  libtraceeval histograms: Add traceeval struct to compare function
  libtraceeval histogram: Fix the return value of traceeval_query()
  libtraceeval samples: Update task-eval to use the histogram logic

 Makefile                 |   3 +
 include/traceeval-hist.h |  38 +-
 samples/Makefile         |  29 ++
 samples/task-eval.c      | 948 +++++++++++++++++++++++++++++++++++++++
 src/histograms.c         |  48 +-
 5 files changed, 1026 insertions(+), 40 deletions(-)
 create mode 100644 samples/Makefile
 create mode 100644 samples/task-eval.c

-- 
2.40.1

[PATCH 1/6] libtraceeval: Add sample task-eval program

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Add a sample task eval program to test the traceeval logic.

This relies on having libtracecmd installed.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 Makefile            |   3 +
 samples/Makefile    |  29 ++
 samples/task-eval.c | 859 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 891 insertions(+)
 create mode 100644 samples/Makefile
 create mode 100644 samples/task-eval.c

diff --git a/Makefile b/Makefile
index 3ea051cbebf5..62ca9285d7a0 100644
--- a/Makefile
+++ b/Makefile
@@ -348,6 +348,9 @@ $(LIBRARY_STATIC): force
 $(LIBRARY_SHARED): force
 	$(Q)$(call descend,$(src)/src,$(LIBRARY_SO))
 
+samples: $(LIBRARY_STATIC) force
+	$(Q)$(call descend,$(src)/samples,all)
+
 #	$(Q)$(call descend_clean,utest)
 clean:
 	$(Q)$(call descend_clean,src)
diff --git a/samples/Makefile b/samples/Makefile
new file mode 100644
index 000000000000..012301ec5542
--- /dev/null
+++ b/samples/Makefile
@@ -0,0 +1,29 @@
+# SPDX-License-Identifier: LGPL-2.1
+
+include $(src)/scripts/utils.mk
+
+TARGETS :=
+TARGETS += task-eval
+
+sdir := $(obj)/bin
+
+CFLAGS += `pkg-config --cflags libtracecmd`
+LIBRARY_LIBS += `pkg-config --libs libtracecmd`
+
+TARGETS := $(patsubst %,$(sdir)/%,$(TARGETS))
+
+all: $(TARGETS)
+
+$(sdir):
+	@mkdir -p $(sdir)
+
+$(TARGETS): $(sdir) $(LIBTRACEEVAL_STATIC)
+
+$(sdir)/%: $(bdir)/%.o
+	$(call do_sample_build,$@,$<)
+
+$(bdir)/%.o: $(bdir)/%.c
+	$(Q)$(CC) -o $@ -c $< $(CFLAGS) $(INCLUDES)
+
+clean:
+	$(Q)$(call do_clean,$(sdir)/*)
diff --git a/samples/task-eval.c b/samples/task-eval.c
new file mode 100644
index 000000000000..26e48c376f45
--- /dev/null
+++ b/samples/task-eval.c
@@ -0,0 +1,859 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdarg.h>
+#include <getopt.h>
+#include <errno.h>
+#include <trace-cmd.h>
+#include <traceeval.h>
+
+static char *argv0;
+
+static char *get_this_name(void)
+{
+	static char *this_name;
+	char *arg;
+	char *p;
+
+	if (this_name)
+		return this_name;
+
+	arg = argv0;
+	p = arg+strlen(arg);
+
+	while (p >= arg && *p != '/')
+		p--;
+	p++;
+
+	this_name = p;
+	return p;
+}
+
+static void usage(void)
+{
+	char *p = get_this_name();
+
+	printf("usage: %s [-c comm] trace.dat\n"
+	       "\n"
+	       "  Run this after running: trace-cmd record -e sched\n"
+	       "\n"
+	       "  Do some work and then hit Ctrl^C to stop the recording.\n"
+	       "  Run this on the resulting trace.dat file\n"
+	       "\n"
+	       "-c comm - to look at only a specific process called 'comm'\n"
+	       "\n",p);
+	exit(-1);
+}
+
+static void __vdie(const char *fmt, va_list ap, int err)
+{
+	int ret = errno;
+	char *p = get_this_name();
+
+	if (err && errno)
+		perror(p);
+	else
+		ret = -1;
+
+	fprintf(stderr, "  ");
+	vfprintf(stderr, fmt, ap);
+
+	fprintf(stderr, "\n");
+	exit(ret);
+}
+
+void die(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	__vdie(fmt, ap, 0);
+	va_end(ap);
+}
+
+void pdie(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	__vdie(fmt, ap, 1);
+	va_end(ap);
+}
+
+enum sched_state {
+	RUNNING,
+	BLOCKED,
+	PREEMPT,
+	SLEEP,
+	IDLE,
+	OTHER
+};
+
+struct process_data {
+	struct traceeval	*teval_cpus;
+	struct traceeval	*teval_threads;
+	char			*comm;
+	int			state;
+};
+
+struct task_data {
+	struct traceeval	*teval_cpus;
+	struct traceeval	*teval_processes;
+	char			*comm;
+};
+
+enum command {
+	START,
+	STOP
+};
+
+static void update_process(struct task_data *tdata, const char *comm,
+			   enum sched_state state, enum command cmd,
+			   unsigned long long ts)
+{
+	struct traceeval_key keys[] = {
+		{
+			.type = TRACEEVAL_TYPE_STRING,
+			.string = comm,
+		},
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.number = state,
+		}
+	};
+	int ret;
+
+	switch (cmd) {
+	case START:
+		ret = traceeval_n_start(tdata->teval_processes, keys, ts);
+		if (ret < 0)
+			pdie("Could not start process");
+		return;
+	case STOP:
+		ret = traceeval_n_stop(tdata->teval_processes, keys, ts);
+		if (ret < 0)
+			pdie("Could not stop process");
+		return;
+	}
+}
+
+static void start_process(struct task_data *tdata, const char *comm,
+			   enum sched_state state, unsigned long long ts)
+{
+	update_process(tdata, comm, state, START, ts);
+}
+
+static void stop_process(struct task_data *tdata, const char *comm,
+			   enum sched_state state, unsigned long long ts)
+{
+	update_process(tdata, comm, state, STOP, ts);
+}
+
+static struct process_data *
+get_process_data(struct task_data *tdata, const char *comm)
+{
+	struct traceeval_key keys[] = {
+		{
+			.type = TRACEEVAL_TYPE_STRING,
+			.string = comm,
+		},
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.number = RUNNING,
+		}
+	};
+					     
+	return traceeval_n_get_private(tdata->teval_processes, keys);
+}
+
+void set_process_data(struct task_data *tdata, const char *comm, void *data)
+{
+	struct traceeval_key keys[] = {
+		{
+			.type = TRACEEVAL_TYPE_STRING,
+			.string = comm,
+		},
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.number = RUNNING,
+		}
+	};
+	int ret;
+					     
+	ret = traceeval_n_set_private(tdata->teval_processes, keys, data);
+	if (ret < 0)
+		pdie("Failed to set process data");
+}
+
+static void update_cpu(struct traceeval *teval, int cpu,
+		       enum sched_state state, enum command cmd,
+		       unsigned long long ts)
+{
+	struct traceeval_key keys[] = {
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.number = cpu,
+		},
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.number = state,
+		}
+	};
+	int ret;
+
+	switch (cmd) {
+	case START:
+		ret = traceeval_n_continue(teval, keys, ts);
+		if (ret < 0)
+			pdie("Could not start CPU");
+		return;
+	case STOP:
+		ret = traceeval_n_stop(teval, keys, ts);
+		if (ret < 0)
+			pdie("Could not stop CPU");
+		return;
+	}
+}
+
+static void start_cpu(struct traceeval *teval, int cpu,
+		      enum sched_state state,  unsigned long long ts)
+{
+	update_cpu(teval, cpu, state, START, ts);
+}
+
+static void stop_cpu(struct traceeval *teval, int cpu,
+		     enum sched_state state, unsigned long long ts)
+{
+	update_cpu(teval, cpu, state, STOP, ts);
+}
+
+static void update_thread(struct process_data *pdata, int tid,
+			  enum sched_state state, enum command cmd,
+			  unsigned long long ts)
+{
+	struct traceeval_key keys[] = {
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.number = tid,
+		},
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.number = state,
+		}
+	};
+	int ret;
+
+	switch (cmd) {
+	case START:
+		ret = traceeval_n_start(pdata->teval_threads, keys, ts);
+		if (ret < 0)
+			pdie("Could not start thread");
+		return;
+	case STOP:
+		ret = traceeval_n_stop(pdata->teval_threads, keys, ts);
+		if (ret < 0)
+			pdie("Could not stop thread");
+		return;
+	}
+}
+
+static void start_thread(struct process_data *pdata, int tid,
+			   enum sched_state state, unsigned long long ts)
+{
+	update_thread(pdata, tid, state, START, ts);
+}
+
+static void stop_thread(struct process_data *pdata, int tid,
+			enum sched_state state, unsigned long long ts)
+{
+	update_thread(pdata, tid, state, STOP, ts);
+}
+
+static struct tep_format_field *get_field(struct tep_event *event, const char *name)
+{
+	static struct tep_format_field *field;
+
+	field = tep_find_field(event, name);
+	if (!field)
+		die("Could not find field %s for %s",
+		    name, event->name);
+
+	return field;
+}
+
+static void init_process_data(struct process_data *pdata)
+{
+	struct traceeval_key_info cpu_info[] = {
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.name = "CPU",
+		},
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.name = "Schedule state",
+		}
+	};
+	struct traceeval_key_info thread_info[] = {
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.name = "TID",
+		},
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.name = "Schedule state",
+		}
+	};
+
+	pdata->teval_cpus = traceeval_2_alloc("CPUs", cpu_info);
+	if (!pdata->teval_cpus)
+		pdie("Creating trace eval");
+
+	pdata->teval_threads = traceeval_2_alloc("Threads", thread_info);
+	if (!pdata->teval_threads)
+		pdie("Creating trace eval");
+}
+
+static struct process_data *alloc_pdata(struct task_data *tdata, const char *comm)
+{
+	struct process_data *pdata;
+
+	pdata = calloc(1, sizeof(*pdata));
+	if (!pdata)
+		pdie("Allocating process data");
+	init_process_data(pdata);
+	set_process_data(tdata, comm, pdata);
+
+	return pdata;
+}
+
+static void sched_out(struct task_data *tdata, const char *comm,
+		      struct tep_event *event,
+		      struct tep_record *record, struct tep_format_field *prev_pid,
+		      struct tep_format_field *prev_state)
+{
+	struct process_data *pdata;
+	unsigned long long val;
+	int pid;
+	int ret;
+
+	ret = tep_read_number_field(prev_pid, record->data, &val);
+	if (ret < 0)
+		die("Could not read sched_switch next_pid for record");
+
+	/* Ignore the idle task */
+	pid = val;
+	if (!pid) {
+		/* Record the runtime for the process CPUs */
+		stop_cpu(tdata->teval_cpus, record->cpu, IDLE, record->ts);
+		return;
+	}
+
+	/* The process is scheduling out. Stop the run time. */
+	update_process(tdata, comm, RUNNING, STOP, record->ts);
+
+	/* Get the process data from the process running state */
+	pdata = get_process_data(tdata, comm);
+	if (!pdata)
+		pdata = alloc_pdata(tdata, comm);
+
+	ret = tep_read_number_field(prev_state, record->data, &val);
+	if (ret < 0)
+		die("Could not read sched_switch next_pid for record");
+	val &= 3;
+	/*
+	 * Save the state the process is exiting with. Will need this
+	 * when scheduled back in.
+	 */
+	if (!val)
+		pdata->state = PREEMPT;
+	else if (val & 1)
+		pdata->state = SLEEP;
+	else if (val & 2)
+		pdata->state = BLOCKED;
+
+	/* Record the state timings for the process */
+	start_process(tdata, comm, pdata->state, record->ts);
+
+	/* Record the state timings for the individual thread */
+	stop_thread(pdata, pid, RUNNING, record->ts);
+
+	/* Record the state timings for the individual thread */
+	start_thread(pdata, pid, pdata->state, record->ts);
+
+	/* Record the runtime for the process CPUs */
+	stop_cpu(pdata->teval_cpus, record->cpu, RUNNING, record->ts);
+
+	/* Record the runtime for the all CPUs */
+	stop_cpu(tdata->teval_cpus, record->cpu, RUNNING, record->ts);
+}
+
+static void sched_in(struct task_data *tdata, const char *comm,
+		     struct tep_event *event,
+		     struct tep_record *record, struct tep_format_field *next_pid)
+{
+	struct process_data *pdata;
+	unsigned long long val;
+	bool is_new = false;
+	int ret;
+	int pid;
+
+	ret = tep_read_number_field(next_pid, record->data, &val);
+	if (ret < 0)
+		die("Could not read sched_switch next_pid for record");
+	pid = val;
+
+	/* Ignore the idle task */
+	if (!pid) {
+		/* Record the runtime for the process CPUs */
+		start_cpu(tdata->teval_cpus, record->cpu, IDLE, record->ts);
+		return;
+	}
+
+	/* Start recording the running time of this process */
+	start_process(tdata, comm, RUNNING, record->ts);
+
+	pdata = get_process_data(tdata, comm);
+
+	/* Start recording the running time of process CPUs */
+	start_cpu(tdata->teval_cpus, record->cpu, RUNNING, record->ts);
+
+	/* If there was no pdata, then this process did not go through sched out */
+	if (!pdata) {
+		pdata = alloc_pdata(tdata, comm);
+		is_new = true;
+	}
+
+	/* Record the state timings for the individual thread */
+	start_thread(pdata, pid, RUNNING, record->ts);
+
+	/* Start recording the running time of process CPUs */
+	start_cpu(pdata->teval_cpus, record->cpu, RUNNING, record->ts);
+
+	/* If it was just created, there's nothing to stop */
+	if (is_new)
+		return;
+
+	/* Stop recording the thread time for its scheduled out state */
+	stop_thread(pdata, val, pdata->state, record->ts);
+
+	/* Stop recording the process time for its scheduled out state */
+	stop_process(tdata, comm, pdata->state, record->ts);
+}
+
+static int switch_func(struct tracecmd_input *handle, struct tep_event *event,
+		       struct tep_record *record, int cpu, void *data)
+{
+	static struct tep_format_field *prev_comm;
+	static struct tep_format_field *prev_pid;
+	static struct tep_format_field *prev_state;
+	static struct tep_format_field *next_comm;
+	static struct tep_format_field *next_pid;
+	struct task_data *tdata = data;
+	const char *comm;
+
+	if (!next_comm) {
+		prev_comm = get_field(event, "prev_comm");
+		prev_pid = get_field(event, "prev_pid");
+		prev_state = get_field(event, "prev_state");
+
+		next_comm = get_field(event, "next_comm");
+		next_pid = get_field(event, "next_pid");
+	}
+
+	comm = record->data + prev_comm->offset;
+	if (!tdata->comm || strcmp(comm, tdata->comm) == 0)
+		sched_out(tdata, comm, event, record, prev_pid, prev_state);
+
+	comm = record->data + next_comm->offset;
+	if (!tdata->comm || strcmp(comm, tdata->comm) == 0)
+		sched_in(tdata, comm, event, record, next_pid);
+		
+	return 0;
+}
+
+static void print_microseconds(int idx, unsigned long long nsecs)
+{
+	unsigned long long usecs;
+
+	usecs = nsecs / 1000;
+	if (!nsecs || usecs)
+		printf("%*lld\n", idx, usecs);
+	else
+		printf("%*d.%03lld\n", idx, 0, nsecs);
+}
+
+static void display_cpus(struct traceeval *teval)
+{
+	struct traceeval_key_array *karray;
+	const struct traceeval_key *ckey;
+	const struct traceeval_key *skey;
+	int last_cpu = -1;
+	int i, nr;
+
+	printf("\n");
+
+	nr = traceeval_result_nr(teval);
+	if (!nr)
+		die("No result for CPUs\n");
+
+	for (i = 0; i < nr; i++) {
+		karray = traceeval_result_indx_key_array(teval, i);
+		if (!karray)
+			die("No cpu key for result %d\n", i);
+		ckey = traceeval_key_array_indx(karray, 0);
+		skey = traceeval_key_array_indx(karray, 1);
+
+
+		if (last_cpu != ckey->number)
+			printf("    CPU [%d]:\n", (int)ckey->number);
+
+		switch (skey->number) {
+		case RUNNING:
+			printf("       Running: ");
+			break;
+		case IDLE:
+			printf("          Idle: ");
+			break;
+		case BLOCKED:
+		case PREEMPT:
+		case SLEEP:
+		case OTHER:
+			printf("         \?\?(%ld): ", skey->number);
+			break;
+		}
+		printf(" time (us):");
+		print_microseconds(12, traceeval_result_indx_total(teval, i));
+
+		last_cpu = ckey->number;
+	}
+}
+
+static void display_thread(struct traceeval *teval, int tid)
+{
+	struct traceeval_key keys[2] =
+		{
+			{
+				.type = TRACEEVAL_TYPE_NUMBER,
+				.number = tid,
+			},
+			{
+				.type = TRACEEVAL_TYPE_NUMBER,
+				.number = RUNNING,
+			}
+		};
+	size_t ret;
+
+	printf("\n    thread id: %d\n", tid);
+
+	printf("      Total run time (us):");
+	print_microseconds(14, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
+
+	keys[1].number = PREEMPT;
+	printf("      Total preempt time (us):");
+	print_microseconds(10, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
+
+	keys[1].number = BLOCKED;
+	printf("      Total blocked time (us):");
+	print_microseconds(10, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
+
+	keys[1].number = SLEEP;
+	printf("      Total sleep time (us):");
+	print_microseconds(12, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
+};
+
+static void display_threads(struct traceeval *teval)
+{
+	struct traceeval_key_array *karray;
+	const struct traceeval_key *tkey;
+	struct traceeval_key keys[2];
+	int last_tid = -1;
+	int i, nr;
+
+	nr = traceeval_result_nr(teval);
+	if (!nr)
+		die("No result for threads\n");
+
+	memset(keys, 0, sizeof(keys));
+	keys[1].type = TRACEEVAL_TYPE_NUMBER;
+
+	for (i = 0; i < nr; i++) {
+		karray = traceeval_result_indx_key_array(teval, i);
+		if (!karray)
+			die("No thread key for result %d\n", i);
+		tkey = traceeval_key_array_indx(karray, 0);
+		if (!tkey)
+			die("No thread keys for result?");
+
+		/*
+		 * All the TIDS should be together in the results,
+		 * as the results are sorted by the first key, which
+		 * is the comm.
+		 */
+		if (last_tid == tkey->number)
+			continue;
+
+		last_tid = tkey->number;
+
+		display_thread(teval, tkey->number);
+	}
+}
+
+static void display_process(struct traceeval *teval, struct process_data *pdata,
+			    const char *comm)
+{
+	struct traceeval_key keys[2] =
+		{
+			{
+				.type = TRACEEVAL_TYPE_STRING,
+				.string = comm,
+			},
+			{
+				.type = TRACEEVAL_TYPE_NUMBER,
+				.number = RUNNING,
+			}
+		};
+	size_t ret;
+
+	printf("Task: %s\n", comm);
+
+	printf("  Total run time (us):");
+	print_microseconds(18, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
+
+	keys[1].number = PREEMPT;
+	printf("  Total preempt time (us):");
+	print_microseconds(14, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
+
+	keys[1].number = BLOCKED;
+	printf("  Total blocked time (us):");
+	print_microseconds(14, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
+
+	keys[1].number = SLEEP;
+	printf("  Total sleep time (us):");
+	print_microseconds(16, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
+
+	display_threads(pdata->teval_threads);
+	display_cpus(pdata->teval_cpus);
+	printf("\n");
+}
+
+static int compare_pdata(struct traceeval *teval,
+			 const struct traceeval_key_array *A,
+			 const struct traceeval_key_array *B,
+			 void *data)
+{
+	struct traceeval_key akeys[2];
+	struct traceeval_key bkeys[2];
+	const struct traceeval_key *akey;
+	const struct traceeval_key *bkey;
+	long long aval;
+	long long bval;
+	int ret;
+
+	/* Get the RUNNING values for this process */
+
+	akey = traceeval_key_array_indx(A, 0);
+	akeys[0] = *akey;
+	akeys[1].type = TRACEEVAL_TYPE_NUMBER;
+	akeys[1].number = RUNNING;
+
+	bkey = traceeval_key_array_indx(B, 0);
+	bkeys[0] = *bkey;
+	bkeys[1].type = TRACEEVAL_TYPE_NUMBER;
+	bkeys[1].number = RUNNING;
+
+	aval = traceeval_result_keys_total(teval, akey);
+	bval = traceeval_result_keys_total(teval, bkeys);
+
+	if (aval < 0)
+		return -1;
+	if (bval < 0)
+		return -1;
+
+	if (bval < aval)
+		return -1;
+	if (bval > aval)
+		return 1;
+
+	ret = strcmp(bkeys[0].string, akeys[0].string);
+
+	/* If two different processes have the same runtime, sort by name */
+	if (ret)
+		return ret;
+
+	/* Same process, sort by state */
+
+	akey = traceeval_key_array_indx(A, 1);
+	bkey = traceeval_key_array_indx(B, 1);
+
+	if (bkey->number < akey->number)
+		return -1;
+
+	return bkey->number > akey->number;
+}
+
+static void display_processes(struct traceeval *teval)
+{
+	struct traceeval_key_array *karray;
+	const struct traceeval_key *tkey;
+	struct traceeval_key keys[2];
+	struct process_data *pdata;
+	const char *last_comm = NULL;
+	int i, nr;
+
+	nr = traceeval_result_nr(teval);
+	if (!nr)
+		die("No result for processes\n");
+
+	memset(keys, 0, sizeof(keys));
+	keys[1].type = TRACEEVAL_TYPE_NUMBER;
+
+	for (i = 0; i < nr; i++) {
+		karray = traceeval_result_indx_key_array(teval, i);
+		if (!karray)
+			die("No process key for result %d\n", i);
+		tkey = traceeval_key_array_indx(karray, 0);
+		if (!tkey)
+			die("No process keys for result?");
+
+		/*
+		 * All the comms should be together in the results,
+		 * as the results are sorted by the first key, which
+		 * is the comm.
+		 */
+		if (last_comm && strcmp(tkey->string, last_comm) == 0)
+			continue;
+
+		last_comm = tkey->string;
+
+		keys[0] = *tkey;
+		keys[1].number = RUNNING;
+
+		/* All processes should have a running state */
+		pdata = traceeval_n_get_private(teval, keys);
+		if (pdata)
+			display_process(teval, pdata, keys[0].string);
+	}
+}
+
+static void display(struct task_data *tdata)
+{
+	unsigned long long total_time = 0;
+	unsigned long long idle_time = 0;
+	struct traceeval_key_array *karray;
+	const struct traceeval_key *tkey;
+	unsigned long long val;
+	int i, nr;
+
+	if (tdata->comm)
+		return display_processes(tdata->teval_processes);
+
+	printf("Total:\n");
+
+	nr = traceeval_result_nr(tdata->teval_cpus);
+	for (i = 0; i < nr; i++) {
+		karray = traceeval_result_indx_key_array(tdata->teval_cpus, i);
+		if (!karray)
+			die("No CPU keys for result %d\n", i);
+		tkey = traceeval_key_array_indx(karray, 1);
+		if (!tkey)
+			die("No state keys for CPU result %d?", i);
+
+		val = traceeval_result_indx_total(tdata->teval_cpus, i);
+		switch (tkey->number) {
+		case RUNNING:
+			total_time += val;
+			break;
+		case IDLE:
+			idle_time += val;
+			break;
+		default:
+			die("Invalid CPU state: %d\n", tkey->number);
+		}
+	}
+
+	printf("  Total  run time (us):");
+	print_microseconds(16, total_time);
+	printf("  Total idle time (us):");
+	print_microseconds(16, idle_time);
+
+	display_cpus(tdata->teval_cpus);
+
+	traceeval_sort_custom(tdata->teval_processes, compare_pdata, NULL);
+
+	printf("\n");
+	display_processes(tdata->teval_processes);
+}
+
+static void free_tdata(struct task_data *tdata)
+{
+}
+
+int main (int argc, char **argv)
+{
+	struct tracecmd_input *handle;
+	struct task_data data;
+	struct traceeval_key_info cpu_tinfo[2] = {
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.name = "CPU"
+		},
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.name = "Schedule state"
+		}
+	};
+	struct traceeval_key_info process_tinfo[2] = {
+		{
+			.type = TRACEEVAL_TYPE_STRING,
+			.name = "COMM"
+		},
+		{
+			.type = TRACEEVAL_TYPE_NUMBER,
+			.name = "Schedule state"
+		}
+	};
+	int c;
+
+	memset(&data, 0, sizeof(data));
+
+	argv0 = argv[0];
+
+	while ((c = getopt(argc, argv, "c:h")) >= 0) {
+		switch (c) {
+		case 'c':
+			data.comm = optarg;
+			break;
+		case 'h':
+		default:
+			usage();
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+
+	if (argc < 1)
+		usage();
+
+	handle = tracecmd_open(argv[0], TRACECMD_FL_LOAD_NO_PLUGINS);
+	if (!handle)
+		pdie("Error opening %s", argv[0]);
+
+	data.teval_processes = traceeval_2_alloc("Processes", process_tinfo);
+	if (!data.teval_processes)
+		pdie("Creating trace eval");
+
+	data.teval_cpus = traceeval_2_alloc("CPUs", cpu_tinfo);
+	if (!data.teval_cpus)
+		pdie("Creating trace eval");
+
+	tracecmd_follow_event(handle, "sched", "sched_switch", switch_func, &data);
+
+	tracecmd_iterate_events(handle, NULL, 0, NULL, NULL);
+
+	display(&data);
+
+	free_tdata(&data);
+
+	return 0;
+}
-- 
2.40.1

[PATCH 2/6] libtraceeval hist: Add pointer and const string types

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Add a pointer type to traceeval_data, as this can be used as a generic
pointer type. This may even obsolete the dynamic type.

Also, add a const char * "cstring" type. There's times where the key and
value data needs to be assigned to a const char *string, and without
having an option of that type, the compiler complains about losing the
const.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/traceeval-hist.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
index e713c70e3fa3..03e050cdbed4 100644
--- a/include/traceeval-hist.h
+++ b/include/traceeval-hist.h
@@ -25,6 +25,7 @@ enum traceeval_data_type {
 	TRACEEVAL_TYPE_NUMBER_16,
 	TRACEEVAL_TYPE_NUMBER_8,
 	TRACEEVAL_TYPE_NUMBER,
+	TRACEEVAL_TYPE_POINTER,
 	TRACEEVAL_TYPE_STRING,
 	TRACEEVAL_TYPE_DYNAMIC
 };
@@ -41,7 +42,9 @@ enum traceeval_flags {
  */
 union traceeval_data {
 	char				*string;
+	const char			*cstring;
 	struct traceeval_dynamic	*dyn_data;
+	void				*pointer;
 	unsigned long long		number_64;
 	unsigned long			number;
 	unsigned int			number_32;
-- 
2.40.1

[PATCH 3/6] libtraceeval histogram: Have cmp and release functions be generic

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Having the ability to override the compare function for a given type can
be very advantageous. There's also no reason that any type could ask for a
release callback to be called when the type is being released. It could be
used for information as well as for freeing.

Rename traceeval_dyn_cmp_fn to traceeval_data_cmp_fn and
 traceeval_dyn_release_fn to traceeval_data_release_fn and have
them take the union traceeval_type instead of struct traceeval_dynamic.

In the structure, rename dyn_cmp to just cmp, and dyn_release to just
release.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/traceeval-hist.h | 33 +++++++++++++++------------------
 src/histograms.c         | 26 ++++++++++++++------------
 2 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
index 03e050cdbed4..996fcab1b92b 100644
--- a/include/traceeval-hist.h
+++ b/include/traceeval-hist.h
@@ -65,13 +65,13 @@ struct traceeval_dynamic {
 struct traceeval_type;
 
 /* struct traceeval_dynamic release function signature */
-typedef void (*traceeval_dyn_release_fn)(struct traceeval_type *,
-					 struct traceeval_dynamic *);
+typedef void (*traceeval_data_release_fn)(struct traceeval_type *,
+					  union traceeval_data *);
 
 /* struct traceeval_dynamic compare function signature */
-typedef int (*traceeval_dyn_cmp_fn)(struct traceeval_dynamic *,
-				    struct traceeval_dynamic *,
-				    struct traceeval_type *);
+typedef int (*traceeval_data_cmp_fn)(const union traceeval_data *,
+				     const union traceeval_data *,
+				     struct traceeval_type *);
 
 /*
  * struct traceeval_type - Describes the type of a traceevent_data instance
@@ -79,8 +79,8 @@ typedef int (*traceeval_dyn_cmp_fn)(struct traceeval_dynamic *,
  * @name: The string name of the traceeval_data
  * @flags: flags to describe the traceeval_data
  * @id: User specified identifier
- * @dyn_release: For dynamic types called on release (ignored for other types)
- * @dyn_cmp: A way to compare dynamic types (ignored for other types)
+ * @data_release: An optional callback for when the data is being released
+ * @data_cmp: An optional callback to specify a way to compare the type
  *
  * The traceeval_type structure defines expectations for a corresponding
  * traceeval_data instance for a traceeval histogram instance. Used to
@@ -91,29 +91,26 @@ typedef int (*traceeval_dyn_cmp_fn)(struct traceeval_dynamic *,
  * which each relate to distinct user defined struct traceeval_dynamic
  * 'sub-types'.
  *
- * For flexibility, @dyn_cmp() and @dyn_release() take a struct
+ * For flexibility, @data_cmp() and @data_release() take a struct
  * traceeval_type instance. This allows the user to distinguish between
  * different sub-types of struct traceeval_dynamic within a single
  * callback function by examining the @id field. This is not a required
  * approach, merely one that is accommodated.
  *
- * @dyn_cmp() is used to compare two struct traceeval_dynamic instances when a
- * corresponding struct traceeval_type is reached with its type field set to
- * TRACEEVAL_TYPE_DYNAMIC. It should return 0 on equality, 1 if the first
- * argument is greater than the second, -1 for the other way around, and -2 on
- * error.
+ * @data_cmp() is used to override the default compare of a type. This is
+ * required to compare types of TRACEEVAL_TYPE_DYNAMIC. It should return 0
+ * on equality, 1 if the first argument is greater than the second,
+ * -1 for the other way around, and -2 on error.
  *
- * dyn_release() is used during traceeval_release() to release a union
- * traceeval_data's struct traceeval_dynamic field when the corresponding
- * traceeval_type type is set to TRACEEVAL_TYPE_DYNAMIC.
+ * data_release() is called when a data element is being released (or freed).
  */
 struct traceeval_type {
 	char				*name;
 	enum traceeval_data_type	type;
 	size_t				flags;
 	size_t				id;
-	traceeval_dyn_release_fn	dyn_release;
-	traceeval_dyn_cmp_fn		dyn_cmp;
+	traceeval_data_release_fn	release;
+	traceeval_data_cmp_fn		cmp;
 };
 
 /* Statistics about a given entry element */
diff --git a/src/histograms.c b/src/histograms.c
index a59542afdea1..f35fccb1d4fe 100644
--- a/src/histograms.c
+++ b/src/histograms.c
@@ -89,9 +89,9 @@ static int compare_traceeval_type(struct traceeval_type *orig,
 			return 1;
 		if (orig[i].id != copy[i].id)
 			return 1;
-		if (orig[i].dyn_release != copy[i].dyn_release)
+		if (orig[i].release != copy[i].release)
 			return 1;
-		if (orig[i].dyn_cmp != copy[i].dyn_cmp)
+		if (orig[i].cmp != copy[i].cmp)
 			return 1;
 
 		// make sure both names are same type
@@ -127,6 +127,9 @@ static int compare_traceeval_data(union traceeval_data *orig,
 	if (!copy)
 		return 1;
 
+	if (type->cmp)
+		return type->cmp(orig, copy, type);
+
 	switch (type->type) {
 	case TRACEEVAL_TYPE_STRING:
 		i = strcmp(orig->string, copy->string);
@@ -148,8 +151,7 @@ static int compare_traceeval_data(union traceeval_data *orig,
 		compare_numbers_return(orig->number_8, copy->number_8);
 
 	case TRACEEVAL_TYPE_DYNAMIC:
-		if (type->dyn_cmp)
-			return type->dyn_cmp(orig->dyn_data, copy->dyn_data, type);
+		/* If it didn't specify a cmp function, then punt */
 		return 0;
 
 	default:
@@ -235,8 +237,8 @@ static int compare_hist(struct traceeval *orig, struct traceeval *copy)
  *
  * This compares the values of the key definitions, value definitions, and
  * inserted data between @orig and @copy in order. It does not compare
- * by memory address, except for struct traceeval_type's dyn_release() and
- * dyn_cmp() fields.
+ * by memory address, except for struct traceeval_type's release() and
+ * cmp() fields.
  *
  * Returns 0 if @orig and @copy are the same, 1 if not, and -1 on error.
  */
@@ -450,14 +452,14 @@ static void clean_data(union traceeval_data *data, struct traceeval_type *defs,
 	}
 
 	for (i = 0; i < size; i++) {
+		if (defs[i].release) {
+			defs[i].release(defs + i, data + i);
+			continue;
+		}
 		switch (defs[i].type) {
 		case TRACEEVAL_TYPE_STRING:
 			free(data[i].string);
 			break;
-		case TRACEEVAL_TYPE_DYNAMIC:
-			if (defs[i].dyn_release)
-				defs[i].dyn_release(defs + i, data[i].dyn_data);
-			break;
 		default:
 			break;
 		}
@@ -506,7 +508,7 @@ static void hist_table_release(struct traceeval *teval)
  * it must call traceeval_release().
  *
  * This frees all internally allocated data of @teval and will call the
- * corresponding dyn_release() functions registered for keys and values of
+ * corresponding release() functions registered for keys and values of
  * type TRACEEVAL_TYPE_DYNAMIC.
  */
 void traceeval_release(struct traceeval *teval)
@@ -588,7 +590,7 @@ static int copy_traceeval_data(struct traceeval_type *type,
 /*
  * Free @data with respect to @size and @type.
  *
- * Does not call dyn_release on type TRACEEVAL_TYPE_DYNAMIC.
+ * Does not call release on type TRACEEVAL_TYPE_DYNAMIC.
  */
 static void data_release(size_t size, union traceeval_data **data,
 				struct traceeval_type *type)
-- 
2.40.1

[PATCH 4/6] libtraceeval histograms: Add traceeval struct to compare function

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When looking at how this code would be implemented, I found that I needed
access to the traceeval structure within the compare callbacks. Pass that
in too.

Also, rearrange the parameters a little. The traceeval and type should go
first as they describe the "object", and the data should last as they are
the values of the function.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/traceeval-hist.h | 13 +++++++------
 src/histograms.c         | 18 ++++++++++--------
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
index 996fcab1b92b..b3427c662d99 100644
--- a/include/traceeval-hist.h
+++ b/include/traceeval-hist.h
@@ -63,16 +63,17 @@ struct traceeval_dynamic {
 };
 
 struct traceeval_type;
+struct traceeval;
 
 /* struct traceeval_dynamic release function signature */
-typedef void (*traceeval_data_release_fn)(struct traceeval_type *,
-					  union traceeval_data *);
+typedef void (*traceeval_data_release_fn)(const struct traceeval_type *type,
+					  union traceeval_data *data);
 
 /* struct traceeval_dynamic compare function signature */
-typedef int (*traceeval_data_cmp_fn)(const union traceeval_data *,
-				     const union traceeval_data *,
-				     struct traceeval_type *);
-
+typedef int (*traceeval_data_cmp_fn)(struct traceeval *teval,
+				     const struct traceeval_type *type,
+				     const union traceeval_data *A,
+				     const union traceeval_data *B);
 /*
  * struct traceeval_type - Describes the type of a traceevent_data instance
  * @type: The enum type that describes the traceeval_data
diff --git a/src/histograms.c b/src/histograms.c
index f35fccb1d4fe..3cf5c5389700 100644
--- a/src/histograms.c
+++ b/src/histograms.c
@@ -112,7 +112,8 @@ static int compare_traceeval_type(struct traceeval_type *orig,
  * Return 0 if @orig and @copy are the same, 1 if @orig is greater than @copy,
  * -1 for the other way around, and -2 on error.
  */
-static int compare_traceeval_data(union traceeval_data *orig,
+static int compare_traceeval_data(struct traceeval *teval,
+				  union traceeval_data *orig,
 				  const union traceeval_data *copy,
 				  struct traceeval_type *type)
 {
@@ -128,7 +129,7 @@ static int compare_traceeval_data(union traceeval_data *orig,
 		return 1;
 
 	if (type->cmp)
-		return type->cmp(orig, copy, type);
+		return type->cmp(teval, type, orig, copy);
 
 	switch (type->type) {
 	case TRACEEVAL_TYPE_STRING:
@@ -166,7 +167,8 @@ static int compare_traceeval_data(union traceeval_data *orig,
  *
  * Return 0 if @orig and @copy are the same, 1 if not, and -1 on error.
  */
-static int compare_traceeval_data_set(union traceeval_data *orig,
+static int compare_traceeval_data_set(struct traceeval *teval,
+				      union traceeval_data *orig,
 				      const union traceeval_data *copy,
 				      struct traceeval_type *defs, size_t size)
 {
@@ -175,7 +177,7 @@ static int compare_traceeval_data_set(union traceeval_data *orig,
 
 	/* compare data arrays */
 	for (i = 0; i < size; i++) {
-		if ((check = compare_traceeval_data(orig + i, copy + i, defs + i)))
+		if ((check = compare_traceeval_data(teval, orig + i, copy + i, defs + i)))
 			return check == -2 ? -1 : 1;
 	}
 
@@ -191,13 +193,13 @@ static int compare_entries(struct entry *orig, struct entry *copy,
 	int check;
 
 	/* compare keys */
-	check = compare_traceeval_data_set(orig->keys, copy->keys,
+	check = compare_traceeval_data_set(teval, orig->keys, copy->keys,
 			teval->key_types, teval->nr_key_types);
 	if (check)
 		return check;
 
 	/* compare values */
-	check = compare_traceeval_data_set(orig->vals, copy->vals,
+	check = compare_traceeval_data_set(teval, orig->vals, copy->vals,
 			teval->val_types, teval->nr_val_types);
 	return check;
 }
@@ -543,8 +545,8 @@ static int get_entry(struct traceeval *teval, const union traceeval_data *keys,
 	i = 0;
 	for (struct entry *entry = hist->map; i < hist->nr_entries;
 			entry = &hist->map[++i]) {
-		check = compare_traceeval_data_set(entry->keys, keys,
-				teval->key_types, teval->nr_key_types);
+		check = compare_traceeval_data_set(teval, entry->keys, keys,
+						   teval->key_types, teval->nr_key_types);
 
 		/* return entry if keys match */
 		if (!check) {
-- 
2.40.1

[PATCH 5/6] libtraceeval histogram: Fix the return value of traceeval_query()

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The comments state that traceeval_query() returns 1 if found, 0 if not,
and -1 on error, but in reality it returns 0 if found and 1 if not found.

It makes more sense to have it return 1 if found and zero if not found as
the comment states.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 src/histograms.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/histograms.c b/src/histograms.c
index 3cf5c5389700..226c2792995c 100644
--- a/src/histograms.c
+++ b/src/histograms.c
@@ -551,7 +551,7 @@ static int get_entry(struct traceeval *teval, const union traceeval_data *keys,
 		/* return entry if keys match */
 		if (!check) {
 			*result = entry;
-			return 0;
+			return 1;
 		} else if (check == 1) {
 			continue;
 		} else {
@@ -559,7 +559,7 @@ static int get_entry(struct traceeval *teval, const union traceeval_data *keys,
 		}
 	}
 
-	return 1;
+	return 0;
 }
 
 /*
@@ -660,7 +660,7 @@ int traceeval_query(struct traceeval *teval, const union traceeval_data *keys,
 		return -1;
 
 	/* find key and copy its corresponding value pair */
-	if ((check = get_entry(teval, keys, &entry)))
+	if ((check = get_entry(teval, keys, &entry)) < 0)
 		return check;
 	return copy_traceeval_data_set(teval->nr_val_types, teval->val_types,
 			entry->vals, results);
-- 
2.40.1

[PATCH 6/6] libtraceeval samples: Update task-eval to use the histogram logic

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Convert task-eval over to the new API.

Note, it still requires some functions for displaying the output, but
those were added just as stubs to get an idea on how to use it.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/traceeval-hist.h |   1 +
 samples/Makefile         |   2 +-
 samples/task-eval.c      | 683 ++++++++++++++++++++++-----------------
 3 files changed, 388 insertions(+), 298 deletions(-)

diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
index b3427c662d99..c363444f7fae 100644
--- a/include/traceeval-hist.h
+++ b/include/traceeval-hist.h
@@ -74,6 +74,7 @@ typedef int (*traceeval_data_cmp_fn)(struct traceeval *teval,
 				     const struct traceeval_type *type,
 				     const union traceeval_data *A,
 				     const union traceeval_data *B);
+
 /*
  * struct traceeval_type - Describes the type of a traceevent_data instance
  * @type: The enum type that describes the traceeval_data
diff --git a/samples/Makefile b/samples/Makefile
index 012301ec5542..eb14411189f6 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -19,7 +19,7 @@ $(sdir):
 
 $(TARGETS): $(sdir) $(LIBTRACEEVAL_STATIC)
 
-$(sdir)/%: $(bdir)/%.o
+$(sdir)/%: $(LIBTRACEEVAL_STATIC) $(bdir)/%.o
 	$(call do_sample_build,$@,$<)
 
 $(bdir)/%.o: $(bdir)/%.c
diff --git a/samples/task-eval.c b/samples/task-eval.c
index 26e48c376f45..f5515df9fef2 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -5,7 +5,7 @@
 #include <getopt.h>
 #include <errno.h>
 #include <trace-cmd.h>
-#include <traceeval.h>
+#include <traceeval-hist.h>
 
 static char *argv0;
 
@@ -80,6 +80,83 @@ void pdie(const char *fmt, ...)
 	va_end(ap);
 }
 
+static struct traceeval_type cpu_keys[] = {
+	{
+		.type = TRACEEVAL_TYPE_NUMBER,
+		.name = "CPU",
+	},
+	{
+		.type = TRACEEVAL_TYPE_NUMBER,
+		.name = "Schedule state",
+	},
+	{
+		.type = TRACEEVAL_TYPE_NONE
+	}
+};
+
+static struct traceeval_type process_keys[] = {
+	{
+		.type = TRACEEVAL_TYPE_STRING,
+		.name = "COMM"
+	},
+	{
+		.type = TRACEEVAL_TYPE_NUMBER,
+		.name = "Schedule state"
+	},
+	{
+		.type	= TRACEEVAL_TYPE_NONE,
+	}
+};
+
+static struct traceeval_type process_vals[] = {
+	{
+		.type = TRACEEVAL_TYPE_NUMBER_64,
+		.name = "delta",
+	},
+	{
+		.type = TRACEEVAL_TYPE_POINTER,
+		.name = "data",
+	},
+	{
+		.type = TRACEEVAL_TYPE_NONE
+	}
+};
+
+static struct traceeval_type thread_keys[] = {
+	{
+		.type = TRACEEVAL_TYPE_NUMBER,
+		.name = "TID",
+	},
+	{
+		.type = TRACEEVAL_TYPE_NUMBER,
+		.name = "Schedule state",
+	},
+	{
+		.type = TRACEEVAL_TYPE_NONE,
+	}
+};
+
+static struct traceeval_type timestamp_vals[] = {
+	{
+		.type = TRACEEVAL_TYPE_NUMBER_64,
+		.name = "Timestamp",
+		.flags = TRACEEVAL_FL_TIMESTAMP,
+	},
+	{
+		.type = TRACEEVAL_TYPE_NONE
+	}
+};
+
+static struct traceeval_type delta_vals[] = {
+	{
+		.type	= TRACEEVAL_TYPE_NUMBER_64,
+		.name	= "delta",
+	},
+	{
+		.type	= TRACEEVAL_TYPE_NONE,
+	},
+};
+
 enum sched_state {
 	RUNNING,
 	BLOCKED,
@@ -90,14 +167,18 @@ enum sched_state {
 };
 
 struct process_data {
+	struct traceeval	*teval_cpus_start;
 	struct traceeval	*teval_cpus;
+	struct traceeval	*teval_threads_start;
 	struct traceeval	*teval_threads;
 	char			*comm;
 	int			state;
 };
 
 struct task_data {
+	struct traceeval	*teval_cpus_start;
 	struct traceeval	*teval_cpus;
+	struct traceeval	*teval_processes_start;
 	struct traceeval	*teval_processes;
 	char			*comm;
 };
@@ -111,28 +192,45 @@ static void update_process(struct task_data *tdata, const char *comm,
 			   enum sched_state state, enum command cmd,
 			   unsigned long long ts)
 {
-	struct traceeval_key keys[] = {
+	union traceeval_data keys[] = {
 		{
-			.type = TRACEEVAL_TYPE_STRING,
-			.string = comm,
+			.cstring	= comm,
 		},
 		{
-			.type = TRACEEVAL_TYPE_NUMBER,
-			.number = state,
+			.number		= state,
+		},
+	};
+	union traceeval_data vals[] = {
+		{
+			.number_64	= ts,
+		},
+		{
+			.number		= (long)NULL, /* data */
 		}
 	};
+	union traceeval_data *results;
+	unsigned long long delta;
 	int ret;
 
 	switch (cmd) {
 	case START:
-		ret = traceeval_n_start(tdata->teval_processes, keys, ts);
+		ret = traceeval_insert(tdata->teval_processes_start, keys, vals);
 		if (ret < 0)
 			pdie("Could not start process");
 		return;
 	case STOP:
-		ret = traceeval_n_stop(tdata->teval_processes, keys, ts);
+		ret = traceeval_query(tdata->teval_processes_start, keys, &results);
+		if (ret < 0)
+			return;
+
+		delta = ts - results[0].number_64;
+		results[0].number_64 = delta;
+
+		ret = traceeval_insert(tdata->teval_processes, keys, results);
 		if (ret < 0)
 			pdie("Could not stop process");
+
+		traceeval_results_release(tdata->teval_processes_start, results);
 		return;
 	}
 }
@@ -152,105 +250,145 @@ static void stop_process(struct task_data *tdata, const char *comm,
 static struct process_data *
 get_process_data(struct task_data *tdata, const char *comm)
 {
-	struct traceeval_key keys[] = {
+	union traceeval_data keys[] = {
 		{
-			.type = TRACEEVAL_TYPE_STRING,
-			.string = comm,
+			.cstring = comm,
 		},
 		{
-			.type = TRACEEVAL_TYPE_NUMBER,
 			.number = RUNNING,
 		}
 	};
-					     
-	return traceeval_n_get_private(tdata->teval_processes, keys);
+	union traceeval_data *results;
+	void *data;
+	int ret;
+
+	ret = traceeval_query(tdata->teval_processes, keys, &results);
+	if (ret < 0)
+		return NULL;
+
+	data = (void *)results[1].number;
+	traceeval_results_release(tdata->teval_processes, results);
+	return data;
 }
 
 void set_process_data(struct task_data *tdata, const char *comm, void *data)
 {
-	struct traceeval_key keys[] = {
+	union traceeval_data keys[] = {
 		{
-			.type = TRACEEVAL_TYPE_STRING,
-			.string = comm,
+			.cstring = comm,
 		},
 		{
-			.type = TRACEEVAL_TYPE_NUMBER,
 			.number = RUNNING,
 		}
 	};
+	union traceeval_data *results;
 	int ret;
-					     
-	ret = traceeval_n_set_private(tdata->teval_processes, keys, data);
+
+	ret = traceeval_query(tdata->teval_processes_start, keys, &results);
+	if (ret < 0)
+		return;
+
+	results[1].number = (long)data;
+	ret = traceeval_insert(tdata->teval_processes_start, keys, results);
 	if (ret < 0)
 		pdie("Failed to set process data");
+
+	traceeval_results_release(tdata->teval_processes_start, results);
 }
 
-static void update_cpu(struct traceeval *teval, int cpu,
+static void update_cpu(struct traceeval *teval_start,
+		       struct traceeval *teval_stop, int cpu,
 		       enum sched_state state, enum command cmd,
 		       unsigned long long ts)
 {
-	struct traceeval_key keys[] = {
+	union traceeval_data *results;
+	unsigned long long delta;
+	union traceeval_data keys[] = {
 		{
-			.type = TRACEEVAL_TYPE_NUMBER,
 			.number = cpu,
 		},
 		{
-			.type = TRACEEVAL_TYPE_NUMBER,
 			.number = state,
 		}
 	};
+	union traceeval_data vals[] = {
+		{
+			.number_64	= ts,
+		},
+	};
 	int ret;
 
 	switch (cmd) {
 	case START:
-		ret = traceeval_n_continue(teval, keys, ts);
+		ret = traceeval_insert(teval_start, keys, vals);
 		if (ret < 0)
 			pdie("Could not start CPU");
 		return;
 	case STOP:
-		ret = traceeval_n_stop(teval, keys, ts);
+		ret = traceeval_query(teval_start, keys, &results);
+		if (ret < 0)
+			return;
+
+		delta = ts - results[0].number_64;
+		results[0].number_64 = delta;
+
+		ret = traceeval_insert(teval_stop, keys, results);
+		traceeval_results_release(teval_start, results);
 		if (ret < 0)
 			pdie("Could not stop CPU");
 		return;
 	}
 }
 
-static void start_cpu(struct traceeval *teval, int cpu,
-		      enum sched_state state,  unsigned long long ts)
+static void start_cpu(struct traceeval *teval_start, struct traceeval *teval,
+		      int cpu, enum sched_state state,  unsigned long long ts)
 {
-	update_cpu(teval, cpu, state, START, ts);
+	update_cpu(teval_start, teval, cpu, state, START, ts);
 }
 
-static void stop_cpu(struct traceeval *teval, int cpu,
-		     enum sched_state state, unsigned long long ts)
+static void stop_cpu(struct traceeval *teval_start, struct traceeval *teval,
+		     int cpu, enum sched_state state, unsigned long long ts)
 {
-	update_cpu(teval, cpu, state, STOP, ts);
+	update_cpu(teval_start, teval, cpu, state, STOP, ts);
 }
 
 static void update_thread(struct process_data *pdata, int tid,
 			  enum sched_state state, enum command cmd,
 			  unsigned long long ts)
 {
-	struct traceeval_key keys[] = {
+	union traceeval_data *results;
+	unsigned long long delta;
+	union traceeval_data keys[] = {
 		{
-			.type = TRACEEVAL_TYPE_NUMBER,
 			.number = tid,
 		},
 		{
-			.type = TRACEEVAL_TYPE_NUMBER,
 			.number = state,
 		}
 	};
+	union traceeval_data vals[] = {
+		{
+			.number_64	= ts,
+		},
+	};
 	int ret;
 
 	switch (cmd) {
 	case START:
-		ret = traceeval_n_start(pdata->teval_threads, keys, ts);
+		ret = traceeval_insert(pdata->teval_threads_start, keys, vals);
 		if (ret < 0)
 			pdie("Could not start thread");
 		return;
 	case STOP:
-		ret = traceeval_n_stop(pdata->teval_threads, keys, ts);
+		ret = traceeval_query(pdata->teval_threads_start, keys, &results);
+		if (ret < 0)
+			return;
+
+		delta = ts - results[0].number_64;
+		results[0].number_64 = delta;
+
+		ret = traceeval_insert(pdata->teval_threads, keys, results);
+		traceeval_results_release(pdata->teval_threads_start, results);
 		if (ret < 0)
 			pdie("Could not stop thread");
 		return;
@@ -283,32 +421,15 @@ static struct tep_format_field *get_field(struct tep_event *event, const char *n
 
 static void init_process_data(struct process_data *pdata)
 {
-	struct traceeval_key_info cpu_info[] = {
-		{
-			.type = TRACEEVAL_TYPE_NUMBER,
-			.name = "CPU",
-		},
-		{
-			.type = TRACEEVAL_TYPE_NUMBER,
-			.name = "Schedule state",
-		}
-	};
-	struct traceeval_key_info thread_info[] = {
-		{
-			.type = TRACEEVAL_TYPE_NUMBER,
-			.name = "TID",
-		},
-		{
-			.type = TRACEEVAL_TYPE_NUMBER,
-			.name = "Schedule state",
-		}
-	};
 
-	pdata->teval_cpus = traceeval_2_alloc("CPUs", cpu_info);
+	pdata->teval_cpus = traceeval_init(cpu_keys, timestamp_vals);
+	if (!pdata->teval_cpus)
+		pdie("Creating trace eval");
+	pdata->teval_cpus = traceeval_init(cpu_keys, timestamp_vals);
 	if (!pdata->teval_cpus)
 		pdie("Creating trace eval");
 
-	pdata->teval_threads = traceeval_2_alloc("Threads", thread_info);
+	pdata->teval_threads = traceeval_init(thread_keys, timestamp_vals);
 	if (!pdata->teval_threads)
 		pdie("Creating trace eval");
 }
@@ -344,7 +465,8 @@ static void sched_out(struct task_data *tdata, const char *comm,
 	pid = val;
 	if (!pid) {
 		/* Record the runtime for the process CPUs */
-		stop_cpu(tdata->teval_cpus, record->cpu, IDLE, record->ts);
+		stop_cpu(tdata->teval_cpus_start, tdata->teval_cpus,
+			 record->cpu, IDLE, record->ts);
 		return;
 	}
 
@@ -381,10 +503,12 @@ static void sched_out(struct task_data *tdata, const char *comm,
 	start_thread(pdata, pid, pdata->state, record->ts);
 
 	/* Record the runtime for the process CPUs */
-	stop_cpu(pdata->teval_cpus, record->cpu, RUNNING, record->ts);
+	stop_cpu(pdata->teval_cpus_start, pdata->teval_cpus,
+		 record->cpu, RUNNING, record->ts);
 
 	/* Record the runtime for the all CPUs */
-	stop_cpu(tdata->teval_cpus, record->cpu, RUNNING, record->ts);
+	stop_cpu(tdata->teval_cpus_start, tdata->teval_cpus,
+		 record->cpu, RUNNING, record->ts);
 }
 
 static void sched_in(struct task_data *tdata, const char *comm,
@@ -405,7 +529,8 @@ static void sched_in(struct task_data *tdata, const char *comm,
 	/* Ignore the idle task */
 	if (!pid) {
 		/* Record the runtime for the process CPUs */
-		start_cpu(tdata->teval_cpus, record->cpu, IDLE, record->ts);
+		start_cpu(tdata->teval_cpus_start, tdata->teval_cpus,
+			  record->cpu, IDLE, record->ts);
 		return;
 	}
 
@@ -415,7 +540,8 @@ static void sched_in(struct task_data *tdata, const char *comm,
 	pdata = get_process_data(tdata, comm);
 
 	/* Start recording the running time of process CPUs */
-	start_cpu(tdata->teval_cpus, record->cpu, RUNNING, record->ts);
+	start_cpu(tdata->teval_cpus_start, tdata->teval_cpus,
+		  record->cpu, RUNNING, record->ts);
 
 	/* If there was no pdata, then this process did not go through sched out */
 	if (!pdata) {
@@ -427,7 +553,8 @@ static void sched_in(struct task_data *tdata, const char *comm,
 	start_thread(pdata, pid, RUNNING, record->ts);
 
 	/* Start recording the running time of process CPUs */
-	start_cpu(pdata->teval_cpus, record->cpu, RUNNING, record->ts);
+	start_cpu(pdata->teval_cpus_start, pdata->teval_cpus,
+		  record->cpu, RUNNING, record->ts);
 
 	/* If it was just created, there's nothing to stop */
 	if (is_new)
@@ -482,32 +609,108 @@ static void print_microseconds(int idx, unsigned long long nsecs)
 		printf("%*d.%03lld\n", idx, 0, nsecs);
 }
 
+/* TODO */
+
+typedef int (*traceeval_cmp_fn)(struct traceeval *teval,
+				const union traceeval_data *Akeys,
+				const union traceeval_data *Avals,
+				const union traceeval_data *Bkeys,
+				const union traceeval_data *Bvals,
+				void *data);
+
+struct traceeval_iterator;
+struct traceeval_iterator *traceeval_iterator_get(struct traceeval *teval)
+{ return NULL; }
+
+int traceeval_iterator_sort(struct traceeval_iterator *iter, const char *sort_field,
+			    int level, bool ascending) { return 0; }
+
+int traceeval_iterator_sort_custom(struct traceeval_iterator *iter,
+				   traceeval_cmp_fn, void *data) { return 0; };
+
+int traceeval_iterator_next(struct traceeval_iterator *iter,
+			    const union traceeval_data **keys) {return 0; }
+int traceeval_stat(struct traceeval *teval, const union traceeval_data *keys,
+		   const char *field, struct traceeval_stat *stat) { return 0; }
+
+/*
+ * Sort all the processes by the RUNNING state.
+ *  If A and B have the same COMM, then sort by state.
+ *  else
+ *    Find the RUNNNIG state for A and B
+ *    If the RUNNING state does not exist, it's considered -1
+ *  If RUNNING is equal, then sort by COMM.
+ */
+static int compare_pdata(struct traceeval *teval,
+				const union traceeval_data *Akeys,
+				const union traceeval_data *Avals,
+				const union traceeval_data *Bkeys,
+				const union traceeval_data *Bvals,
+				void *data)
+{
+	union traceeval_data keysA[] = {
+		{ .cstring = Akeys[0].cstring }, { .number = RUNNING } };
+	union traceeval_data keysB[] = {
+		{ .cstring = Bkeys[0].cstring }, { .number = RUNNING } };
+	struct traceeval_stat statA;;
+	struct traceeval_stat statB;
+	unsigned long long totalA = -1;
+	unsigned long long totalB = -1;
+	int ret;
+
+	/* First check if we are on the same task */
+	if (strcmp(Akeys[0].cstring, Bkeys[0].cstring) == 0) {
+		/* Sort decending */
+		if (Akeys[1].number > Bkeys[1].number)
+			return -1;
+		return Akeys[1].number != Bkeys[1].number;
+	}
+
+	/* Get the RUNNING values for both processes */
+	ret = traceeval_stat(teval, keysA, process_keys[1].name, &statA);
+	if (ret > 0)
+		totalA = statA.total;
+
+	ret = traceeval_stat(teval, keysB, process_keys[1].name, &statB);
+	if (ret > 0)
+		totalB = statB.total;
+
+	if (totalA < totalB)
+		return -1;
+	if (totalA > totalB)
+		return 1;
+
+	return strcmp(Akeys[0].cstring, Bkeys[0].cstring);
+}
+
 static void display_cpus(struct traceeval *teval)
 {
-	struct traceeval_key_array *karray;
-	const struct traceeval_key *ckey;
-	const struct traceeval_key *skey;
+	struct traceeval_iterator *iter = traceeval_iterator_get(teval);
+	const union traceeval_data *keys;
+	struct traceeval_stat stat;
 	int last_cpu = -1;
-	int i, nr;
+	int ret;
+
+	if (!iter)
+		pdie("Could not get iterator?");
 
 	printf("\n");
 
-	nr = traceeval_result_nr(teval);
-	if (!nr)
-		die("No result for CPUs\n");
+	traceeval_iterator_sort(iter, cpu_keys[0].name, 0, true);
+	traceeval_iterator_sort(iter, cpu_keys[1].name, 1, true);
 
-	for (i = 0; i < nr; i++) {
-		karray = traceeval_result_indx_key_array(teval, i);
-		if (!karray)
-			die("No cpu key for result %d\n", i);
-		ckey = traceeval_key_array_indx(karray, 0);
-		skey = traceeval_key_array_indx(karray, 1);
+	while (traceeval_iterator_next(iter, &keys) > 0) {
+		int state = keys[1].number;
+		int cpu = keys[0].number;
 
+		ret = traceeval_stat(teval, keys, delta_vals[0].name, &stat);
+		if (ret <= 0)
+			continue; // die?
 
-		if (last_cpu != ckey->number)
-			printf("    CPU [%d]:\n", (int)ckey->number);
+		if (last_cpu != cpu)
+			printf("    CPU [%d]:\n", cpu);
 
-		switch (skey->number) {
+		switch (state) {
 		case RUNNING:
 			printf("       Running: ");
 			break;
@@ -518,256 +721,158 @@ static void display_cpus(struct traceeval *teval)
 		case PREEMPT:
 		case SLEEP:
 		case OTHER:
-			printf("         \?\?(%ld): ", skey->number);
+			printf("         \?\?(%d): ", state);
 			break;
 		}
 		printf(" time (us):");
-		print_microseconds(12, traceeval_result_indx_total(teval, i));
+		print_microseconds(12, stat.total);
 
-		last_cpu = ckey->number;
+		last_cpu = cpu;
 	}
+
+	if (last_cpu < 0)
+		die("No result for CPUs\n");
+
 }
 
-static void display_thread(struct traceeval *teval, int tid)
-{
-	struct traceeval_key keys[2] =
-		{
-			{
-				.type = TRACEEVAL_TYPE_NUMBER,
-				.number = tid,
-			},
-			{
-				.type = TRACEEVAL_TYPE_NUMBER,
-				.number = RUNNING,
-			}
-		};
-	size_t ret;
-
-	printf("\n    thread id: %d\n", tid);
-
-	printf("      Total run time (us):");
-	print_microseconds(14, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
-
-	keys[1].number = PREEMPT;
-	printf("      Total preempt time (us):");
-	print_microseconds(10, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
-
-	keys[1].number = BLOCKED;
-	printf("      Total blocked time (us):");
-	print_microseconds(10, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
-
-	keys[1].number = SLEEP;
-	printf("      Total sleep time (us):");
-	print_microseconds(12, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
-};
+static void display_state_times(int state, unsigned long long total)
+{
+	switch (state) {
+	case RUNNING:
+		printf("      Total run time (us):");
+		print_microseconds(14, total);
+		break;
+	case BLOCKED:
+		printf("      Total blocked time (us):");
+		print_microseconds(10, total);
+	case PREEMPT:
+		printf("      Total preempt time (us):");
+		print_microseconds(10, total);
+	case SLEEP:
+		print_microseconds(12, total);
+	}
+}
 
 static void display_threads(struct traceeval *teval)
 {
-	struct traceeval_key_array *karray;
-	const struct traceeval_key *tkey;
-	struct traceeval_key keys[2];
+	struct traceeval_iterator *iter = traceeval_iterator_get(teval);
+	const union traceeval_data *keys;
+	struct traceeval_stat stat;
 	int last_tid = -1;
-	int i, nr;
+	int ret;
 
-	nr = traceeval_result_nr(teval);
-	if (!nr)
-		die("No result for threads\n");
+	traceeval_iterator_sort(iter, thread_keys[0].name, 0, true);
+	traceeval_iterator_sort(iter, thread_keys[1].name, 1, true);
 
-	memset(keys, 0, sizeof(keys));
-	keys[1].type = TRACEEVAL_TYPE_NUMBER;
+	while (traceeval_iterator_next(iter, &keys) > 0) {
+		int state = keys[1].number;
+		int tid = keys[0].number;
 
-	for (i = 0; i < nr; i++) {
-		karray = traceeval_result_indx_key_array(teval, i);
-		if (!karray)
-			die("No thread key for result %d\n", i);
-		tkey = traceeval_key_array_indx(karray, 0);
-		if (!tkey)
-			die("No thread keys for result?");
+		ret = traceeval_stat(teval, keys, delta_vals[0].name, &stat);
+		if (ret <= 0)
+			continue; // die?
 
-		/*
-		 * All the TIDS should be together in the results,
-		 * as the results are sorted by the first key, which
-		 * is the comm.
-		 */
-		if (last_tid == tkey->number)
-			continue;
+		if (last_tid != keys[0].number)
+			printf("\n    thread id: %d\n", tid);
 
-		last_tid = tkey->number;
+		last_tid = tid;
 
-		display_thread(teval, tkey->number);
+		display_state_times(state, stat.total);
 	}
+
+	if (last_tid < 0)
+		die("No result for threads\n");
+
 }
 
-static void display_process(struct traceeval *teval, struct process_data *pdata,
-			    const char *comm)
+static void display_process(struct process_data *pdata, const char *comm)
 {
-	struct traceeval_key keys[2] =
-		{
-			{
-				.type = TRACEEVAL_TYPE_STRING,
-				.string = comm,
-			},
-			{
-				.type = TRACEEVAL_TYPE_NUMBER,
-				.number = RUNNING,
-			}
-		};
-	size_t ret;
 
 	printf("Task: %s\n", comm);
 
-	printf("  Total run time (us):");
-	print_microseconds(18, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
-
-	keys[1].number = PREEMPT;
-	printf("  Total preempt time (us):");
-	print_microseconds(14, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
-
-	keys[1].number = BLOCKED;
-	printf("  Total blocked time (us):");
-	print_microseconds(14, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
-
-	keys[1].number = SLEEP;
-	printf("  Total sleep time (us):");
-	print_microseconds(16, (ret = traceeval_result_keys_total(teval, keys)) < 0 ? 0 : ret);
-
 	display_threads(pdata->teval_threads);
 	display_cpus(pdata->teval_cpus);
 	printf("\n");
 }
 
-static int compare_pdata(struct traceeval *teval,
-			 const struct traceeval_key_array *A,
-			 const struct traceeval_key_array *B,
-			 void *data)
-{
-	struct traceeval_key akeys[2];
-	struct traceeval_key bkeys[2];
-	const struct traceeval_key *akey;
-	const struct traceeval_key *bkey;
-	long long aval;
-	long long bval;
-	int ret;
-
-	/* Get the RUNNING values for this process */
-
-	akey = traceeval_key_array_indx(A, 0);
-	akeys[0] = *akey;
-	akeys[1].type = TRACEEVAL_TYPE_NUMBER;
-	akeys[1].number = RUNNING;
-
-	bkey = traceeval_key_array_indx(B, 0);
-	bkeys[0] = *bkey;
-	bkeys[1].type = TRACEEVAL_TYPE_NUMBER;
-	bkeys[1].number = RUNNING;
-
-	aval = traceeval_result_keys_total(teval, akey);
-	bval = traceeval_result_keys_total(teval, bkeys);
-
-	if (aval < 0)
-		return -1;
-	if (bval < 0)
-		return -1;
-
-	if (bval < aval)
-		return -1;
-	if (bval > aval)
-		return 1;
-
-	ret = strcmp(bkeys[0].string, akeys[0].string);
-
-	/* If two different processes have the same runtime, sort by name */
-	if (ret)
-		return ret;
-
-	/* Same process, sort by state */
-
-	akey = traceeval_key_array_indx(A, 1);
-	bkey = traceeval_key_array_indx(B, 1);
-
-	if (bkey->number < akey->number)
-		return -1;
-
-	return bkey->number > akey->number;
-}
-
 static void display_processes(struct traceeval *teval)
 {
-	struct traceeval_key_array *karray;
-	const struct traceeval_key *tkey;
-	struct traceeval_key keys[2];
-	struct process_data *pdata;
+	struct traceeval_iterator *iter = traceeval_iterator_get(teval);
+	const union traceeval_data *keys;
+	union traceeval_data *results;
+	struct traceeval_stat stat;
+	struct process_data *pdata = NULL;
 	const char *last_comm = NULL;
-	int i, nr;
-
-	nr = traceeval_result_nr(teval);
-	if (!nr)
-		die("No result for processes\n");
+	int ret;
 
-	memset(keys, 0, sizeof(keys));
-	keys[1].type = TRACEEVAL_TYPE_NUMBER;
+	traceeval_iterator_sort_custom(iter, compare_pdata, NULL);
 
-	for (i = 0; i < nr; i++) {
-		karray = traceeval_result_indx_key_array(teval, i);
-		if (!karray)
-			die("No process key for result %d\n", i);
-		tkey = traceeval_key_array_indx(karray, 0);
-		if (!tkey)
-			die("No process keys for result?");
+	while (traceeval_iterator_next(iter, &keys) > 0) {
+		const char *comm = keys[0].cstring;
+		int state = keys[1].number;
 
 		/*
-		 * All the comms should be together in the results,
-		 * as the results are sorted by the first key, which
-		 * is the comm.
+		 * All the process should be together as they are
+		 * by the running times and then by comms.
 		 */
-		if (last_comm && strcmp(tkey->string, last_comm) == 0)
-			continue;
+		if (!last_comm || strcmp(comm, last_comm) != 0) {
+			if (pdata)
+				display_process(pdata, last_comm);
 
-		last_comm = tkey->string;
+			printf("Task: %s\n", comm);
+		}
 
-		keys[0] = *tkey;
-		keys[1].number = RUNNING;
+		last_comm = comm;
 
-		/* All processes should have a running state */
-		pdata = traceeval_n_get_private(teval, keys);
-		if (pdata)
-			display_process(teval, pdata, keys[0].string);
+		ret = traceeval_stat(teval, keys, delta_vals[0].name, &stat);
+		if (ret > 0)
+			display_state_times(state, stat.total);
+
+		last_comm = comm;
+
+		if (state == RUNNING) {
+			ret = traceeval_query(teval, keys, &results);
+			if (ret < 0)
+				continue;
+			pdata = results[1].pointer;
+		}
 	}
 }
 
 static void display(struct task_data *tdata)
 {
+	struct traceeval *teval = tdata->teval_cpus;
+	struct traceeval_iterator *iter = traceeval_iterator_get(teval);
+	const union traceeval_data *keys;
+	struct traceeval_stat stat;
 	unsigned long long total_time = 0;
 	unsigned long long idle_time = 0;
-	struct traceeval_key_array *karray;
-	const struct traceeval_key *tkey;
-	unsigned long long val;
-	int i, nr;
+	int ret;
 
 	if (tdata->comm)
 		return display_processes(tdata->teval_processes);
 
 	printf("Total:\n");
 
-	nr = traceeval_result_nr(tdata->teval_cpus);
-	for (i = 0; i < nr; i++) {
-		karray = traceeval_result_indx_key_array(tdata->teval_cpus, i);
-		if (!karray)
-			die("No CPU keys for result %d\n", i);
-		tkey = traceeval_key_array_indx(karray, 1);
-		if (!tkey)
-			die("No state keys for CPU result %d?", i);
-
-		val = traceeval_result_indx_total(tdata->teval_cpus, i);
-		switch (tkey->number) {
+	if (!iter)
+		pdie("No cpus?");
+
+	while (traceeval_iterator_next(iter, &keys) > 0) {
+		int state = keys[1].number;
+
+		ret = traceeval_stat(teval, keys, delta_vals[0].name, &stat);
+		if (ret < 0)
+			continue;
+
+		switch (state) {
 		case RUNNING:
-			total_time += val;
+			total_time += stat.total;
 			break;
 		case IDLE:
-			idle_time += val;
+			idle_time += stat.total;
 			break;
 		default:
-			die("Invalid CPU state: %d\n", tkey->number);
+			die("Invalid CPU state: %d\n", state);
 		}
 	}
 
@@ -778,8 +883,6 @@ static void display(struct task_data *tdata)
 
 	display_cpus(tdata->teval_cpus);
 
-	traceeval_sort_custom(tdata->teval_processes, compare_pdata, NULL);
-
 	printf("\n");
 	display_processes(tdata->teval_processes);
 }
@@ -792,26 +895,6 @@ int main (int argc, char **argv)
 {
 	struct tracecmd_input *handle;
 	struct task_data data;
-	struct traceeval_key_info cpu_tinfo[2] = {
-		{
-			.type = TRACEEVAL_TYPE_NUMBER,
-			.name = "CPU"
-		},
-		{
-			.type = TRACEEVAL_TYPE_NUMBER,
-			.name = "Schedule state"
-		}
-	};
-	struct traceeval_key_info process_tinfo[2] = {
-		{
-			.type = TRACEEVAL_TYPE_STRING,
-			.name = "COMM"
-		},
-		{
-			.type = TRACEEVAL_TYPE_NUMBER,
-			.name = "Schedule state"
-		}
-	};
 	int c;
 
 	memset(&data, 0, sizeof(data));
@@ -839,11 +922,17 @@ int main (int argc, char **argv)
 	if (!handle)
 		pdie("Error opening %s", argv[0]);
 
-	data.teval_processes = traceeval_2_alloc("Processes", process_tinfo);
+	data.teval_processes_start = traceeval_init(process_keys, timestamp_vals);
+	if (!data.teval_processes_start)
+		pdie("Creating trace eval start");
+	data.teval_processes = traceeval_init(process_keys, process_vals);
 	if (!data.teval_processes)
 		pdie("Creating trace eval");
 
-	data.teval_cpus = traceeval_2_alloc("CPUs", cpu_tinfo);
+	data.teval_cpus_start = traceeval_init(cpu_keys, timestamp_vals);
+	if (!data.teval_cpus_start)
+		pdie("Creating trace eval");
+	data.teval_cpus = traceeval_init(cpu_keys, delta_vals);
 	if (!data.teval_cpus)
 		pdie("Creating trace eval");
 
-- 
2.40.1

Re: [PATCH 6/6] libtraceeval samples: Update task-eval to use the histogram logic

[Steven Rostedt]

On Tue,  8 Aug 2023 23:13:13 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> Convert task-eval over to the new API.
> 
> Note, it still requires some functions for displaying the output, but
> those were added just as stubs to get an idea on how to use it.

I should add that this is still RFC, and not ready at all for inclusion.

There's lots of issues with this patch (like I see I use .number still for
the data that's of POINTER type). These will all be fixed before they go
upstream.

Anyway, I posted this so that you can have an idea of a use case for how
this will work.

-- Steve


> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  include/traceeval-hist.h |   1 +
>  samples/Makefile         |   2 +-
>  samples/task-eval.c      | 683 ++++++++++++++++++++++-----------------
>  3 files changed, 388 insertions(+), 298 deletions(-)
> 

Re: [PATCH 5/6] libtraceeval histogram: Fix the return value of traceeval_query()

[Ross Zwisler]

On Tue, Aug 08, 2023 at 11:13:12PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> The comments state that traceeval_query() returns 1 if found, 0 if not,
> and -1 on error, but in reality it returns 0 if found and 1 if not found.

The comment currently says:

> /*
>  * Find the entry that @keys corresponds to within @teval.
>  *
>  * Returns 0 on success, 1 if no match found, -1 on error.
>  */

I think this needs to be updated to match the new logic (1=found, 0=not
found, -1=error)

> It makes more sense to have it return 1 if found and zero if not found as
> the comment states.
> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  src/histograms.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/histograms.c b/src/histograms.c
> index 3cf5c5389700..226c2792995c 100644
> --- a/src/histograms.c
> +++ b/src/histograms.c
> @@ -551,7 +551,7 @@ static int get_entry(struct traceeval *teval, const union traceeval_data *keys,
>  		/* return entry if keys match */
>  		if (!check) {
>  			*result = entry;
> -			return 0;
> +			return 1;
>  		} else if (check == 1) {
>  			continue;
>  		} else {
> @@ -559,7 +559,7 @@ static int get_entry(struct traceeval *teval, const union traceeval_data *keys,
>  		}
>  	}
>  
> -	return 1;
> +	return 0;
>  }
>  
>  /*
> @@ -660,7 +660,7 @@ int traceeval_query(struct traceeval *teval, const union traceeval_data *keys,
>  		return -1;
>  
>  	/* find key and copy its corresponding value pair */
> -	if ((check = get_entry(teval, keys, &entry)))
> +	if ((check = get_entry(teval, keys, &entry)) < 0)

should this be 
	if ((check = get_entry(teval, keys, &entry)) <= 0)

so we return 0 (not found) in that case, and avoid dropping into
copy_traceeval_data_set() when we have nothing to copy?

>  		return check;
>  	return copy_traceeval_data_set(teval->nr_val_types, teval->val_types,
>  			entry->vals, results);
> -- 
> 2.40.1
> 

I think you also need to update the logic in traceeval_insert(), which right
now uses a return of 1 to mean "not found" and will do a create_entry() in
response.

For clarity maybe in all these consumers 'check' should be renamed 'found'?

Then the logic would read:

 found = get_entry(..);

 if (found == -1)
  error;

 if (found)
  update_entry();
 else /* ! found */
  create_entry();

Re: [PATCH 6/6] libtraceeval samples: Update task-eval to use the histogram logic

[Ross Zwisler]

On Tue, Aug 08, 2023 at 11:13:13PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> Convert task-eval over to the new API.
> 
> Note, it still requires some functions for displaying the output, but
> those were added just as stubs to get an idea on how to use it.
> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  include/traceeval-hist.h |   1 +
>  samples/Makefile         |   2 +-
>  samples/task-eval.c      | 683 ++++++++++++++++++++++-----------------
>  3 files changed, 388 insertions(+), 298 deletions(-)
> 
> diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
> index b3427c662d99..c363444f7fae 100644
> --- a/include/traceeval-hist.h
> +++ b/include/traceeval-hist.h
<>
> @@ -111,28 +192,45 @@ static void update_process(struct task_data *tdata, const char *comm,
>  			   enum sched_state state, enum command cmd,
>  			   unsigned long long ts)

update_process(), update_cpu() and update_thread() are all pretty much the
same, and I think you could combine them if you wanted.  Just create a generic
'update' that takes keys & vals & a pair of 'struct traceval' pointers, one
for start data and one for delta data.  That also relies on the fact that the
TS always comes first in 'vals'.  Up to you if that's cleaner or not.

>  {
> -	struct traceeval_key keys[] = {
> +	union traceeval_data keys[] = {
>  		{
> -			.type = TRACEEVAL_TYPE_STRING,
> -			.string = comm,
> +			.cstring	= comm,
>  		},
>  		{
> -			.type = TRACEEVAL_TYPE_NUMBER,
> -			.number = state,
> +			.number		= state,
> +		},
> +	};
> +	union traceeval_data vals[] = {
> +		{
> +			.number_64	= ts,
> +		},
> +		{
> +			.number		= (long)NULL, /* data */
>  		}
>  	};
> +	union traceeval_data *results;
> +	unsigned long long delta;
>  	int ret;
>  
>  	switch (cmd) {
>  	case START:
> -		ret = traceeval_n_start(tdata->teval_processes, keys, ts);
> +		ret = traceeval_insert(tdata->teval_processes_start, keys, vals);
>  		if (ret < 0)
>  			pdie("Could not start process");
>  		return;
>  	case STOP:
> -		ret = traceeval_n_stop(tdata->teval_processes, keys, ts);
> +		ret = traceeval_query(tdata->teval_processes_start, keys, &results);
> +		if (ret < 0)
> +			return;
> +
> +		delta = ts - results[0].number_64;
> +		results[0].number_64 = delta;
> +
> +		ret = traceeval_insert(tdata->teval_processes, keys, results);
>  		if (ret < 0)
>  			pdie("Could not stop process");
> +
> +		traceeval_results_release(tdata->teval_processes_start, results);
>  		return;
>  	}
>  }
<>
> @@ -283,32 +421,15 @@ static struct tep_format_field *get_field(struct tep_event *event, const char *n
>  
>  static void init_process_data(struct process_data *pdata)
>  {
> -	struct traceeval_key_info cpu_info[] = {
> -		{
> -			.type = TRACEEVAL_TYPE_NUMBER,
> -			.name = "CPU",
> -		},
> -		{
> -			.type = TRACEEVAL_TYPE_NUMBER,
> -			.name = "Schedule state",
> -		}
> -	};
> -	struct traceeval_key_info thread_info[] = {
> -		{
> -			.type = TRACEEVAL_TYPE_NUMBER,
> -			.name = "TID",
> -		},
> -		{
> -			.type = TRACEEVAL_TYPE_NUMBER,
> -			.name = "Schedule state",
> -		}
> -	};
>  
> -	pdata->teval_cpus = traceeval_2_alloc("CPUs", cpu_info);
> +	pdata->teval_cpus = traceeval_init(cpu_keys, timestamp_vals);
> +	if (!pdata->teval_cpus)
> +		pdie("Creating trace eval");
> +	pdata->teval_cpus = traceeval_init(cpu_keys, timestamp_vals);
>  	if (!pdata->teval_cpus)
>  		pdie("Creating trace eval");

We're initializing pdata->teval_cpus twice - I'm guessing one of these wants to
be pdata->teval_cpus_start?

>  
> -	pdata->teval_threads = traceeval_2_alloc("Threads", thread_info);
> +	pdata->teval_threads = traceeval_init(thread_keys, timestamp_vals);
>  	if (!pdata->teval_threads)
>  		pdie("Creating trace eval");

Missing init of pdata->teval_threads_start?

>  }

Re: [PATCH 5/6] libtraceeval histogram: Fix the return value of traceeval_query()

[Steven Rostedt]

On Thu, 10 Aug 2023 11:47:50 -0600
Ross Zwisler <zwisler@google.com> wrote:

> On Tue, Aug 08, 2023 at 11:13:12PM -0400, Steven Rostedt wrote:
> > From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> > 
> > The comments state that traceeval_query() returns 1 if found, 0 if not,
> > and -1 on error, but in reality it returns 0 if found and 1 if not found.  
> 
> The comment currently says:
> 
> > /*
> >  * Find the entry that @keys corresponds to within @teval.
> >  *
> >  * Returns 0 on success, 1 if no match found, -1 on error.
> >  */  
> 
> I think this needs to be updated to match the new logic (1=found, 0=not
> found, -1=error)
> 

Yeah, but I believe Stevie's latest version fixed all this.

-- Steve

Re: [PATCH 6/6] libtraceeval samples: Update task-eval to use the histogram logic

[Steven Rostedt]

On Thu, 10 Aug 2023 14:28:38 -0600
Ross Zwisler <zwisler@google.com> wrote:

> On Tue, Aug 08, 2023 at 11:13:13PM -0400, Steven Rostedt wrote:
> > From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> > 
> > Convert task-eval over to the new API.
> > 
> > Note, it still requires some functions for displaying the output, but
> > those were added just as stubs to get an idea on how to use it.
> > 
> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> > ---
> >  include/traceeval-hist.h |   1 +
> >  samples/Makefile         |   2 +-
> >  samples/task-eval.c      | 683 ++++++++++++++++++++++-----------------
> >  3 files changed, 388 insertions(+), 298 deletions(-)
> > 
> > diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
> > index b3427c662d99..c363444f7fae 100644
> > --- a/include/traceeval-hist.h
> > +++ b/include/traceeval-hist.h  
> <>
> > @@ -111,28 +192,45 @@ static void update_process(struct task_data *tdata, const char *comm,
> >  			   enum sched_state state, enum command cmd,
> >  			   unsigned long long ts)  
> 
> update_process(), update_cpu() and update_thread() are all pretty much the
> same, and I think you could combine them if you wanted.  Just create a generic
> 'update' that takes keys & vals & a pair of 'struct traceval' pointers, one
> for start data and one for delta data.  That also relies on the fact that the
> TS always comes first in 'vals'.  Up to you if that's cleaner or not.

I did actually create a struct teval_pair because I was getting confused ;-)

But I do agree, some of this can be consolidated a bit more. Right now,
this is just a bare minimum switch from the old logic to the new so that I
can make sure they produce the same output.

> 
> >  {
> > -	struct traceeval_key keys[] = {
> > +	union traceeval_data keys[] = {
> >  		{
> > -			.type = TRACEEVAL_TYPE_STRING,
> > -			.string = comm,
> > +			.cstring	= comm,
> >  		},
> >  		{
> > -			.type = TRACEEVAL_TYPE_NUMBER,
> > -			.number = state,
> > +			.number		= state,
> > +		},
> > +	};
> > +	union traceeval_data vals[] = {
> > +		{
> > +			.number_64	= ts,
> > +		},
> > +		{
> > +			.number		= (long)NULL, /* data */
> >  		}
> >  	};
> > +	union traceeval_data *results;
> > +	unsigned long long delta;
> >  	int ret;
> >  
> >  	switch (cmd) {
> >  	case START:
> > -		ret = traceeval_n_start(tdata->teval_processes, keys, ts);
> > +		ret = traceeval_insert(tdata->teval_processes_start, keys, vals);
> >  		if (ret < 0)
> >  			pdie("Could not start process");
> >  		return;
> >  	case STOP:
> > -		ret = traceeval_n_stop(tdata->teval_processes, keys, ts);
> > +		ret = traceeval_query(tdata->teval_processes_start, keys, &results);
> > +		if (ret < 0)
> > +			return;
> > +
> > +		delta = ts - results[0].number_64;
> > +		results[0].number_64 = delta;
> > +
> > +		ret = traceeval_insert(tdata->teval_processes, keys, results);
> >  		if (ret < 0)
> >  			pdie("Could not stop process");
> > +
> > +		traceeval_results_release(tdata->teval_processes_start, results);
> >  		return;
> >  	}
> >  }  
> <>
> > @@ -283,32 +421,15 @@ static struct tep_format_field *get_field(struct tep_event *event, const char *n
> >  
> >  static void init_process_data(struct process_data *pdata)
> >  {
> > -	struct traceeval_key_info cpu_info[] = {
> > -		{
> > -			.type = TRACEEVAL_TYPE_NUMBER,
> > -			.name = "CPU",
> > -		},
> > -		{
> > -			.type = TRACEEVAL_TYPE_NUMBER,
> > -			.name = "Schedule state",
> > -		}
> > -	};
> > -	struct traceeval_key_info thread_info[] = {
> > -		{
> > -			.type = TRACEEVAL_TYPE_NUMBER,
> > -			.name = "TID",
> > -		},
> > -		{
> > -			.type = TRACEEVAL_TYPE_NUMBER,
> > -			.name = "Schedule state",
> > -		}
> > -	};
> >  
> > -	pdata->teval_cpus = traceeval_2_alloc("CPUs", cpu_info);
> > +	pdata->teval_cpus = traceeval_init(cpu_keys, timestamp_vals);
> > +	if (!pdata->teval_cpus)
> > +		pdie("Creating trace eval");
> > +	pdata->teval_cpus = traceeval_init(cpu_keys, timestamp_vals);
> >  	if (!pdata->teval_cpus)
> >  		pdie("Creating trace eval");  
> 
> We're initializing pdata->teval_cpus twice - I'm guessing one of these wants to
> be pdata->teval_cpus_start?

Good catch. I also caught it when I started working back on it.

> 
> >  
> > -	pdata->teval_threads = traceeval_2_alloc("Threads", thread_info);
> > +	pdata->teval_threads = traceeval_init(thread_keys, timestamp_vals);
> >  	if (!pdata->teval_threads)
> >  		pdie("Creating trace eval");  
> 
> Missing init of pdata->teval_threads_start?

Yep.

new patches in a bit. (now I can go to bed!)

-- Steve