[PATCH v2 0/8] More modifications toward KS 1.0

[PATCH v2 0/8] More modifications toward KS 1.0

[Yordan Karadzhov]

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

This is the version 2 of this series of patches. The major changes
from v1 are in patches 1-3 (new patches). These patches aim to address
the deadlock problem reported by Steven.

Yordan Karadzhov (8):
  kernel-shark-qt: Lock completely the searching panel when searching
  kernel-shark-qt: Fix a simple bug in KsTraceViewer::_searchReset()
  kernel-shark-qt: Make the parallelized search stoppable
  kernel-shark-qt: Avoid spurious searches
  kernel-shark-qt: Create "Apply filter XX" checkboxes in KsUtils
  kernel-shark-qt: Improve 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/KsModels.cpp           |  6 ++-
 kernel-shark-qt/src/KsModels.hpp           |  5 +-
 kernel-shark-qt/src/KsQuickContextMenu.cpp | 39 +++++++++++++++
 kernel-shark-qt/src/KsQuickContextMenu.hpp |  6 ++-
 kernel-shark-qt/src/KsTraceViewer.cpp      | 57 ++++++++++++++++------
 kernel-shark-qt/src/KsTraceViewer.hpp      |  2 +
 kernel-shark-qt/src/KsUtils.cpp            | 25 ++++++++++
 kernel-shark-qt/src/KsUtils.hpp            |  2 +
 11 files changed, 146 insertions(+), 47 deletions(-)

-- 
2.17.1

[PATCH v2 1/8] kernel-shark-qt: Lock completely the searching panel when searching

[Yordan Karadzhov]

So far, when searching we lock only the text field of the  searching
panel. This may create a deadlock (as reported by Steven) in the case
when the user presses "Next" or "Prev." button in the same time when
a parallelized search is in progress. This patch aims to protect
against such a deadlock by locking all components of the panel, except
the "Stop search" button.
The text panel gets locked only during the actual searching.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/KsTraceViewer.cpp | 27 +++++++++++++++++++--------
 kernel-shark-qt/src/KsTraceViewer.hpp |  2 ++
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/kernel-shark-qt/src/KsTraceViewer.cpp b/kernel-shark-qt/src/KsTraceViewer.cpp
index a308ea0..afb9892 100644
--- a/kernel-shark-qt/src/KsTraceViewer.cpp
+++ b/kernel-shark-qt/src/KsTraceViewer.cpp
@@ -306,18 +306,29 @@ static bool matchCond(const QString &searchText, const QString &itemText)
 	return (itemText.compare(searchText, Qt::CaseInsensitive) == 0);
 }
 
+void KsTraceViewer::_lockSearchPanel(bool lock)
+{
+	_columnComboBox.setEnabled(!lock);
+	_selectComboBox.setEnabled(!lock);
+	_searchLineEdit.setReadOnly(lock);
+	_prevButton.setEnabled(!lock);
+	_nextButton.setEnabled(!lock);
+	_graphFollowsCheckBox.setEnabled(!lock);
+}
+
 void KsTraceViewer::_search()
 {
-	/* Disable the user input until the search is done. */
-	_searchLineEdit.setReadOnly(true);
 	if (!_searchDone) {
-		int xColumn, xSelect;
-		QString xText;
-
 		/*
 		 * The search is not done. This means that the search settings
 		 * have been modified since the last time we searched.
 		 */
+		int xColumn, xSelect;
+		QString xText;
+
+		/* Disable the user input until the search is done. */
+		_lockSearchPanel(true);
+
 		_matchList.clear();
 		xText = _searchLineEdit.text();
 		xColumn = _columnComboBox.currentIndex();
@@ -346,6 +357,9 @@ void KsTraceViewer::_search()
 			if (_graphFollows)
 				emit select(*_it); // Send a signal to the Graph widget.
 		}
+
+		/* Enable the user input. */
+		_lockSearchPanel(false);
 	} else {
 		/*
 		 * If the search is done, pressing "Enter" is equivalent
@@ -353,9 +367,6 @@ void KsTraceViewer::_search()
 		 */
 		this->_next();
 	}
