linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Make Sched event plugin use its own data collections
@ 2018-11-30 15:42 Yordan Karadzhov
  2018-11-30 15:42 ` [PATCH 1/2] kernel-shark-qt: Add a method for adding a new collection to a list Yordan Karadzhov
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Yordan Karadzhov @ 2018-11-30 15:42 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

These two patches aim to restore the log(n) time complexity of
the latency plotting, by adding Data Collections for Sched events.

Steven, please try playing with this modification before applying it.
If you can test this, when doing some real work this would be great.

Yordan Karadzhov (2):
  kernel-shark-qt: Add a method for adding a new collection to a list
  kernel-shark-qt: Make Sched event plugin use its own data collections

 kernel-shark-qt/src/libkshark-collection.c  | 45 ++++++++++++++++++++-
 kernel-shark-qt/src/libkshark.c             |  1 +
 kernel-shark-qt/src/libkshark.h             |  9 +++++
 kernel-shark-qt/src/plugins/SchedEvents.cpp | 33 ++++++---------
 kernel-shark-qt/src/plugins/sched_events.c  | 21 ++++++++++
 kernel-shark-qt/src/plugins/sched_events.h  |  6 +++
 6 files changed, 93 insertions(+), 22 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] kernel-shark-qt: Add a method for adding a new collection to a list
  2018-11-30 15:42 [PATCH 0/2] Make Sched event plugin use its own data collections Yordan Karadzhov
@ 2018-11-30 15:42 ` Yordan Karadzhov
  2018-11-30 15:42 ` [PATCH 2/2] kernel-shark-qt: Make Sched event plugin use its own data collections Yordan Karadzhov
  2018-11-30 16:12 ` [PATCH 0/2] " Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Yordan Karadzhov @ 2018-11-30 15:42 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

