[PATCH 0/8] This series of patches contains various independent

[PATCH 0/8] This series of patches contains various independent

[Yordan Karadzhov]

Yordan Karadzhov (8):
  kernel-shark-qt: Free all collections when closing the trace file.
  kernel-shark-qt: Fix potential memory leak when searching in
    collections
  kernel-shark-qt: Put error messages always on top
  kernel-shark-qt: Fix bug when loading the Dual Marker
  kernel-shark-qt: Fix bug when loading plugins from session
  kernel-shark-qt: Fix bug when resizing the KS window for session
  kernel-shark-qt: Change the color convention of the task graphs
  kernel-shark-qt: Remove duplicate code in KsWidgetsLib.cpp

 kernel-shark-qt/examples/dataplot.cpp |  30 +++--
 kernel-shark-qt/src/KsGLWidget.cpp    |  15 ++-
 kernel-shark-qt/src/KsGLWidget.hpp    |   2 +
 kernel-shark-qt/src/KsMainWindow.cpp  |  12 +-
 kernel-shark-qt/src/KsMainWindow.hpp  |  10 +-
 kernel-shark-qt/src/KsPlotTools.cpp   | 152 ++++++++++++++++----------
 kernel-shark-qt/src/KsPlotTools.hpp   |  45 ++++++--
 kernel-shark-qt/src/KsSession.cpp     |  46 +++++---
 kernel-shark-qt/src/KsSession.hpp     |   4 +-
 kernel-shark-qt/src/KsWidgetsLib.cpp  |  28 ++---
 kernel-shark-qt/src/libkshark-model.c |   8 +-
 kernel-shark-qt/src/libkshark.c       |   7 ++
 12 files changed, 233 insertions(+), 126 deletions(-)

-- 
2.17.1

[PATCH 1/8] kernel-shark-qt: Free all collections when closing the trace file.

[Yordan Karadzhov]

All data collections are file specific. We have to guarantee that
collections from one file cannot be used with another file.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/libkshark.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
index 33d25de..64236a5 100644
--- a/kernel-shark-qt/src/libkshark.c
+++ b/kernel-shark-qt/src/libkshark.c
@@ -188,6 +188,13 @@ void kshark_close(struct kshark_context *kshark_ctx)
 		kshark_ctx->advanced_event_filter = NULL;
 	}
 
+	/*
+	 * All data collections are file specific. Make sure that collections
+	 * from this file are not going to be used with another file.
+	 */
+	kshark_free_collection_list(kshark_ctx->collections);
+	kshark_ctx->collections = NULL;
+
 	tracecmd_close(kshark_ctx->handle);
 	kshark_ctx->handle = NULL;
 	kshark_ctx->pevent = NULL;
-- 
2.17.1

[PATCH 2/8] kernel-shark-qt: Fix potential memory leak when searching in collections

[Yordan Karadzhov]

In some cases the Data Collection can map the original data request into
a list of data requests. This is done in the cases when the range of the
original request overlaps with more the one data interval (as defined by
the collection). In such a case we must free the entire list of requests.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/libkshark-model.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel-shark-qt/src/libkshark-model.c b/kernel-shark-qt/src/libkshark-model.c
index 246a60c..7800090 100644
--- a/kernel-shark-qt/src/libkshark-model.c
+++ b/kernel-shark-qt/src/libkshark-model.c
@@ -900,7 +900,7 @@ ksmodel_get_entry_front(struct kshark_trace_histo *histo,
 	else
 		entry = kshark_get_entry_front(req, histo->data, index);
 
-	free(req);
+	kshark_free_entry_request(req);
 
 	return entry;
 }
@@ -947,7 +947,7 @@ ksmodel_get_entry_back(struct kshark_trace_histo *histo,
 	else
 		entry = kshark_get_entry_back(req, histo->data, index);
 
-	free(req);
+	kshark_free_entry_request(req);
 
 	return entry;
 }