-
-	/* Enable the user input. */
-	_searchLineEdit.setReadOnly(false);
 }
 
 void KsTraceViewer::_next()
diff --git a/kernel-shark-qt/src/KsTraceViewer.hpp b/kernel-shark-qt/src/KsTraceViewer.hpp
index 4e35c17..a89fce1 100644
--- a/kernel-shark-qt/src/KsTraceViewer.hpp
+++ b/kernel-shark-qt/src/KsTraceViewer.hpp
@@ -147,6 +147,8 @@ private:
 
 	void _graphFollowsChanged(int);
 
+	void _lockSearchPanel(bool lock);
+
 	void _search();
 
 	void _next();
-- 
2.17.1

[PATCH v2 2/8] kernel-shark-qt: Fix a simple bug in KsTraceViewer::_searchReset()

[Yordan Karadzhov]

Most probably this is a copy-and-paste bug.

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

diff --git a/kernel-shark-qt/src/KsTraceViewer.cpp b/kernel-shark-qt/src/KsTraceViewer.cpp
index afb9892..7f0f1e2 100644
--- a/kernel-shark-qt/src/KsTraceViewer.cpp
+++ b/kernel-shark-qt/src/KsTraceViewer.cpp
@@ -223,7 +223,7 @@ void KsTraceViewer::_searchReset()
 {
 	_searchProgBar.setValue(0);
 	_searchCountLabel.setText("");
-	_proxyModel.searchProgress();
+	_proxyModel.searchReset();
 	_searchDone = false;
 }
 
-- 
2.17.1

[PATCH v2 3/8] kernel-shark-qt: Make the parallelized search stoppable

[Yordan Karadzhov]