So far we have only one method that creates a new Data Collection.
It is kshark_register_data_collection(). The problem with this method
is that it automatically adds the new collection to the list of
collections, owned by the context of the session. This patch adds
a more generic method, which can be used to add new data collection
to a given list. The method will be used by the Sched event plugin,
which will keep its own list of collections, relevant only for the
plugin.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/libkshark-collection.c | 45 +++++++++++++++++++++-
 kernel-shark-qt/src/libkshark.c            |  1 +
 kernel-shark-qt/src/libkshark.h            |  9 +++++
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/kernel-shark-qt/src/libkshark-collection.c b/kernel-shark-qt/src/libkshark-collection.c
index 9838042..c70da1e 100644
--- a/kernel-shark-qt/src/libkshark-collection.c
+++ b/kernel-shark-qt/src/libkshark-collection.c
@@ -781,14 +781,55 @@ kshark_register_data_collection(struct kshark_context *kshark_ctx,
 {
 	struct kshark_entry_collection *col;
 
+	col = kshark_add_collection_to_list(kshark_ctx,
+					    &kshark_ctx->collections,
+					    data, n_rows,
+					    cond, val,
+					    margin);
+
+	return col;
+}
+
+/**
+ * @brief Allocate and process data collection, defined with a given Matching
+ *	  condition function and value. Add this collection to a given list of
+ *	  collections.
+ *
+ * @param kshark_ctx: Input location for the session context pointer.
+ * @param col_list: Input location for the list of collections.
+ * @param data: Input location for the trace data.
+ * @param n_rows: The size of the inputted data.
+ * @param cond: Matching condition function for the collection to be
+ *	        registered.
+ * @param val: Matching condition value of for collection to be registered.
+ * @param margin: The size of the additional (margin) data which do not
+ *		  satisfy the matching condition, but is added at the
+ *		  beginning and at the end of each interval of the collection
+ *		  as well as at the beginning and at the end of data-set. If
+ *		  "0", no margin data is added.
+ *
+ * @returns Pointer to the registered Data collections on success, or NULL
+ *	    on failure.
+ */
+struct kshark_entry_collection *
+kshark_add_collection_to_list(struct kshark_context *kshark_ctx,
+			      struct kshark_entry_collection **col_list,
+			      struct kshark_entry **data,
+			      size_t n_rows,
+			      matching_condition_func cond,
+			      int val,
+			      size_t margin)
+{
+	struct kshark_entry_collection *col;
+
 	col = kshark_data_collection_alloc(kshark_ctx, data,
 					   0, n_rows,
 					   cond, val,
 					   margin);
 
 	if (col) {
-		col->next = kshark_ctx->collections;
-		kshark_ctx->collections = col;
+		col->next = *col_list;
+		*col_list = col;
 	}
 
 	return col;
diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
index 86d3e58..598ea52 100644
--- a/kernel-shark-qt/src/libkshark.c
+++ b/kernel-shark-qt/src/libkshark.c
@@ -33,6 +33,7 @@ static bool kshark_default_context(struct kshark_context **context)
 		return false;
 
 	kshark_ctx->event_handlers = NULL;
+	kshark_ctx->collections = NULL;
 	kshark_ctx->plugins = NULL;
 
 	kshark_ctx->show_task_filter = tracecmd_filter_id_hash_alloc();
diff --git a/kernel-shark-qt/src/libkshark.h b/kernel-shark-qt/src/libkshark.h
index b3bd646..7d1edfc 100644
--- a/kernel-shark-qt/src/libkshark.h
+++ b/kernel-shark-qt/src/libkshark.h
@@ -412,6 +412,15 @@ struct kshark_entry_collection {
 	size_t size;
 };
 
+struct kshark_entry_collection *
+kshark_add_collection_to_list(struct kshark_context *kshark_ctx,
+			      struct kshark_entry_collection **col_list,
+			      struct kshark_entry **data,
+			      size_t n_rows,
+			      matching_condition_func cond,
+			      int val,
+			      size_t margin);
+
 struct kshark_entry_collection *
 kshark_register_data_collection(struct kshark_context *kshark_ctx,
 				struct kshark_entry **data, size_t n_rows,
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] kernel-shark-qt: Make Sched event plugin use its own data collections
  2018-11-30 15:42 [PATCH 0/2] Make Sched event plugin use its own data collections Yordan Karadzhov
  2018-11-30 15:42 ` [PATCH 1/2] kernel-shark-qt: Add a method for adding a new collection to a list Yordan Karadzhov
@ 2018-11-30 15:42 ` Yordan Karadzhov
  2018-11-30 16:12 ` [PATCH 0/2] " Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Yordan Karadzhov @ 2018-11-30 15:42 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

The Sched events plugin now mentains its own list of per-task
data collections of Sched events. Since this restors the log(n)
complexity of the latency plotting, we no longer need the  cut
which disables plotting if the number of entries shown by the
graph is bigger than PLUGIN_MAX_ENTRIES.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/plugins/SchedEvents.cpp | 33 ++++++++-------------
 kernel-shark-qt/src/plugins/sched_events.c  | 21 +++++++++++++
 kernel-shark-qt/src/plugins/sched_events.h  |  6 ++++
 3 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/kernel-shark-qt/src/plugins/SchedEvents.cpp b/kernel-shark-qt/src/plugins/SchedEvents.cpp
index c8dc3bf..8408657 100644
--- a/kernel-shark-qt/src/plugins/SchedEvents.cpp
+++ b/kernel-shark-qt/src/plugins/SchedEvents.cpp
@@ -28,8 +28,6 @@
 
 #define PLUGIN_MIN_BOX_SIZE 4
 
-#define PLUGIN_MAX_ENTRIES 10000
-
 #define KS_TASK_COLLECTION_MARGIN 25
 
 //! @endcond
@@ -133,15 +131,13 @@ static void pluginDraw(plugin_sched_context *plugin_ctx,
 			/*
 			 * Starting from the last element in this bin, go backward
 			 * in time until you find a trace entry that satisfies the
-			 * condition defined by plugin_wakeup_match_rec_pid. Note
-			 * that the wakeup event does not belong to this task,
-			 * hence we cannot use the task's collection.
+			 * condition defined by plugin_wakeup_match_rec_pid.
 			 */
 			entryOpen =
 				ksmodel_get_entry_back(histo, bin, false,
 						       plugin_wakeup_match_rec_pid,
 						       pid,
-						       nullptr, // No collection.
+						       col,
 						       &indexOpen);
 
 			if (entryOpen) {
@@ -207,6 +203,9 @@ static void secondPass(kshark_entry **data,
 		       kshark_entry_collection *col,
 		       int pid)
 {
+	if (!col)
+		return;
+
 	const kshark_entry *e;
 	kshark_entry *last;
 	int first, n;
@@ -275,8 +274,8 @@ void plugin_draw(kshark_cpp_argv *argv_c, int pid, int draw_action)
 	 * Try to find a collections for this task. It is OK if
 	 * coll = NULL.
 	 */
-	col = kshark_find_data_collection(kshark_ctx->collections,
-					  kshark_match_pid, pid);
+	col = kshark_find_data_collection(plugin_ctx->collections,
+					  plugin_match_pid, pid);
 	if (!col) {
 		/*
 		 * If a data collection for this task does not exist,
@@ -284,10 +283,12 @@ void plugin_draw(kshark_cpp_argv *argv_c, int pid, int draw_action)
 		 */
 		kshark_entry **data = argvCpp->_histo->data;
 		int size = argvCpp->_histo->data_size;
-		col = kshark_register_data_collection(kshark_ctx,
-						      data, size,
-						      kshark_match_pid, pid,
-						      KS_TASK_COLLECTION_MARGIN);
+
+		col = kshark_add_collection_to_list(kshark_ctx,
+						    &plugin_ctx->collections,
+						    data, size,
+						    plugin_match_pid, pid,
+						    KS_TASK_COLLECTION_MARGIN);
 	}
 
 	if (!tracecmd_filter_id_find(plugin_ctx->second_pass_hash, pid)) {
@@ -296,14 +297,6 @@ void plugin_draw(kshark_cpp_argv *argv_c, int pid, int draw_action)
 		tracecmd_filter_id_add(plugin_ctx->second_pass_hash, pid);
 	}
 
-	/*
-	 * Plotting the latencies makes sense only in the case of a deep zoom.
-	 * Here we set a threshold based on the total number of entries being
-	 * visualized by the model.
-	 * Don't be afraid to play with different values for this threshold.
-	 */
-	if (argvCpp->_histo->tot_count > PLUGIN_MAX_ENTRIES)
-		return;
 	try {
 		pluginDraw(plugin_ctx, kshark_ctx,
 			   argvCpp->_histo, col,
diff --git a/kernel-shark-qt/src/plugins/sched_events.c b/kernel-shark-qt/src/plugins/sched_events.c
index 59ffcfe..1500110 100644
--- a/kernel-shark-qt/src/plugins/sched_events.c
+++ b/kernel-shark-qt/src/plugins/sched_events.c
@@ -42,6 +42,7 @@ static bool plugin_sched_init_context(struct kshark_context *kshark_ctx)
 	plugin_ctx = plugin_sched_context_handler;
 	plugin_ctx->handle = kshark_ctx->handle;
 	plugin_ctx->pevent = kshark_ctx->pevent;
+	plugin_ctx->collections = NULL;
 
 	event = tep_find_event_by_name(plugin_ctx->pevent,
 				       "sched", "sched_switch");
@@ -279,6 +280,25 @@ bool plugin_switch_match_entry_pid(struct kshark_context *kshark_ctx,
 	return false;
 }
 
+/**
+ * @brief A match function to be used to process a data collections for
+ *	  the Sched events plugin.
+ *
+ * @param kshark_ctx: Input location for the session context pointer.
+ * @param e: kshark_entry to be checked.
+ * @param pid: Matching condition value.
+ *
+ * @returns True if the entry is relevant for the Sched events plugin.
+ *	    Otherwise false.
+ */
+bool plugin_match_pid(struct kshark_context *kshark_ctx,
+		      struct kshark_entry *e, int pid)
+{
+	return plugin_switch_match_entry_pid(kshark_ctx, e, pid) ||
+	       plugin_switch_match_rec_pid(kshark_ctx, e, pid) ||
+	       plugin_wakeup_match_rec_pid(kshark_ctx, e, pid);
+}
+
 static void plugin_sched_action(struct kshark_context *kshark_ctx,
 				struct tep_record *rec,
 				struct kshark_entry *entry)
@@ -326,6 +346,7 @@ static int plugin_sched_close(struct kshark_context *kshark_ctx)
 
 	tracecmd_filter_id_hash_free(plugin_ctx->second_pass_hash);
 
+	kshark_free_collection_list(plugin_ctx->collections);
 	free(plugin_ctx);
 	plugin_sched_context_handler = NULL;
 
diff --git a/kernel-shark-qt/src/plugins/sched_events.h b/kernel-shark-qt/src/plugins/sched_events.h
index 28625e3..5318ef3 100644
--- a/kernel-shark-qt/src/plugins/sched_events.h
+++ b/kernel-shark-qt/src/plugins/sched_events.h
@@ -59,6 +59,9 @@ struct plugin_sched_context {
 	 */
 	struct tep_format_field	*sched_wakeup_new_success_field;
 
+	/** List of Data collections used by this plugin. */
+	struct kshark_entry_collection	*collections;
+
 	/** Hash of the tasks for which the second pass is already done. */
 	struct tracecmd_filter_id	*second_pass_hash;
 };
@@ -75,6 +78,9 @@ bool plugin_switch_match_entry_pid(struct kshark_context *kshark_ctx,
 				   struct kshark_entry *e,
 				   int pid);
 
+bool plugin_match_pid(struct kshark_context *kshark_ctx,
+		      struct kshark_entry *e, int pid);
+
 void plugin_draw(struct kshark_cpp_argv *argv, int pid, int draw_action);
 
 #ifdef __cplusplus
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] Make Sched event plugin use its own data collections
  2018-11-30 15:42 [PATCH 0/2] Make Sched event plugin use its own data collections Yordan Karadzhov
  2018-11-30 15:42 ` [PATCH 1/2] kernel-shark-qt: Add a method for adding a new collection to a list Yordan Karadzhov
  2018-11-30 15:42 ` [PATCH 2/2] kernel-shark-qt: Make Sched event plugin use its own data collections Yordan Karadzhov
@ 2018-11-30 16:12 ` Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2018-11-30 16:12 UTC (permalink / raw)
  To: Yordan Karadzhov; +Cc: linux-trace-devel

On Fri, 30 Nov 2018 15:42:35 +0000
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> These two patches aim to restore the log(n) time complexity of
> the latency plotting, by adding Data Collections for Sched events.
> 
> Steven, please try playing with this modification before applying it.
> If you can test this, when doing some real work this would be great.

This works awesomely!

Check out this screenshot:

  http://rostedt.org/private/ks-latency.png

The old way I could not see that huge wake up latency. Also, I noticed
that fsync got preempted (see the red square to the left).

I have to ask, how hard is it to click on the wakeup (the start of
the green box) and get the marker to go there? I can find it difficult
to measure because I can't get the marker to set to the start without
doing a search for the wakeup.

-- Steve


> 
> Yordan Karadzhov (2):
>   kernel-shark-qt: Add a method for adding a new collection to a list
>   kernel-shark-qt: Make Sched event plugin use its own data collections
> 
>  kernel-shark-qt/src/libkshark-collection.c  | 45 ++++++++++++++++++++-
>  kernel-shark-qt/src/libkshark.c             |  1 +
>  kernel-shark-qt/src/libkshark.h             |  9 +++++
>  kernel-shark-qt/src/plugins/SchedEvents.cpp | 33 ++++++---------
>  kernel-shark-qt/src/plugins/sched_events.c  | 21 ++++++++++
>  kernel-shark-qt/src/plugins/sched_events.h  |  6 +++
>  6 files changed, 93 insertions(+), 22 deletions(-)
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-12-01  3:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-30 15:42 [PATCH 0/2] Make Sched event plugin use its own data collections Yordan Karadzhov
2018-11-30 15:42 ` [PATCH 1/2] kernel-shark-qt: Add a method for adding a new collection to a list Yordan Karadzhov
2018-11-30 15:42 ` [PATCH 2/2] kernel-shark-qt: Make Sched event plugin use its own data collections Yordan Karadzhov
2018-11-30 16:12 ` [PATCH 0/2] " Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).