[PATCH 0/5] More modifications toward KS 1.0

[PATCH 0/5] More modifications toward KS 1.0

[Yordan Karadzhov]

This series of patches contains the last (hopefully) modifications
needed before releasing KernelShark 1.0.

Yordan Karadzhov (5):
  kernel-shark-qt: Avoid spurious searches
  kernel-shark-qt: Create "Apply filter XX" checkboxes in KsUtils
  kernel-shark-qt: Improuve the KsQuickContextMenu
  kernel-shark-qt: Update the documentation link
  kernel-shark-qt: Version 1.0.0

 kernel-shark-qt/CMakeLists.txt             |  4 +--
 kernel-shark-qt/src/KsMainWindow.cpp       | 41 ++++++++++------------
 kernel-shark-qt/src/KsMainWindow.hpp       |  6 ++--
 kernel-shark-qt/src/KsQuickContextMenu.cpp | 39 ++++++++++++++++++++
 kernel-shark-qt/src/KsQuickContextMenu.hpp |  6 +++-
 kernel-shark-qt/src/KsTraceViewer.cpp      | 20 ++++++++---
 kernel-shark-qt/src/KsUtils.cpp            | 25 +++++++++++++
 kernel-shark-qt/src/KsUtils.hpp            |  2 ++
 8 files changed, 109 insertions(+), 34 deletions(-)

-- 
2.17.1

[PATCH 1/5] kernel-shark-qt: Avoid spurious searches

[Yordan Karadzhov]

Do not search if the text field of search panel is empty.
Most probably this is an accidental key press or mouse click.

The text field gets locked only during the actual searching.

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

diff --git a/kernel-shark-qt/src/KsTraceViewer.cpp b/kernel-shark-qt/src/KsTraceViewer.cpp
index a308ea0..599b687 100644
--- a/kernel-shark-qt/src/KsTraceViewer.cpp
+++ b/kernel-shark-qt/src/KsTraceViewer.cpp
@@ -308,8 +308,6 @@ static bool matchCond(const QString &searchText, const QString &itemText)
 
 void KsTraceViewer::_search()
 {
-	/* Disable the user input until the search is done. */
-	_searchLineEdit.setReadOnly(true);
 	if (!_searchDone) {
 		int xColumn, xSelect;
 		QString xText;
@@ -319,7 +317,19 @@ void KsTraceViewer::_search()
 		 * have been modified since the last time we searched.
 		 */
 		_matchList.clear();
+
 		xText = _searchLineEdit.text();
+		if (xText.isEmpty()) {
+			/*
+			 * No text is provided by the user. Most probably this
+			 * is an accidental key press.
+			 */
+			return;
+		}
+
+		/* Disable the user input until the search is done. */
+		_searchLineEdit.setReadOnly(true);
+
 		xColumn = _columnComboBox.currentIndex();
 		xSelect = _selectComboBox.currentIndex();
 
@@ -346,6 +356,9 @@ void KsTraceViewer::_search()
 			if (_graphFollows)
 				emit select(*_it); // Send a signal to the Graph widget.
 		}
+
+		/* Enable the user input. */
+		_searchLineEdit.setReadOnly(false);
 	} else {
 		/*
 		 * If the search is done, pressing "Enter" is equivalent
@@ -353,9 +366,6 @@ void KsTraceViewer::_search()
 		 */
 		this->_next();
 	}
-
-	/* Enable the user input. */
-	_searchLineEdit.setReadOnly(false);
 }
 
 void KsTraceViewer::_next()
-- 
2.17.1

[PATCH 2/5] kernel-shark-qt: Create "Apply filter XX" checkboxes in KsUtils

[Yordan Karadzhov]

The code responsible for the creation of the "Apply filters to Graph"
and "Apply filters to List" checkboxes (showing in the "Filtering" menu),
has been moved outside of the KsMainWindow class and is now available
in KsUtils. This is done because we want to have the same checkboxes
available in the KsQuickContextMenu.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/KsMainWindow.cpp | 39 +++++++++++++---------------
 kernel-shark-qt/src/KsMainWindow.hpp |  6 ++---
 kernel-shark-qt/src/KsUtils.cpp      | 25 ++++++++++++++++++
 kernel-shark-qt/src/KsUtils.hpp      |  2 ++
 4 files changed, 47 insertions(+), 25 deletions(-)