So far only the single-threaded search inside the "Latency" and "Info"
columns can be stopped by the "stop search" button. This patch makes it
possible to stop also the parallelized search. Note that after stopping
the parallelized search, the list of matches will contain entries which
may not be consecutive, because each thread was searching in its own
subset of the data and was stoped at an arbitrary position in this
subset.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/KsModels.cpp      |  6 +++++-
 kernel-shark-qt/src/KsModels.hpp      |  5 ++++-
 kernel-shark-qt/src/KsTraceViewer.cpp | 19 ++++++++++++-------
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/kernel-shark-qt/src/KsModels.cpp b/kernel-shark-qt/src/KsModels.cpp
index d67ee62..c8cc410 100644
--- a/kernel-shark-qt/src/KsModels.cpp
+++ b/kernel-shark-qt/src/KsModels.cpp
@@ -80,7 +80,11 @@ void KsFilterProxyModel::_search(int column,
 			matchList->append(row);
 
 		if (_searchStop) {
-			_searchStop = false;
+			if (notify) {
+				_searchProgress = KS_PROGRESS_BAR_MAX;
+				_pbCond.notify_one();
+			}
+
 			break;
 		}
 
diff --git a/kernel-shark-qt/src/KsModels.hpp b/kernel-shark-qt/src/KsModels.hpp
index b66c259..08019e7 100644
--- a/kernel-shark-qt/src/KsModels.hpp
+++ b/kernel-shark-qt/src/KsModels.hpp
@@ -178,7 +178,10 @@ public:
 	int searchProgress() const {return _searchProgress;}
 
 	/** Reset the progress value of the search. */
-	void searchReset() {_searchProgress = 0;}
+	void searchReset() {
+		_searchProgress = 0;
+		_searchStop = false;
+	}
 
 	/** Stop the serch for all threads. */
 	void searchStop() {_searchStop = true;}
diff --git a/kernel-shark-qt/src/KsTraceViewer.cpp b/kernel-shark-qt/src/KsTraceViewer.cpp
index 7f0f1e2..aeed5f7 100644
--- a/kernel-shark-qt/src/KsTraceViewer.cpp
+++ b/kernel-shark-qt/src/KsTraceViewer.cpp
@@ -446,6 +446,7 @@ void KsTraceViewer::_searchStop()
 {
 	_searchStopAction->setVisible(false);
 	_proxyModel.searchStop();
+	_lockSearchPanel(false);
 }
 
 void KsTraceViewer::_clicked(const QModelIndex& i)
@@ -625,7 +626,6 @@ size_t KsTraceViewer::_searchItems(int column,
 {
 	int count, dataRow;
 
-	_searchProgBar.show();
 	_pbAction->setVisible(true);
 
 	if (_proxyModel.rowCount({}) < KS_SEARCH_SHOW_PROGRESS_MIN) {
@@ -635,15 +635,20 @@ size_t KsTraceViewer::_searchItems(int column,
 		 */
 		_proxyModel.search(column, searchText, cond, &_matchList,
 				   nullptr, nullptr);
-	} else if (column == KsViewModel::TRACE_VIEW_COL_INFO ||
-	    column == KsViewModel::TRACE_VIEW_COL_LAT) {
+	} else {
 		_searchStopAction->setVisible(true);
-		_proxyModel.search(column, searchText, cond, &_matchList,
-				   &_searchProgBar, &_searchCountLabel);
+
+		if (column == KsViewModel::TRACE_VIEW_COL_INFO ||
+		    column == KsViewModel::TRACE_VIEW_COL_LAT) {
+			_proxyModel.search(column, searchText,
+					   cond, &_matchList,
+					   &_searchProgBar,
+					   &_searchCountLabel);
+		} else {
+			_searchItemsMapReduce(column, searchText, cond);
+		}
 
 		_searchStopAction->setVisible(false);
-	} else {
-		_searchItemsMapReduce(column, searchText, cond);
 	}
 
 	count = _matchList.count();
-- 
2.17.1

[PATCH v2 4/8] kernel-shark-qt: Avoid spurious searches

[Yordan Karadzhov]

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

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

diff --git a/kernel-shark-qt/src/KsTraceViewer.cpp b/kernel-shark-qt/src/KsTraceViewer.cpp
index aeed5f7..971793f 100644
--- a/kernel-shark-qt/src/KsTraceViewer.cpp
+++ b/kernel-shark-qt/src/KsTraceViewer.cpp
@@ -331,6 +331,15 @@ void KsTraceViewer::_search()
 
 		_matchList.clear();
 		xText = _searchLineEdit.text();
+		if (xText.isEmpty()) {
+			/*
+			 * No text is provided by the user. Most probably this
+			 * is an accidental key press. Just reenable the input.
+			 */
+			_lockSearchPanel(false);
+			return;
+		}
+
 		xColumn = _columnComboBox.currentIndex();
 		xSelect = _selectComboBox.currentIndex();
 
-- 
2.17.1

[PATCH v2 5/8] 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 v2 6/8] kernel-shark-qt: Improve 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 v2 7/8] 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 v2 8/8] 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 v2 1/8] kernel-shark-qt: Lock completely the searching panel when searching

[Steven Rostedt]

On Fri, 14 Dec 2018 12:52:34 +0000
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> So far, when searching we lock only the text field of the  searching
> panel. This may create a deadlock (as reported by Steven) in the case
> when the user presses "Next" or "Prev." button in the same time when
> a parallelized search is in progress. This patch aims to protect
> against such a deadlock by locking all components of the panel, except
> the "Stop search" button.
> The text panel gets locked only during the actual searching.
> 
> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
> ---
>  kernel-shark-qt/src/KsTraceViewer.cpp | 27 +++++++++++++++++++--------
>  kernel-shark-qt/src/KsTraceViewer.hpp |  2 ++
>  2 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel-shark-qt/src/KsTraceViewer.cpp b/kernel-shark-qt/src/KsTraceViewer.cpp
> index a308ea0..afb9892 100644
> --- a/kernel-shark-qt/src/KsTraceViewer.cpp
> +++ b/kernel-shark-qt/src/KsTraceViewer.cpp
> @@ -306,18 +306,29 @@ static bool matchCond(const QString &searchText, const QString &itemText)
>  	return (itemText.compare(searchText, Qt::CaseInsensitive) == 0);
>  }
>  
> +void KsTraceViewer::_lockSearchPanel(bool lock)
> +{
> +	_columnComboBox.setEnabled(!lock);
> +	_selectComboBox.setEnabled(!lock);
> +	_searchLineEdit.setReadOnly(lock);
> +	_prevButton.setEnabled(!lock);
> +	_nextButton.setEnabled(!lock);
> +	_graphFollowsCheckBox.setEnabled(!lock);
> +}