@@ -1160,7 +1160,7 @@ bool ksmodel_cpu_visible_event_exist(struct kshark_trace_histo *histo,
 	else
 		entry = kshark_get_entry_front(req, histo->data, index);
 
-	free(req);
+	kshark_free_entry_request(req);
 
 	if (!entry || !entry->visible) {
 		/* No visible entry has been found. */
@@ -1213,7 +1213,7 @@ bool ksmodel_task_visible_event_exist(struct kshark_trace_histo *histo,
 	else
 		entry = kshark_get_entry_front(req, histo->data, index);
 
-	free(req);
+	kshark_free_entry_request(req);
 
 	if (!entry || !entry->visible) {
 		/* No visible entry has been found. */
-- 
2.17.1

[PATCH 3/8] kernel-shark-qt: Put error messages always on top

[Yordan Karadzhov]

When an error message is shown its widget must be placed always on top
of all other windows. Also the user shouldn't be able to do anything
else, before closing the error message widget.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/KsMainWindow.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel-shark-qt/src/KsMainWindow.cpp b/kernel-shark-qt/src/KsMainWindow.cpp
index b3cbc3b..4f9a6e2 100644
--- a/kernel-shark-qt/src/KsMainWindow.cpp
+++ b/kernel-shark-qt/src/KsMainWindow.cpp
@@ -859,8 +859,9 @@ void KsMainWindow::_error(const QString &text, const QString &errCode,
 	if (unloadPlugins)
 		_plugins.unloadAll();
 
-	em->showMessage(text, errCode);
 	qCritical() << "ERROR: " << text;
+	em->showMessage(text, errCode);
+	em->exec();
 }
 
 /**
-- 
2.17.1

[PATCH 4/8] kernel-shark-qt: Fix bug when loading the Dual Marker

[Yordan Karadzhov]

When loading the settings of the Dual Marker for a session,
the View(table) must be updated only if the active marker is
actually set.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/KsSession.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel-shark-qt/src/KsSession.cpp b/kernel-shark-qt/src/KsSession.cpp
index 96e09f2..6cd1403 100644
--- a/kernel-shark-qt/src/KsSession.cpp
+++ b/kernel-shark-qt/src/KsSession.cpp
@@ -431,7 +431,6 @@ void KsSession::loadDualMarker(KsDualMarkerSM *dm, KsTraceGraph *graphs)
 
 	dm->reset();
 	dm->setState(DualMarkerState::A);
-
 	if (_getMarker("markA", &pos)) {
 		graphs->markEntry(pos);
 	} else {
@@ -446,9 +445,10 @@ void KsSession::loadDualMarker(KsDualMarkerSM *dm, KsTraceGraph *graphs)
 	}
 
 	dm->setState(_getMarkerState());
-	pos = dm->activeMarker()._pos;
-
-	emit graphs->glPtr()->updateView(pos, true);
+	if (dm->activeMarker()._isSet) {
+		pos = dm->activeMarker()._pos;
+		emit graphs->glPtr()->updateView(pos, true);
+	}
 }
 
 json_object *KsSession::_getMarkerJson()
-- 
2.17.1

[PATCH 5/8] kernel-shark-qt: Fix bug when loading plugins from session

[Yordan Karadzhov]

The session description file may contain plugins which are not available
at the moment when the session is loaded. These can be user-defined
plugins for example. The modification of the code introduced by this
patch deals with such a case.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/KsSession.cpp | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/kernel-shark-qt/src/KsSession.cpp b/kernel-shark-qt/src/KsSession.cpp
index 6cd1403..b7ef81c 100644
--- a/kernel-shark-qt/src/KsSession.cpp
+++ b/kernel-shark-qt/src/KsSession.cpp
@@ -545,7 +545,10 @@ void KsSession::loadPlugins(kshark_context *kshark_ctx, KsPluginManager *pm)
 {
 	kshark_config_doc *plugins = kshark_config_alloc(KS_CONFIG_JSON);
 	json_object *jplugins, *jlist, *jpl;
-	int length;
+	const char *pluginName;
+	QVector<int> pluginIds;
+	int length, index;
+	bool loaded;
 
 	if (!kshark_config_doc_get(_config, "Plugins", plugins) ||
 	    !kshark_type_check(plugins, "kshark.config.plugins"))
@@ -562,13 +565,13 @@ void KsSession::loadPlugins(kshark_context *kshark_ctx, KsPluginManager *pm)
 		length = json_object_array_length(jlist);
 		for (int i = 0; i < length; ++i) {
 			jpl = json_object_array_get_idx(jlist, i);
-			pm->_ksPluginList[i] =
-				json_object_get_string(json_object_array_get_idx(jpl, 0));
-
-			pm->_registeredKsPlugins[i] =
-				json_object_get_boolean(json_object_array_get_idx(jpl, 1));
+			pluginName = json_object_get_string(json_object_array_get_idx(jpl, 0));
+			index = pm->_ksPluginList.indexOf(pluginName);
+			loaded = json_object_get_boolean(json_object_array_get_idx(jpl, 1));
+			if (index >= 0 && loaded)
+				pluginIds.append(index);
 		}
 	}
 
-	pm->registerFromList(kshark_ctx);
+	pm->updatePlugins(pluginIds);
 }
-- 
2.17.1

[PATCH 6/8] kernel-shark-qt: Fix bug when resizing the KS window for session

[Yordan Karadzhov]

Currently the session export/import code cannot deal with the case when
the KernelShark GUI is in Full Screen mode. This patch fixes this.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/KsMainWindow.cpp |  9 +++------
 kernel-shark-qt/src/KsMainWindow.hpp | 10 +++++++---
 kernel-shark-qt/src/KsSession.cpp    | 21 +++++++++++++++++----
 kernel-shark-qt/src/KsSession.hpp    |  4 +++-
 4 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/kernel-shark-qt/src/KsMainWindow.cpp b/kernel-shark-qt/src/KsMainWindow.cpp
index 4f9a6e2..89b1b58 100644
--- a/kernel-shark-qt/src/KsMainWindow.cpp
+++ b/kernel-shark-qt/src/KsMainWindow.cpp
@@ -64,7 +64,6 @@ KsMainWindow::KsMainWindow(QWidget *parent)
   _colSlider(this),
   _colorPhaseSlider(Qt::Horizontal, this),
   _fullScreenModeAction("Full Screen Mode", this),
-  _isFullScreen(false),
   _aboutAction("About", this),
   _contentsAction("Contents", this)
 {
@@ -245,7 +244,7 @@ void KsMainWindow::_createActions()
 	_fullScreenModeAction.setStatusTip("Full Screen Mode");
 
 	connect(&_fullScreenModeAction,	&QAction::triggered,
-		this,			&KsMainWindow::_fullScreenMode);
+		this,			&KsMainWindow::_changeScreenMode);
 
 	/* Help menu */
 	_aboutAction.setIcon(QIcon::fromTheme("help-about"));
@@ -732,18 +731,16 @@ void KsMainWindow::_setColorPhase(int f)
 	_graph.glPtr()->model()->update();
 }
 
-void KsMainWindow::_fullScreenMode()
+void KsMainWindow::_changeScreenMode()
 {
-	if (_isFullScreen) {
+	if (isFullScreen()) {
 		_fullScreenModeAction.setText("Full Screen Mode");
 		_fullScreenModeAction.setIcon(QIcon::fromTheme("view-fullscreen"));
 		showNormal();
-		_isFullScreen = false;
 	} else {
 		_fullScreenModeAction.setText("Exit Full Screen Mode");
 		_fullScreenModeAction.setIcon(QIcon::fromTheme("view-restore"));
 		showFullScreen();
-		_isFullScreen = true;
 	}
 }
 
diff --git a/kernel-shark-qt/src/KsMainWindow.hpp b/kernel-shark-qt/src/KsMainWindow.hpp
index 0e14c80..d711ec1 100644
--- a/kernel-shark-qt/src/KsMainWindow.hpp
+++ b/kernel-shark-qt/src/KsMainWindow.hpp
@@ -61,6 +61,12 @@ public:
 
 	void resizeEvent(QResizeEvent* event);
 
+	/** Set the Full Screen mode. */
+	void setFullScreenMode(bool f) {
+		if ((!isFullScreen() && f) || (isFullScreen() && !f) )
+			_changeScreenMode();
+	}
+
 private:
 	QSplitter	_splitter;
 
@@ -136,8 +142,6 @@ private:
 
 	QAction		_fullScreenModeAction;
 
-	bool		_isFullScreen;
-
 	// Help menu.
 	QAction		_aboutAction;
 
@@ -179,7 +183,7 @@ private:
 
 	void _setColorPhase(int);
 
-	void _fullScreenMode();
+	void _changeScreenMode();
 
 	void _aboutInfo();
 
diff --git a/kernel-shark-qt/src/KsSession.cpp b/kernel-shark-qt/src/KsSession.cpp
index b7ef81c..2242a12 100644
--- a/kernel-shark-qt/src/KsSession.cpp
+++ b/kernel-shark-qt/src/KsSession.cpp
@@ -12,6 +12,7 @@
 // KernelShark
 #include "libkshark.h"
 #include "KsSession.hpp"
+#include "KsMainWindow.hpp"
 
 /** Create a KsSession object. */
 KsSession::KsSession()
@@ -175,10 +176,15 @@ void KsSession::saveMainWindowSize(const QMainWindow &window)
 {
 	kshark_config_doc *windowConf = kshark_config_alloc(KS_CONFIG_JSON);
 	int width = window.width(), height = window.height();
-	json_object *jwindow = json_object_new_array();
+	json_object *jwindow;
 
-	json_object_array_put_idx(jwindow, 0, json_object_new_int(width));
-	json_object_array_put_idx(jwindow, 1, json_object_new_int(height));
+	if (window.isFullScreen()) {
+		jwindow = json_object_new_string("FullScreen");
+	} else {
+		jwindow = json_object_new_array();
+		json_object_array_put_idx(jwindow, 0, json_object_new_int(width));
+		json_object_array_put_idx(jwindow, 1, json_object_new_int(height));
+	}
 
 	windowConf->conf_doc = jwindow;
 	kshark_config_doc_add(_config, "MainWindow", windowConf);
@@ -189,7 +195,7 @@ void KsSession::saveMainWindowSize(const QMainWindow &window)
  *
  * @param window: Input location for the KsMainWindow widget.
  */
-void KsSession::loadMainWindowSize(QMainWindow *window)
+void KsSession::loadMainWindowSize(KsMainWindow *window)
 {
 	kshark_config_doc *windowConf = kshark_config_alloc(KS_CONFIG_JSON);
 	json_object *jwindow, *jwidth, *jheight;
@@ -200,12 +206,19 @@ void KsSession::loadMainWindowSize(QMainWindow *window)
 
 	if (_config->format == KS_CONFIG_JSON) {
 		jwindow = KS_JSON_CAST(windowConf->conf_doc);
+		if (json_object_get_type(jwindow) == json_type_string &&
+		    QString(json_object_get_string(jwindow)) == "FullScreen") {
+			window->setFullScreenMode(true);
+			return;
+		}
+
 		jwidth = json_object_array_get_idx(jwindow, 0);
 		jheight = json_object_array_get_idx(jwindow, 1);
 
 		width = json_object_get_int(jwidth);
 		height = json_object_get_int(jheight);
 
+		window->setFullScreenMode(false);
 		window->resize(width, height);
 	}
 }
diff --git a/kernel-shark-qt/src/KsSession.hpp b/kernel-shark-qt/src/KsSession.hpp
index 4f5a2c4..f5ed5a1 100644
--- a/kernel-shark-qt/src/KsSession.hpp
+++ b/kernel-shark-qt/src/KsSession.hpp
@@ -20,6 +20,8 @@
 #include "KsTraceGraph.hpp"
 #include "KsTraceViewer.hpp"
 
+class KsMainWindow;
+
 /**
  * The KsSession class provides instruments for importing/exporting the state
  * of the different components of the GUI from/to Json documents. These
@@ -57,7 +59,7 @@ public:
 
 	void saveMainWindowSize(const QMainWindow &window);
 
-	void loadMainWindowSize(QMainWindow *window);
+	void loadMainWindowSize(KsMainWindow *window);
 
 	void saveSplitterSize(const QSplitter &splitter);
 
-- 
2.17.1

[PATCH 7/8] kernel-shark-qt: Change the color convention of the task graphs

[Yordan Karadzhov]

So far the Task graphs have used the same color convention as the
CPU graphs. Namely the vertical tick of the record was plotted by
using the CPU color and the horizontal strap was plotted by using
the task color. This patch reverts the color convention and now the
tick has the color of the task and the strap has the color of the
CPU. This change requires significant modification of the logic
used to create and draw the graphs.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/examples/dataplot.cpp |  30 +++--
 kernel-shark-qt/src/KsGLWidget.cpp    |  15 ++-
 kernel-shark-qt/src/KsGLWidget.hpp    |   2 +
 kernel-shark-qt/src/KsPlotTools.cpp   | 152 ++++++++++++++++----------
 kernel-shark-qt/src/KsPlotTools.hpp   |  45 ++++++--
 5 files changed, 162 insertions(+), 82 deletions(-)

diff --git a/kernel-shark-qt/examples/dataplot.cpp b/kernel-shark-qt/examples/dataplot.cpp
index 27a0125..94841e7 100644
--- a/kernel-shark-qt/examples/dataplot.cpp
+++ b/kernel-shark-qt/examples/dataplot.cpp
@@ -78,36 +78,42 @@ void drawShapes()
 /* An example function demonstrating Zoom In and Zoom Out. */
 void play()
 {
-	KsPlot::ColorTable colors = KsPlot::getColorTable();
+	KsPlot::ColorTable taskColors = KsPlot::getTaskColorTable();
+	KsPlot::ColorTable cpuColors = KsPlot::getCPUColorTable();
 	vector<KsPlot::Graph *>::iterator it;
 	vector<int> CPUs, Tasks;
-	KsPlot::Graph *graph;
 	bool zoomIn(true);
 	int base;
+	size_t i;
 
 	CPUs = {3, 4, 6};
 	Tasks = {}; // Add valid pids here, if you want task plots.
 
-	for (size_t i = 0; i < CPUs.size() + Tasks.size(); ++i) {
-		/* Make a new Graph. */
-		graph = new KsPlot::Graph(&histo, &colors);
-
+	auto lamAddGraph = [&] (KsPlot::Graph *g) {
 		/* Set the dimensions of the Graph. */
-		graph->setHeight(GRAPH_HEIGHT);
-		graph->setHMargin(GRAPH_H_MARGIN);
+		g->setHeight(GRAPH_HEIGHT);
+		g->setHMargin(GRAPH_H_MARGIN);
 
 		/*
 		 * Set the Y coordinate of the Graph's base.
 		 * Remember that the "Y" coordinate is inverted.
 		 */
 		base = 1.7 * GRAPH_HEIGHT * (i + 1);
-		graph->setBase(base);
+		g->setBase(base);
 
 		/* Add the Graph. */
-		graphs.push_back(graph);
-	}
+		graphs.push_back(g);
+	};
+
+	for (i = 0; i < CPUs.size(); ++i)
+		lamAddGraph(new KsPlot::Graph(&histo, &taskColors,
+						      &taskColors));
+
+	for (;i < CPUs.size() + Tasks.size(); ++i)
+		lamAddGraph(new KsPlot::Graph(&histo, &taskColors,
+						      &cpuColors));
 
-	for (int i = 1; i < 1000; ++i) {
+	for (i = 1; i < 1000; ++i) {
 		it = graphs.begin();
 
 		for (int const &cpu: CPUs)
diff --git a/kernel-shark-qt/src/KsGLWidget.cpp b/kernel-shark-qt/src/KsGLWidget.cpp
index 22cbd96..2a0b16b 100644
--- a/kernel-shark-qt/src/KsGLWidget.cpp
+++ b/kernel-shark-qt/src/KsGLWidget.cpp
@@ -356,7 +356,9 @@ void KsGLWidget::loadData(KsDataStore *data)
 void KsGLWidget::loadColors()
 {
 	_pidColors.clear();
-	_pidColors = KsPlot::getColorTable();
+	_pidColors = KsPlot::getTaskColorTable();
+	_cpuColors.clear();
+	_cpuColors = KsPlot::getCPUColorTable();
 }
 
 /**
@@ -530,8 +532,12 @@ void KsGLWidget::_makePluginShapes(QVector<int> cpuList, QVector<int> taskList)
 
 KsPlot::Graph *KsGLWidget::_newCPUGraph(int cpu)
 {
+	/* The CPU graph needs to know only the colors of the tasks. */
 	KsPlot::Graph *graph = new KsPlot::Graph(_model.histo(),
+						 &_pidColors,
 						 &_pidColors);
+	graph->setZeroSuppressed(true);
+
 	kshark_context *kshark_ctx(nullptr);
 	kshark_entry_collection *col;
 
@@ -553,8 +559,13 @@ KsPlot::Graph *KsGLWidget::_newCPUGraph(int cpu)
 
 KsPlot::Graph *KsGLWidget::_newTaskGraph(int pid)
 {
+	/*
+	 * The Task graph needs to know the colors of the tasks and the colors
+	 * of the CPUs.
+	 */
 	KsPlot::Graph *graph = new KsPlot::Graph(_model.histo(),
-						 &_pidColors);
+						 &_pidColors,
+						 &_cpuColors);
 	kshark_context *kshark_ctx(nullptr);
 	kshark_entry_collection *col;
 
diff --git a/kernel-shark-qt/src/KsGLWidget.hpp b/kernel-shark-qt/src/KsGLWidget.hpp
index 5b8ff8c..662cd26 100644
--- a/kernel-shark-qt/src/KsGLWidget.hpp
+++ b/kernel-shark-qt/src/KsGLWidget.hpp
@@ -161,6 +161,8 @@ private:
 
 	KsPlot::ColorTable	_pidColors;
 
+	KsPlot::ColorTable	_cpuColors;
+
 	int		_hMargin, _vMargin;
 
 	unsigned int	_vSpacing;
diff --git a/kernel-shark-qt/src/KsPlotTools.cpp b/kernel-shark-qt/src/KsPlotTools.cpp
index 0cd7a7d..816fa56 100644
--- a/kernel-shark-qt/src/KsPlotTools.cpp
+++ b/kernel-shark-qt/src/KsPlotTools.cpp
@@ -106,11 +106,11 @@ void Color::setRainbowColor(int n)
  *
  * @returns ColorTable instance.
  */
-ColorTable getColorTable()
+ColorTable getTaskColorTable()
 {
 	struct kshark_context *kshark_ctx(nullptr);
 	ColorTable colors;
-	int nTasks, pid, *pids;
+	int nTasks, pid, *pids, i(0);
 
 	if (!kshark_instance(&kshark_ctx))
 		return colors;
@@ -122,10 +122,12 @@ ColorTable getColorTable()
 	std::vector<int> temp_pids(pids, pids + nTasks);
 	std::sort(temp_pids.begin(), temp_pids.end());
 
-	/* The "Idle" process (pid = 0) will be plotted in black. */
-	colors[0] = {};
+	if (temp_pids[i] == 0) {
+		/* The "Idle" process (pid = 0) will be plotted in black. */
+		colors[i++] = {};
+	}
 
-	for (int i = 1; i < nTasks; ++i) {
+	for (; i < nTasks; ++i) {
 		pid = temp_pids[i];
 		colors[pid].setRainbowColor(i - 1);
 	}
@@ -133,6 +135,28 @@ ColorTable getColorTable()
 	return colors;
 }
 
+/**
+ * @brief Create a Hash table of Rainbow colors. The CPU Ids are
+ *	  mapped to the palette of Rainbow colors.
+ *
+ * @returns ColorTable instance.
+ */
+ColorTable getCPUColorTable()
+{
+	struct kshark_context *kshark_ctx(nullptr);
+	ColorTable colors;
+	int nCPUs;
+
+	if (!kshark_instance(&kshark_ctx))
+		return colors;
+
+	nCPUs =  tep_get_cpus(kshark_ctx->pevent);
+	for (int i = 0; i < nCPUs; ++i)
+		colors[i].setRainbowColor(i);
+
+	return colors;
+}
+
 /**
  * @brief Search the Hash table of Rainbow colors for a particular key (pid).
  *
@@ -142,7 +166,7 @@ ColorTable getColorTable()
  * @returns The Rainbow color of the key "pid". If "pid" does not exist, the
  *	    returned color is Black.
  */
-Color getPidColor(ColorTable *colors, int pid)
+Color getColor(ColorTable *colors, int pid)
 {
 	auto item = colors->find(pid);
 
@@ -499,8 +523,8 @@ void Mark::setTaskVisible(bool v)
  * @brief Create a default Bin.
  */
 Bin::Bin()
-: _pidFront(KS_EMPTY_BIN),
-  _pidBack(KS_EMPTY_BIN)
+: _idFront(KS_EMPTY_BIN),
+  _idBack(KS_EMPTY_BIN)
 {}
 
 void Bin::_draw(const Color &col, float size) const
@@ -528,22 +552,27 @@ Graph::Graph()
   _size(0),
   _hMargin(30),
   _collectionPtr(nullptr),
-  _pidColors(nullptr)
+  _binColors(nullptr),
+  _ensembleColors(nullptr),
+  _zeroSuppress(false)
 {}
 
 /**
  * @brief Create a Graph to represent the state of the Vis. model.
  *
  * @param histo: Input location for the model descriptor.
- * @param ct: Input location for the Hash table of Task's colors.
+ * @param bct: Input location for the Hash table of bin's colors.
+ * @param ect: Input location for the Hash table of ensemble's colors.
  */
-Graph::Graph(kshark_trace_histo *histo, KsPlot::ColorTable *ct)
+Graph::Graph(kshark_trace_histo *histo, KsPlot::ColorTable *bct, KsPlot::ColorTable *ect)
 : _histoPtr(histo),
   _bins(new(std::nothrow) Bin[histo->n_bins]),
   _size(histo->n_bins),
   _hMargin(30),
   _collectionPtr(nullptr),
-  _pidColors(ct)
+  _binColors(bct),
+  _ensembleColors(ect),
+  _zeroSuppress(false)
 {
 	if (!_bins) {
 		_size = 0;
@@ -678,8 +707,8 @@ void Graph::setBinValue(int bin, int val)
  */
 void Graph::setBinPid(int bin, int pidF, int pidB)
 {
-	_bins[bin]._pidFront = pidF;
-	_bins[bin]._pidBack = pidB;
+	_bins[bin]._idFront = pidF;
+	_bins[bin]._idBack = pidB;
 }
 
 /**
@@ -782,7 +811,7 @@ void Graph::fillCPUGraph(int cpu)
 		if (pidFront != KS_EMPTY_BIN || pidBack != KS_EMPTY_BIN) {
 			/* This is a regular process. */
 			setBin(bin, pidFront, pidBack,
-			       getPidColor(_pidColors, pidFront), visMask);
+			       getColor(_binColors, pidFront), visMask);
 		} else {
 			/* The bin contens no data from this CPU. */
 			setBinPid(bin, KS_EMPTY_BIN, KS_EMPTY_BIN);
@@ -855,27 +884,26 @@ void Graph::fillCPUGraph(int cpu)
  */
 void Graph::fillTaskGraph(int pid)
 {
-	int cpu, pidFront(0), pidBack(0), lastCpu(-1), bin(0);
+	int cpuFront, cpuBack(0), pidFront(0), pidBack(0), lastCpu(-1), bin(0);
 	uint8_t visMask;
 	ssize_t index;
 
 	auto lamSetBin = [&] (int bin)
 	{
-		if (cpu >= 0) {
-			KsPlot::Color col;
-			col.setRainbowColor(cpu);
+		if (cpuFront >= 0) {
+			KsPlot::Color col = getColor(_binColors, pid);
 
 			/* Data from the Task has been found in this bin. */
 			if (pid == pidFront && pid == pidBack) {
 				/* No data from other tasks in this bin. */
-				setBin(bin, pid, pid, col, visMask);
+				setBin(bin, cpuFront, cpuBack, col, visMask);
 			} else if (pid != pidFront && pid != pidBack) {
 				/*
 				 * There is some data from other tasks at both
 				 * front and back sides of this bin. But we
 				 * still want to see this bin drawn.
 				 */
-				setBin(bin, pid, KS_FILTERED_BIN, col,
+				setBin(bin, cpuFront, KS_FILTERED_BIN, col,
 				       visMask);
 			} else {
 				if (pidFront != pid) {
@@ -883,7 +911,7 @@ void Graph::fillTaskGraph(int pid)
 					 * There is some data from another
 					 * task at the front side of this bin.
 					 */
-					pidFront = KS_FILTERED_BIN;
+					cpuFront = KS_FILTERED_BIN;
 				}
 
 				if (pidBack != pid) {
@@ -891,13 +919,13 @@ void Graph::fillTaskGraph(int pid)
 					 * There is some data from another
 					 * task at the back side of this bin.
 					 */
-					pidBack = KS_FILTERED_BIN;
+					cpuBack = KS_FILTERED_BIN;
 				}
 
-				setBin(bin, pidFront, pidBack, col, visMask);
+				setBin(bin, cpuFront, cpuBack, col, visMask);
 			}
 
-			lastCpu = cpu;
+			lastCpu = cpuBack;
 		} else {
 			/*
 			 * No data from the Task in this bin. Check the CPU,
@@ -929,14 +957,20 @@ void Graph::fillTaskGraph(int pid)
 	auto lamGetPidCPU = [&] (int bin)
 	{
 		/* Get the CPU used by this task. */
-		cpu = ksmodel_get_cpu_front(_histoPtr, bin,
-						       pid,
-						       false,
-						       _collectionPtr,
-						       nullptr);
+		cpuFront = ksmodel_get_cpu_front(_histoPtr, bin,
+						 pid,
+						 false,
+						 _collectionPtr,
+						 nullptr);
+
+		cpuBack = ksmodel_get_cpu_back(_histoPtr, bin,
+					       pid,
+					       false,
+					       _collectionPtr,
+					       nullptr);
 
-		if (cpu < 0) {
-			pidFront = pidBack = cpu;
+		if (cpuFront < 0) {
+			pidFront = pidBack = cpuFront;
 		} else {
 			/*
 			 * Get the process Id at the begining and at the end
@@ -944,14 +978,14 @@ void Graph::fillTaskGraph(int pid)
 			 */
 			pidFront = ksmodel_get_pid_front(_histoPtr,
 							 bin,
-							 cpu,
+							 cpuFront,
 							 false,
 							 _collectionPtr,
 							 nullptr);
 
 			pidBack = ksmodel_get_pid_back(_histoPtr,
 						       bin,
-						       cpu,
+						       cpuBack,
 						       false,
 						       _collectionPtr,
 						       nullptr);
@@ -973,7 +1007,7 @@ void Graph::fillTaskGraph(int pid)
 	 */
 	lamGetPidCPU(bin);
 
-	if (cpu >= 0) {
+	if (cpuFront >= 0) {
 		/* The Task is active. Set this bin. */
 		lamSetBin(bin);
 	} else {
@@ -981,9 +1015,9 @@ void Graph::fillTaskGraph(int pid)
 		 * No data from this Task in the very first bin. Use the Lower
 		 * Overflow Bin to retrieve the CPU used by the task (if any).
 		 */
-		cpu = ksmodel_get_cpu_back(_histoPtr, LOWER_OVERFLOW_BIN, pid,
+		cpuFront = ksmodel_get_cpu_back(_histoPtr, LOWER_OVERFLOW_BIN, pid,
 					   false, _collectionPtr, nullptr);
-		if (cpu >= 0) {
+		if (cpuFront >= 0) {
 			/*
 			 * The Lower Overflow Bin contains data from this Task.
 			 * Now look again in the Lower Overflow Bin and find
@@ -991,7 +1025,7 @@ void Graph::fillTaskGraph(int pid)
 			 */
 			int pidCpu = ksmodel_get_pid_back(_histoPtr,
 							  LOWER_OVERFLOW_BIN,
-							  cpu,
+							  cpuFront,
 							  false,
 							  _collectionPtr,
 							  nullptr);
@@ -1002,8 +1036,10 @@ void Graph::fillTaskGraph(int pid)
 				 * the very first bin is empty but we derive
 				 * the Process Id from the Lower Overflow Bin.
 				 */
-				setBinPid(bin, pid, pid);
-				lastCpu = cpu;
+				setBinPid(bin, cpuFront, cpuFront);
+				lastCpu = cpuFront;
+			} else {
+				setBinPid(bin, KS_EMPTY_BIN, KS_EMPTY_BIN);
 			}
 		}
 	}
@@ -1027,7 +1063,7 @@ void Graph::fillTaskGraph(int pid)
  */
 void Graph::draw(float size)
 {
-	int lastPid(0), b(0), boxH(_height * .3);
+	int lastPid(-1), b(0), boxH(_height * .3);
 	Rectangle taskBox;
 
 	/*
@@ -1038,23 +1074,27 @@ void Graph::draw(float size)
 
 	/* Draw as vartical lines all bins containing data. */
 	for (int i = 0; i < _size; ++i)
-		if (_bins[i]._pidFront >= 0 || _bins[i]._pidBack >= 0)
+		if (_bins[i]._idFront >= 0 || _bins[i]._idBack >= 0)
 			if (_bins[i]._visMask & KS_EVENT_VIEW_FILTER_MASK)
 				_bins[i].draw();
 
+	auto lamCheckEnsblVal = [this] (int v) {
+		return v > 0 || (v == 0 && !this->_zeroSuppress);
+	};
+
 	/*
 	 * Draw colored boxes for processes. First find the first bin, which
 	 * contains data and determine its PID.
 	 */
 	for (; b < _size; ++b) {
-		if (_bins[b]._pidBack > 0) {
-			lastPid = _bins[b]._pidFront;
+		if (lamCheckEnsblVal(_bins[b]._idBack)) {
+			lastPid = _bins[b]._idFront;
 			/*
 			 * Initialize a box starting from this bin.
 			 * The color of the taskBox corresponds to the Pid
 			 * of the process.
 			 */
-			taskBox._color = getPidColor(_pidColors, lastPid);
+			taskBox._color = getColor(_ensembleColors, lastPid);
 			taskBox.setPoint(0, _bins[b]._base.x(),
 					_bins[b]._base.y() - boxH);
 			taskBox.setPoint(1, _bins[b]._base.x(),
@@ -1064,8 +1104,8 @@ void Graph::draw(float size)
 	}
 
 	for (; b < _size; ++b) {
-		if (_bins[b]._pidFront == KS_EMPTY_BIN &&
-		    _bins[b]._pidBack == KS_EMPTY_BIN) {
+		if (_bins[b]._idFront == KS_EMPTY_BIN &&
+		    _bins[b]._idBack == KS_EMPTY_BIN) {
 			/*
 			 * This bin is empty. If a colored taskBox is already
 			 * initialized, it will be extended.
@@ -1073,11 +1113,11 @@ void Graph::draw(float size)
 			continue;
 		}
 
-		if (_bins[b]._pidFront != _bins[b]._pidBack ||
-		    _bins[b]._pidFront != lastPid ||
-		    _bins[b]._pidBack  != lastPid) {
+		if (_bins[b]._idFront != _bins[b]._idBack ||
+		    _bins[b]._idFront != lastPid ||
+		    _bins[b]._idBack  != lastPid) {
 			/* A new process starts here. */
-			if (lastPid > 0 && b > 0) {
+			if (b > 0 && lamCheckEnsblVal(lastPid)) {
 				/*
 				 * There is another process running up to this
 				 * point. Close its colored box here and draw.
@@ -1089,13 +1129,13 @@ void Graph::draw(float size)
 				taskBox.draw();
 			}
 
-			if (_bins[b]._pidBack > 0) {
+			if (lamCheckEnsblVal(_bins[b]._idBack)) {
 				/*
 				 * This is a regular process. Initialize
 				 * colored box starting from this bin.
 				 */
-				taskBox._color = getPidColor(_pidColors,
-							 _bins[b]._pidBack);
+				taskBox._color = getColor(_ensembleColors,
+							 _bins[b]._idBack);
 
 				taskBox.setPoint(0, _bins[b]._base.x() - 1,
 						_bins[b]._base.y() - boxH);
@@ -1103,11 +1143,11 @@ void Graph::draw(float size)
 						_bins[b]._base.y());
 			}
 
-			lastPid = _bins[b]._pidBack;
+			lastPid = _bins[b]._idBack;
 		}
 	}
 
-	if (lastPid > 0) {
+	if (lamCheckEnsblVal(lastPid) > 0) {
 		/*
 		 * This is the end of the Graph and we have a process running.
 		 * Close its colored box and draw.
diff --git a/kernel-shark-qt/src/KsPlotTools.hpp b/kernel-shark-qt/src/KsPlotTools.hpp
index d5779d6..11c49eb 100644
--- a/kernel-shark-qt/src/KsPlotTools.hpp
+++ b/kernel-shark-qt/src/KsPlotTools.hpp
@@ -74,9 +74,11 @@ private:
 /** Hash table of colors. */
 typedef std::unordered_map<int, KsPlot::Color> ColorTable;
 
-ColorTable getColorTable();
+ColorTable getTaskColorTable();
 
-Color getPidColor(ColorTable *colors, int pid);
+ColorTable getCPUColorTable();
+
+Color getColor(ColorTable *colors, int pid);
 
 /** Represents an abstract graphical element. */
 class PlotObject {
@@ -359,16 +361,16 @@ public:
 	void setVal(int v) {_val.setY(_base.y() - v); }
 
 	/**
-	 * The Process Id detected at the front (first in time) edge of
-	 * the bin.
+	 * The Id value (pid or cpu) detected at the front (first in time) edge
+	 * of the bin.
 	 */
-	int	_pidFront;
+	int	_idFront;
 
 	/**
-	 * The Process Id detected at the back (last in time) edge of
-	 * the bin.
+	 * The Id value (pid or cpu) detected at the back (last in time) edge
+	 * of the bin.
 	 */
-	int	_pidBack;
+	int	_idBack;
 
 	/** Lower finishing point of the line, representing the Bin. */
 	Point	_base;
@@ -397,7 +399,8 @@ public:
 	/* Disable moving. Same as copying.*/
 	Graph(Graph &&) = delete;
 
-	Graph(kshark_trace_histo *histo, KsPlot::ColorTable *ct);
+	Graph(kshark_trace_histo *histo, KsPlot::ColorTable *bct,
+					 KsPlot::ColorTable *ect);
 
 	/* Keep this destructor virtual. */
 	virtual ~Graph();
@@ -418,7 +421,7 @@ public:
 	}
 
 	/** @brief Set the Hash table of Task's colors. */
-	void setColorTablePtr(KsPlot::ColorTable *ct) {_pidColors = ct;}
+	void setBinColorTablePtr(KsPlot::ColorTable *ct) {_binColors = ct;}
 
 	void fillCPUGraph(int cpu);
 
@@ -452,6 +455,19 @@ public:
 
 	void setHMargin(int hMargin);
 
+	/**
+	 * Check if this graph is Zero Suppressed. Zero Suppressed means that
+	 * bins having Id value = 0 (Idle task records) are not grouped
+	 * together.
+	 */
+	bool zeroSuppressed(bool zs) {return _zeroSuppress;}
+
+	/**
+	 * Set Zero Suppression. If True, the bins having Id value = 0 (Idle
+	 * task records) are not grouped together.
+	 */
+	void setZeroSuppressed(bool zs) {_zeroSuppress = zs;}
+
 private:
 	/** Pointer to the model descriptor object. */
 	kshark_trace_histo	*_histoPtr;
@@ -474,8 +490,13 @@ private:
 	/** Pointer to the data collection object. */
 	kshark_entry_collection	*_collectionPtr;
 
-	/** Hash table of Task's colors. */
-	ColorTable		*_pidColors;
+	/** Hash table of bin's colors. */
+	ColorTable		*_binColors;
+
+	/** Hash table of ensemble's colors. */
+	ColorTable		*_ensembleColors;
+
+	bool			_zeroSuppress;
 
 	void _initBins();
 };
-- 
2.17.1

[PATCH 8/8] kernel-shark-qt: Remove duplicate code in KsWidgetsLib.cpp

[Yordan Karadzhov]

Currently the "Task" and "CPU" checkbox widgets have their own
implementation of the code that generates the Rainbow color palettes
of the CPUs and Tasks. This patch make the two widgets to use the same
hash tables of colors used when plotting the graphs.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/KsWidgetsLib.cpp | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/kernel-shark-qt/src/KsWidgetsLib.cpp b/kernel-shark-qt/src/KsWidgetsLib.cpp
index b4b62a4..dd6ab0f 100644
--- a/kernel-shark-qt/src/KsWidgetsLib.cpp
+++ b/kernel-shark-qt/src/KsWidgetsLib.cpp
@@ -622,7 +622,7 @@ KsCPUCheckBoxWidget::KsCPUCheckBoxWidget(struct tep_handle *tep,
 : KsCheckBoxTreeWidget("CPUs", parent)
 {
 	int nCPUs(0), height(FONT_HEIGHT * 1.5);
-	KsPlot::Color cpuCol;
+	KsPlot::ColorTable colors;
 	QString style;
 
 	style = QString("QTreeView::item { height: %1 ;}").arg(height);
@@ -635,16 +635,16 @@ KsCPUCheckBoxWidget::KsCPUCheckBoxWidget(struct tep_handle *tep,
 
 	_id.resize(nCPUs);
 	_cb.resize(nCPUs);
+	colors = KsPlot::getCPUColorTable();
 
 	for (int i = 0; i < nCPUs; ++i) {
-		cpuCol.setRainbowColor(i);
 		QTreeWidgetItem *cpuItem = new QTreeWidgetItem;
 		cpuItem->setText(0, "  ");
 		cpuItem->setText(1, QString("CPU %1").arg(i));
 		cpuItem->setCheckState(0, Qt::Checked);
-		cpuItem->setBackgroundColor(0, QColor(cpuCol.r(),
-						      cpuCol.g(),
-						      cpuCol.b()));
+		cpuItem->setBackgroundColor(0, QColor(colors[i].r(),
+						      colors[i].g(),
+						      colors[i].b()));
 		_tree.addTopLevelItem(cpuItem);
 		_id[i] = i;
 		_cb[i] = cpuItem;
@@ -719,10 +719,10 @@ KsTasksCheckBoxWidget::KsTasksCheckBoxWidget(struct tep_handle *pevent,
 {
 	kshark_context *kshark_ctx(nullptr);
 	QTableWidgetItem *pidItem, *comItem;
-	KsPlot::Color pidCol;
+	KsPlot::ColorTable colors;
 	QStringList headers;
 	const char *comm;
-	int nTasks;
+	int nTasks, pid;
 
 	if (!kshark_instance(&kshark_ctx))
 		return;
@@ -735,24 +735,24 @@ KsTasksCheckBoxWidget::KsTasksCheckBoxWidget(struct tep_handle *pevent,
 	_id = KsUtils::getPidList();
 	nTasks = _id.count();
 	_initTable(headers, nTasks);
+	colors = KsPlot::getTaskColorTable();
 
 	for (int i = 0; i < nTasks; ++i) {
-		pidItem	= new QTableWidgetItem(tr("%1").arg(_id[i]));
+		pid = _id[i];
+		pidItem	= new QTableWidgetItem(tr("%1").arg(pid));
 		_table.setItem(i, 1, pidItem);
 
-		comm = tep_data_comm_from_pid(kshark_ctx->pevent, _id[i]);
+		comm = tep_data_comm_from_pid(kshark_ctx->pevent, pid);
 		comItem = new QTableWidgetItem(tr(comm));
 
-		pidItem->setBackgroundColor(QColor(pidCol.r(),
-						   pidCol.g(),
-						   pidCol.b()));
+		pidItem->setBackgroundColor(QColor(colors[pid].r(),
+						   colors[pid].g(),
+						   colors[pid].b()));
 
 		if (_id[i] == 0)
 			pidItem->setTextColor(Qt::white);
 
 		_table.setItem(i, 2, comItem);
-
-		pidCol.setRainbowColor(i);
 	}
 
 	_adjustSize();
-- 
2.17.1

Re: [PATCH 0/8] This series of patches contains various independent

[Steven Rostedt]

On Thu, 1 Nov 2018 21:45:36 +0000
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> Yordan Karadzhov (8):
>   kernel-shark-qt: Free all collections when closing the trace file.
>   kernel-shark-qt: Fix potential memory leak when searching in
>     collections
>   kernel-shark-qt: Put error messages always on top
>   kernel-shark-qt: Fix bug when loading the Dual Marker
>   kernel-shark-qt: Fix bug when loading plugins from session
>   kernel-shark-qt: Fix bug when resizing the KS window for session
>   kernel-shark-qt: Change the color convention of the task graphs
>   kernel-shark-qt: Remove duplicate code in KsWidgetsLib.cpp
> 
>  kernel-shark-qt/examples/dataplot.cpp |  30 +++--
>  kernel-shark-qt/src/KsGLWidget.cpp    |  15 ++-
>  kernel-shark-qt/src/KsGLWidget.hpp    |   2 +
>  kernel-shark-qt/src/KsMainWindow.cpp  |  12 +-
>  kernel-shark-qt/src/KsMainWindow.hpp  |  10 +-
>  kernel-shark-qt/src/KsPlotTools.cpp   | 152 ++++++++++++++++----------
>  kernel-shark-qt/src/KsPlotTools.hpp   |  45 ++++++--
>  kernel-shark-qt/src/KsSession.cpp     |  46 +++++---
>  kernel-shark-qt/src/KsSession.hpp     |   4 +-
>  kernel-shark-qt/src/KsWidgetsLib.cpp  |  28 ++---
>  kernel-shark-qt/src/libkshark-model.c |   8 +-
>  kernel-shark-qt/src/libkshark.c       |   7 ++
>  12 files changed, 233 insertions(+), 126 deletions(-)
> 

Pull. Thanks Yordan!

-- Steve