diff --git a/kernel-shark-qt/src/KsMainWindow.cpp b/kernel-shark-qt/src/KsMainWindow.cpp
index 7213c01..142cd1f 100644
--- a/kernel-shark-qt/src/KsMainWindow.cpp
+++ b/kernel-shark-qt/src/KsMainWindow.cpp
@@ -49,9 +49,7 @@ KsMainWindow::KsMainWindow(QWidget *parent)
   _quitAction("Quit", this),
   _importFilterAction("Import Filter", this),
   _exportFilterAction("Export Filter", this),
-  _graphFilterSyncAction(this),
   _graphFilterSyncCBox(nullptr),
-  _listFilterSyncAction(this),
   _listFilterSyncCBox(nullptr),
   _showEventsAction("Show events", this),
   _showTasksAction("Show tasks", this),
@@ -292,22 +290,13 @@ void KsMainWindow::_createMenus()
 
 	/* Filter menu */
 	filter = menuBar()->addMenu("Filter");
+
+	connect(filter, 		&QMenu::aboutToShow,
+		this,			&KsMainWindow::_updateFilterMenu);
+
 	filter->addAction(&_importFilterAction);
 	filter->addAction(&_exportFilterAction);
 
-	auto lamMakeCBAction = [&](QWidgetAction *action, QString name)
-	{
-		QWidget  *containerWidget = new QWidget(filter);
-		containerWidget->setLayout(new QHBoxLayout());
-		containerWidget->layout()->setContentsMargins(FONT_WIDTH, FONT_HEIGHT/5,
-							      FONT_WIDTH, FONT_HEIGHT/5);
-		QCheckBox *checkBox = new QCheckBox(name, filter);
-		checkBox->setChecked(true);
-		containerWidget->layout()->addWidget(checkBox);
-		action->setDefaultWidget(containerWidget);
-		return checkBox;
-	};
-
 	/*
 	 * Set the default filter mask. Filter will apply to both View and
 	 * Graph.
@@ -317,20 +306,20 @@ void KsMainWindow::_createMenus()
 
 	kshark_ctx->filter_mask |= KS_EVENT_VIEW_FILTER_MASK;
 
-	_graphFilterSyncCBox = lamMakeCBAction(&_graphFilterSyncAction,
-					       "Apply filters to Graph");
+	_graphFilterSyncCBox =
+		KsUtils::addCheckBoxToMenu(filter, "Apply filters to Graph");
+	_graphFilterSyncCBox->setChecked(true);
 
 	connect(_graphFilterSyncCBox,	&QCheckBox::stateChanged,
 		this,			&KsMainWindow::_graphFilterSync);
 
-	_listFilterSyncCBox = lamMakeCBAction(&_listFilterSyncAction,
-					      "Apply filters to List");
+	_listFilterSyncCBox =
+		KsUtils::addCheckBoxToMenu(filter, "Apply filters to List");
+	_listFilterSyncCBox->setChecked(true);
 
 	connect(_listFilterSyncCBox,	&QCheckBox::stateChanged,
 		this,			&KsMainWindow::_listFilterSync);
 
-	filter->addAction(&_graphFilterSyncAction);
-	filter->addAction(&_listFilterSyncAction);
 	filter->addAction(&_showEventsAction);
 	filter->addAction(&_showTasksAction);
 	filter->addAction(&_hideTasksAction);
@@ -446,6 +435,14 @@ void KsMainWindow::_filterSyncCBoxUpdate(kshark_context *kshark_ctx)
 		_graphFilterSyncCBox->setChecked(false);
 }
 
+void KsMainWindow::_updateFilterMenu()
+{
+	kshark_context *kshark_ctx(nullptr);
+
+	if (kshark_instance(&kshark_ctx))
+		_filterSyncCBoxUpdate(kshark_ctx);
+}
+
 void KsMainWindow::_importFilter()
 {
 	kshark_context *kshark_ctx(nullptr);
diff --git a/kernel-shark-qt/src/KsMainWindow.hpp b/kernel-shark-qt/src/KsMainWindow.hpp
index 72f7059..301acc9 100644
--- a/kernel-shark-qt/src/KsMainWindow.hpp
+++ b/kernel-shark-qt/src/KsMainWindow.hpp
@@ -110,12 +110,8 @@ private:
 
 	QAction		_exportFilterAction;
 
-	QWidgetAction	_graphFilterSyncAction;
-
 	QCheckBox	*_graphFilterSyncCBox;
 
-	QWidgetAction	_listFilterSyncAction;
-
 	QCheckBox	*_listFilterSyncCBox;
 
 	QAction		_showEventsAction;
@@ -222,6 +218,8 @@ private:
 
 	void _deselect();
 
+	void _updateFilterMenu();
+
 	void _filterSyncCBoxUpdate(kshark_context *kshark_ctx);
 
 private slots:
diff --git a/kernel-shark-qt/src/KsUtils.cpp b/kernel-shark-qt/src/KsUtils.cpp
index 2ebbae3..0298010 100644
--- a/kernel-shark-qt/src/KsUtils.cpp
+++ b/kernel-shark-qt/src/KsUtils.cpp
@@ -95,6 +95,31 @@ void graphFilterSync(bool state)
 	}
 }
 
+
+/**
+ * @brief Add a checkbox to a menu.
+ *
+ * @param menu: Input location for the menu object, to which the checkbox will be added.
+ * @param name: The name of the checkbox.
+ *
+ * @returns The checkbox object;
+ */
+QCheckBox *addCheckBoxToMenu(QMenu *menu, QString name)
+{
+	QWidget  *containerWidget = new QWidget(menu);
+	containerWidget->setLayout(new QHBoxLayout());
+	containerWidget->layout()->setContentsMargins(FONT_WIDTH, FONT_HEIGHT/5,
+						      FONT_WIDTH, FONT_HEIGHT/5);
+	QCheckBox *checkBox = new QCheckBox(name, menu);
+	containerWidget->layout()->addWidget(checkBox);
+
+	QWidgetAction *action = new QWidgetAction(menu);
+	action->setDefaultWidget(containerWidget);
+	menu->addAction(action);
+
+	return checkBox;
+}
+
 /**
  * @brief Simple CPU matching function to be user for data collections.
  *
diff --git a/kernel-shark-qt/src/KsUtils.hpp b/kernel-shark-qt/src/KsUtils.hpp
index 052cc71..cb95b4f 100644
--- a/kernel-shark-qt/src/KsUtils.hpp
+++ b/kernel-shark-qt/src/KsUtils.hpp
@@ -93,6 +93,8 @@ void listFilterSync(bool state);
 
 void graphFilterSync(bool state);
 
+QCheckBox *addCheckBoxToMenu(QMenu *menu, QString name);
+
 /** @brief Convert the timestamp of the trace record into a string showing
  *	   the time in seconds.
  *
-- 
2.17.1

[PATCH 3/5] kernel-shark-qt: Improuve the KsQuickContextMenu

[Yordan Karadzhov]

In this patch the KsQuickContextMenu gets upgraded according
to the user feedback received from Steven. First of all a
"Show CPU X only" action is added to the version of the menu
that gets opened from the Table widget. In addition to this
"Apply filter to XX" check-boxes are added in order to control
the visibility of the filtered data.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/KsQuickContextMenu.cpp | 39 ++++++++++++++++++++++
 kernel-shark-qt/src/KsQuickContextMenu.hpp |  6 +++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/kernel-shark-qt/src/KsQuickContextMenu.cpp b/kernel-shark-qt/src/KsQuickContextMenu.cpp
index 815e4b9..c225269 100644
--- a/kernel-shark-qt/src/KsQuickContextMenu.cpp
+++ b/kernel-shark-qt/src/KsQuickContextMenu.cpp
@@ -50,11 +50,14 @@ KsQuickContextMenu::KsQuickContextMenu(KsDataStore *data, size_t row,
 : KsQuickMarkerMenu(dm, parent),
   _data(data),
   _row(row),
+  _graphSyncCBox(nullptr),
+  _listSyncCBox(nullptr),
   _hideTaskAction(this),
   _showTaskAction(this),
   _hideEventAction(this),
   _showEventAction(this),
   _hideCPUAction(this),
+  _showCPUAction(this),
   _addCPUPlotAction(this),
   _addTaskPlotAction(this),
   _removeCPUPlotAction(this),
@@ -85,6 +88,32 @@ KsQuickContextMenu::KsQuickContextMenu(KsDataStore *data, size_t row,
 	parentName = parent->metaObject()->className();
 
 	addSection("Pointer menu");
+
+	if (parentName == "KsTraceViewer") {
+		_graphSyncCBox =
+			KsUtils::addCheckBoxToMenu(this, "Apply filters to Graph");
+
+		connect(_graphSyncCBox,	&QCheckBox::stateChanged,
+					&KsUtils::graphFilterSync);
+
+		bool state(false);
+		KsUtils::graphFilterSync(state);
+		_graphSyncCBox->setChecked(state);
+	}
+
+	if (parentName == "KsTraceGraph" &&
+	    (graphs = dynamic_cast<KsTraceGraph *>(parent))) {
+		_listSyncCBox =
+			KsUtils::addCheckBoxToMenu(this, "Apply filters to Graph");
+
+		connect(_listSyncCBox,	&QCheckBox::stateChanged,
+					&KsUtils::listFilterSync);
+
+		bool state(false);
+		KsUtils::listFilterSync(state);
+		_listSyncCBox->setChecked(state);
+	}
+
 	descr = "Hide task [";
 	descr += taskName;
 	descr += "-";
@@ -113,6 +142,9 @@ KsQuickContextMenu::KsQuickContextMenu(KsDataStore *data, size_t row,
 	lamAddAction(&_hideCPUAction, &KsQuickContextMenu::_hideCPU);
 
 	if (parentName == "KsTraceViewer") {
+		descr = QString("Show CPU [%1] only").arg(cpu);
+		lamAddAction(&_showCPUAction, &KsQuickContextMenu::_showCPU);
+
 		descr = "Add [";
 		descr += taskName;
 		descr += "-";
@@ -198,6 +230,13 @@ void KsQuickContextMenu::_showEvent()
 	_data->applyPosEventFilter(QVector<int>(1, eventId));
 }
 
+void KsQuickContextMenu::_showCPU()
+{
+	int cpu = _data->rows()[_row]->cpu;
+
+	_data->applyPosCPUFilter(QVector<int>(1, cpu));
+}
+
 void KsQuickContextMenu::_hideCPU()
 {
 	kshark_context *kshark_ctx(nullptr);
diff --git a/kernel-shark-qt/src/KsQuickContextMenu.hpp b/kernel-shark-qt/src/KsQuickContextMenu.hpp
index 6ca1b08..f5a2a78 100644
--- a/kernel-shark-qt/src/KsQuickContextMenu.hpp
+++ b/kernel-shark-qt/src/KsQuickContextMenu.hpp
@@ -71,6 +71,8 @@ private:
 
 	void _showEvent();
 
+	void _showCPU();
+
 	void _hideCPU();
 
 	void _addCPUPlot();
@@ -87,11 +89,13 @@ private:
 
 	size_t		_row;
 
+	QCheckBox	*_graphSyncCBox, *_listSyncCBox;
+
 	QAction _hideTaskAction, _showTaskAction;
 
 	QAction _hideEventAction, _showEventAction;
 
-	QAction _hideCPUAction;
+	QAction _hideCPUAction, _showCPUAction;
 
 	QAction _addCPUPlotAction;
 
-- 
2.17.1

[PATCH 4/5] kernel-shark-qt: Update the documentation link

[Yordan Karadzhov]

Help->Contents now opens http://kernelshark.org/

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

diff --git a/kernel-shark-qt/src/KsMainWindow.cpp b/kernel-shark-qt/src/KsMainWindow.cpp
index 142cd1f..a375126 100644
--- a/kernel-shark-qt/src/KsMainWindow.cpp
+++ b/kernel-shark-qt/src/KsMainWindow.cpp
@@ -820,7 +820,7 @@ void KsMainWindow::_aboutInfo()
 
 void KsMainWindow::_contents()
 {
-	QDesktopServices::openUrl(QUrl("https://www.google.bg/search?q=kernelshark",
+	QDesktopServices::openUrl(QUrl("http://kernelshark.org/",
 				  QUrl::TolerantMode));
 }
 
-- 
2.17.1

[PATCH 5/5] kernel-shark-qt: Version 1.0.0

[Yordan Karadzhov]

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

diff --git a/kernel-shark-qt/CMakeLists.txt b/kernel-shark-qt/CMakeLists.txt
index d92bc3d..46137a3 100644
--- a/kernel-shark-qt/CMakeLists.txt
+++ b/kernel-shark-qt/CMakeLists.txt
@@ -4,8 +4,8 @@ cmake_minimum_required(VERSION 2.8.11 FATAL_ERROR)
 # Set the name and version of the project
 project(kernel-shark-qt)
 
-set(KS_VERSION_MAJOR 0)
-set(KS_VERSION_MINOR 9)
+set(KS_VERSION_MAJOR 1)
+set(KS_VERSION_MINOR 0)
 set(KS_VERSION_PATCH 0)
 set(KS_VERSION_STRING ${KS_VERSION_MAJOR}.${KS_VERSION_MINOR}.${KS_VERSION_PATCH})
 message("\n project: Kernel Shark: (version: ${KS_VERSION_STRING})\n")
-- 
2.17.1

Re: [PATCH 1/5] kernel-shark-qt: Avoid spurious searches

[Steven Rostedt]

On Wed, 12 Dec 2018 16:58:44 +0000
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> Do not search if the text field of search panel is empty.
> Most probably this is an accidental key press or mouse click.
> 
> The text field gets locked only during the actual searching.
> 
> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
> ---
>  kernel-shark-qt/src/KsTraceViewer.cpp | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel-shark-qt/src/KsTraceViewer.cpp b/kernel-shark-qt/src/KsTraceViewer.cpp
> index a308ea0..599b687 100644
> --- a/kernel-shark-qt/src/KsTraceViewer.cpp
> +++ b/kernel-shark-qt/src/KsTraceViewer.cpp
> @@ -308,8 +308,6 @@ static bool matchCond(const QString &searchText, const QString &itemText)
>  
>  void KsTraceViewer::_search()
>  {
> -	/* Disable the user input until the search is done. */
> -	_searchLineEdit.setReadOnly(true);