Can we add two helper functions and use that instead?

void KsTraceViewer::_searchPanelLock(void)
{
	_lockSearchPanel(true);
}

void KsTraceViewer::_searchPanelUnlock(void)
{
	_lockSearchPanel(false);
}

This its more in line with the lock / unlock paradigm than passing in
true and false.

Thanks!

-- Steve


> +
>  void KsTraceViewer::_search()
>  {
> -	/* Disable the user input until the search is done. */
> -	_searchLineEdit.setReadOnly(true);
>  	if (!_searchDone) {
> -		int xColumn, xSelect;
> -		QString xText;
> -
>  		/*
>  		 * The search is not done. This means that the search settings
>  		 * have been modified since the last time we searched.
>  		 */
> +		int xColumn, xSelect;
> +		QString xText;
> +
> +		/* Disable the user input until the search is done. */
> +		_lockSearchPanel(true);
> +
>  		_matchList.clear();
>  		xText = _searchLineEdit.text();
>  		xColumn = _columnComboBox.currentIndex();
> @@ -346,6 +357,9 @@ void KsTraceViewer::_search()
>  			if (_graphFollows)
>  				emit select(*_it); // Send a signal to the Graph widget.
>  		}
> +
> +		/* Enable the user input. */
> +		_lockSearchPanel(false);
>  	} else {
>  		/*
>  		 * If the search is done, pressing "Enter" is equivalent
> @@ -353,9 +367,6 @@ void KsTraceViewer::_search()
>  		 */
>  		this->_next();
>  	}
> -
> -	/* Enable the user input. */
> -	_searchLineEdit.setReadOnly(false);
>  }
>  
>  void KsTraceViewer::_next()
> diff --git a/kernel-shark-qt/src/KsTraceViewer.hpp b/kernel-shark-qt/src/KsTraceViewer.hpp
> index 4e35c17..a89fce1 100644
> --- a/kernel-shark-qt/src/KsTraceViewer.hpp
> +++ b/kernel-shark-qt/src/KsTraceViewer.hpp
> @@ -147,6 +147,8 @@ private:
>  
>  	void _graphFollowsChanged(int);
>  
> +	void _lockSearchPanel(bool lock);
> +
>  	void _search();
>  
>  	void _next();

Re: [PATCH v2 1/8] kernel-shark-qt: Lock completely the searching panel when searching

[Steven Rostedt]

On Fri, 14 Dec 2018 09:50:37 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 14 Dec 2018 12:52:34 +0000
> Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
> 
> > So far, when searching we lock only the text field of the  searching
> > panel. This may create a deadlock (as reported by Steven) in the case

BTW, we should probably use the normal way of documenting "reported
by", which would be (see below).

> > when the user presses "Next" or "Prev." button in the same time when
> > a parallelized search is in progress. This patch aims to protect
> > against such a deadlock by locking all components of the panel, except
> > the "Stop search" button.
> > The text panel gets locked only during the actual searching.
> > 

Reporte-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

> > Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
> > ---
> >  kernel-shark-qt/src/KsTraceViewer.cpp | 27 +++++++++++++++++++--------
> >  kernel-shark-qt/src/KsTraceViewer.hpp |  2 ++
> >  2 files changed, 21 insertions(+), 8 deletions(-)
> > 


> Can we add two helper functions and use that instead?
> 
> void KsTraceViewer::_searchPanelLock(void)
> {
> 	_lockSearchPanel(true);
> }
> 
> void KsTraceViewer::_searchPanelUnlock(void)
> {
> 	_lockSearchPanel(false);
> }
> 
> This its more in line with the lock / unlock paradigm than passing in
> true and false.
> 

I may apply this series anyway, and we could just add the helper
functions in a separate patch.

-- Steve

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

[Steven Rostedt]

On Fri, 14 Dec 2018 12:52:39 +0000
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> 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.
> 

I applied this patch and it shows the "Hide CPU [x]". But it's a bit
flaky. The first time I tried it, it didn't do anything. Then after
starting kernelshark again, I was able to have it hide the CPU, but
when I clicked on the "Filter" menu on the top toolbar, the filter went
away.

Another thing, the "Apply filters to Graph" checkbox is a bit
confusing. What does it actually mean? The filters are applied by
default to the graph, correct? But the checkbox isn't set.

Also, does it make sense to have that "Apply filters to graph" checkbox
in the Graph menu. It would make more sense if it said "Apply filters
to table".

I think I'll apply this series except for this patch and the last one
(5 and 8).

-- Steve

Re: [PATCH v2 0/8] More modifications toward KS 1.0

[Steven Rostedt]

On Fri, 14 Dec 2018 12:52:33 +0000
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> This series of patches contains the last (hopefully) modifications
> needed before releasing KernelShark 1.0.
> 
> This is the version 2 of this series of patches. The major changes
> from v1 are in patches 1-3 (new patches). These patches aim to address
> the deadlock problem reported by Steven.
> 
> Yordan Karadzhov (8):
>   kernel-shark-qt: Lock completely the searching panel when searching
>   kernel-shark-qt: Fix a simple bug in KsTraceViewer::_searchReset()
>   kernel-shark-qt: Make the parallelized search stoppable
>   kernel-shark-qt: Avoid spurious searches
>   kernel-shark-qt: Create "Apply filter XX" checkboxes in KsUtils
>   kernel-shark-qt: Improve the KsQuickContextMenu
>   kernel-shark-qt: Update the documentation link
>   kernel-shark-qt: Version 1.0.0
> 

One thing I found annoying, and I'm not sure how to stop this, but the
menu can be awfully touchy. When I'm on the table, and right click
(not hold, just click), it will open up the menu and then select
whatever menu item that the mouse was on, usually hiding the row I'm on.

One solution is to have the mouse be on something that it can not
select immediately. Like "Pointer menu".

Speaking of which, when selecting on the graph, not on an event, where
the menu is:

"Plots 
  Remove [CPU 0]"

the menu is way above the mouse. We probably want the menu to pop up
where the mouse is over "Plots".

-- Steve
  

Re: [PATCH v2 3/8] kernel-shark-qt: Make the parallelized search stoppable

[Steven Rostedt]

On Fri, 14 Dec 2018 12:52:37 +0000
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> So far only the single-threaded search inside the "Latency" and "Info"
> columns can be stopped by the "stop search" button. This patch makes it
> possible to stop also the parallelized search. Note that after stopping
> the parallelized search, the list of matches will contain entries which
> may not be consecutive, because each thread was searching in its own
> subset of the data and was stoped at an arbitrary position in this
> subset.
> 

Also I noticed:

  _searchStopButton(QIcon::fromTheme("process-stop"), "", this),

I don't have a "process-stop" theme. I'm not running KDE. All I see is
a blank button. Is there a way to add text to it too, or define the
theme if it is not set?

-- Steve

Re: [PATCH v2 3/8] kernel-shark-qt: Make the parallelized search stoppable

[Yordan Karadzhov (VMware)]