Hmm, can any races happen by setting the search line read only after
the empty checks?

Also, I found a bug. Try this on a large data file. It doesn't matter
what data file it is.

With the Search: at its start up defaults (with Column = "#"), type
"aaa" in the search window and then hit enter. While the search is
going on, hit the "Next" button. See what happens.

-- Steve


>  	if (!_searchDone) {
>  		int xColumn, xSelect;
>  		QString xText;
> @@ -319,7 +317,19 @@ void KsTraceViewer::_search()
>  		 * have been modified since the last time we searched.
>  		 */
>  		_matchList.clear();
> +
>  		xText = _searchLineEdit.text();
> +		if (xText.isEmpty()) {
> +			/*
> +			 * No text is provided by the user. Most probably this
> +			 * is an accidental key press.
> +			 */
> +			return;
> +		}
> +
> +		/* Disable the user input until the search is done. */
> +		_searchLineEdit.setReadOnly(true);
> +
>  		xColumn = _columnComboBox.currentIndex();
>  		xSelect = _selectComboBox.currentIndex();
>  
> @@ -346,6 +356,9 @@ void KsTraceViewer::_search()
>  			if (_graphFollows)
>  				emit select(*_it); // Send a signal to the Graph widget.
>  		}
> +
> +		/* Enable the user input. */
> +		_searchLineEdit.setReadOnly(false);
>  	} else {
>  		/*
>  		 * If the search is done, pressing "Enter" is equivalent
> @@ -353,9 +366,6 @@ void KsTraceViewer::_search()
>  		 */
>  		this->_next();
>  	}
> -
> -	/* Enable the user input. */
> -	_searchLineEdit.setReadOnly(false);
>  }
>  
>  void KsTraceViewer::_next()

Re: [PATCH 1/5] kernel-shark-qt: Avoid spurious searches

[Yordan Karadzhov (VMware)]

On 14.12.18 г. 5:47 ч., Steven Rostedt wrote:
> On Wed, 12 Dec 2018 16:58:44 +0000
> Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
> 
>> Do not search if the text field of search panel is empty.
>> Most probably this is an accidental key press or mouse click.
>>
>> The text field gets locked only during the actual searching.
>>
>> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
>> ---
>>   kernel-shark-qt/src/KsTraceViewer.cpp | 20 +++++++++++++++-----
>>   1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel-shark-qt/src/KsTraceViewer.cpp b/kernel-shark-qt/src/KsTraceViewer.cpp
>> index a308ea0..599b687 100644
>> --- a/kernel-shark-qt/src/KsTraceViewer.cpp
>> +++ b/kernel-shark-qt/src/KsTraceViewer.cpp
>> @@ -308,8 +308,6 @@ static bool matchCond(const QString &searchText, const QString &itemText)
>>   
>>   void KsTraceViewer::_search()
>>   {
>> -	/* Disable the user input until the search is done. */
>> -	_searchLineEdit.setReadOnly(true);
> 
> Hmm, can any races happen by setting the search line read only after
> the empty checks?
> 
> Also, I found a bug. Try this on a large data file. It doesn't matter
> what data file it is.
> 
> With the Search: at its start up defaults (with Column = "#"), type
> "aaa" in the search window and then hit enter. While the search is
> going on, hit the "Next" button. See what happens.
Deadlock :)

Well spotted, thanks a lot!
I will resend the whole series.

Yordan