On 14.12.18 г. 19:17 ч., Steven Rostedt wrote:
> On Fri, 14 Dec 2018 12:52:37 +0000
> Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
> 
>> So far only the single-threaded search inside the "Latency" and "Info"
>> columns can be stopped by the "stop search" button. This patch makes it
>> possible to stop also the parallelized search. Note that after stopping
>> the parallelized search, the list of matches will contain entries which
>> may not be consecutive, because each thread was searching in its own
>> subset of the data and was stoped at an arbitrary position in this
>> subset.
>>
> 
> Also I noticed:
> 
>    _searchStopButton(QIcon::fromTheme("process-stop"), "", this),
> 
> I don't have a "process-stop" theme. I'm not running KDE. All I see is
> a blank button. Is there a way to add text to it too, or define the
> theme if it is not set?
> 

This is strange. This line means that the button have to show the 
"process-stop" icon and the version (look) of the icon has to be 
determined form your current theme. Everywhere in the code I use the 
standardized icon names provided here

https://specifications.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html 


Looks like for some reason your theme does not provide this icon.

Thanks!
Yordan

> -- Steve
> 

Re: [PATCH v2 0/8] More modifications toward KS 1.0

[Yordan Karadzhov (VMware)]

On 14.12.18 г. 19:10 ч., Steven Rostedt wrote:
> On Fri, 14 Dec 2018 12:52:33 +0000
> Yordan Karadzhov<ykaradzhov@vmware.com>  wrote:
> 
>> This series of patches contains the last (hopefully) modifications
>> needed before releasing KernelShark 1.0.
>>
>> This is the version 2 of this series of patches. The major changes
>> from v1 are in patches 1-3 (new patches). These patches aim to address
>> the deadlock problem reported by Steven.
>>
>> Yordan Karadzhov (8):
>>    kernel-shark-qt: Lock completely the searching panel when searching
>>    kernel-shark-qt: Fix a simple bug in KsTraceViewer::_searchReset()
>>    kernel-shark-qt: Make the parallelized search stoppable
>>    kernel-shark-qt: Avoid spurious searches
>>    kernel-shark-qt: Create "Apply filter XX" checkboxes in KsUtils
>>    kernel-shark-qt: Improve the KsQuickContextMenu
>>    kernel-shark-qt: Update the documentation link
>>    kernel-shark-qt: Version 1.0.0
>>
> One thing I found annoying, and I'm not sure how to stop this, but the
> menu can be awfully touchy. When I'm on the table, and right click
> (not hold, just click), it will open up the menu and then select
> whatever menu item that the mouse was on, usually hiding the row I'm on.
> 
> One solution is to have the mouse be on something that it can not
> select immediately. Like "Pointer menu".
> 
> Speaking of which, when selecting on the graph, not on an event, where
> the menu is:
> 
> "Plots
>    Remove [CPU 0]"
> 
> the menu is way above the mouse. We probably want the menu to pop up
> where the mouse is over "Plots".
> 

Hi Steve,
Thanks a lot for reporting those problems. I will send patches fixing 
the problems.

Yordan


> -- Steve
>    

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

[Yordan Karadzhov (VMware)]

On 14.12.18 г. 19:01 ч., Steven Rostedt wrote:
> On Fri, 14 Dec 2018 12:52:39 +0000
> Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
> 
>> 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.
>>
> 
> I applied this patch and it shows the "Hide CPU [x]". But it's a bit
> flaky. The first time I tried it, it didn't do anything. Then after
> starting kernelshark again, I was able to have it hide the CPU, but
> when I clicked on the "Filter" menu on the top toolbar, the filter went
> away.

Interesting, I have an idea why this may happen, but I am not sure.


> 
> Another thing, the "Apply filters to Graph" checkbox is a bit
> confusing. What does it actually mean? The filters are applied by
> default to the graph, correct? But the checkbox isn't set.
> 
> Also, does it make sense to have that "Apply filters to graph" checkbox
> in the Graph menu. It would make more sense if it said "Apply filters
> to table".

You are right. There is a typo in the name of the checkbox. But it does 
the correct thing.

I will fix the two problems and will resend the patch.

Thanks a lot!
Yordan


> 
> I think I'll apply this series except for this patch and the last one
> (5 and 8).
> 
> -- Steve
>