All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] Add support for Firefox's gecko profile format
@ 2023-07-05 19:40 Anup Sharma
  2023-07-05 19:42 ` [PATCH v2 1/7] scripts: python: Extact necessary information from process event Anup Sharma
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Anup Sharma @ 2023-07-05 19:40 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Anup Sharma, linux-perf-users,
	linux-kernel

This patch series adds support for Firefox's gecko profile format.
The format is documented here [1].

I have addressed few comments from the previous version of the patch
like using perf script python interface to process the samples. Also
fixed trailing whitespace and other minor issues.

[1] https://github.com/firefox-devtools/profiler/blob/main/docs-developer/gecko-profile-format.md

Anup Sharma (7):
  scripts: python: Extact necessary information from process event
  scripts: python: Introduce thread sample processing to create thread
  scripts: python: create threads with schemas
  scripts: python: implement get or create stack function
  scripts: python: implement get or create frame function
  scripts: python: implement add sample function and return finish
  scripts: python: Add trace end processing and JSON output

 .../scripts/python/firefox-gecko-converter.py | 204 ++++++++++++++++++
 1 file changed, 204 insertions(+)
 create mode 100644 tools/perf/scripts/python/firefox-gecko-converter.py

-- 
2.34.1


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

* [PATCH v2 1/7] scripts: python: Extact necessary information from process event
  2023-07-05 19:40 [PATCH v2 0/7] Add support for Firefox's gecko profile format Anup Sharma
@ 2023-07-05 19:42 ` Anup Sharma
  2023-07-06  5:35   ` Namhyung Kim
  2023-07-05 19:44 ` [PATCH v2 2/7] scripts: python: Introduce thread sample processing to create thread Anup Sharma
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Anup Sharma @ 2023-07-05 19:42 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Anup Sharma, linux-perf-users,
	linux-kernel

The script takes in a sample event dictionary(param_dict) and retrieves
relevant data such as time stamp, PID, TID, thread name, and call stack
information. If available, the call stack is parsed to include function
names and the associated DSO, which are requires for creating json schema.
Also few libaries has been included which will be used in later commit.

Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
---
 .../scripts/python/firefox-gecko-converter.py | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 tools/perf/scripts/python/firefox-gecko-converter.py

diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
new file mode 100644
index 000000000000..ce663840d212
--- /dev/null
+++ b/tools/perf/scripts/python/firefox-gecko-converter.py
@@ -0,0 +1,37 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+#
+# Usage:
+#
+#     perf record -a -g -F 99 sleep 1
+#     perf script firefox-gecko-converter.py
+
+from __future__ import print_function
+import os
+import sys
+import json
+from functools import reduce
+
+sys.path.append(os.environ['PERF_EXEC_PATH'] + \
+	'/scripts/python/Perf-Trace-Util/lib/Perf/Trace')
+
+from perf_trace_context import *
+from Core import *
+
+def process_event(param_dict):
+	time_stamp = (param_dict['sample']['time'] // 1000) / 1000
+	pid = param_dict['sample']['pid']
+	tid = param_dict['sample']['tid']
+	thread_name = param_dict['comm']
+	start_time = time_stamp if not start_time else start_time
+	if param_dict['callchain']:
+		stack = []
+		for call in param_dict['callchain']:
+			if 'sym' not in call:
+				continue
+			stack.append(call['sym']['name'] + f' (in {call["dso"]})')
+		if len(stack) != 0:
+			stack = stack[::-1]
+	else:
+		mod = param_dict['symbol'] if 'symbol' in param_dict else '[unknown]'
+		dso = param_dict['dso'] if 'dso' in param_dict else '[unknown]'
-- 
2.34.1


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

* [PATCH v2 2/7] scripts: python: Introduce thread sample processing to create thread
  2023-07-05 19:40 [PATCH v2 0/7] Add support for Firefox's gecko profile format Anup Sharma
  2023-07-05 19:42 ` [PATCH v2 1/7] scripts: python: Extact necessary information from process event Anup Sharma
@ 2023-07-05 19:44 ` Anup Sharma
  2023-07-06  5:42   ` Namhyung Kim
  2023-07-05 19:47 ` [PATCH v2 3/7] scripts: python: create threads with schemas Anup Sharma
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Anup Sharma @ 2023-07-05 19:44 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Anup Sharma, linux-perf-users,
	linux-kernel

The _addThreadSample function is responsible for adding a sample to a specific
thread. It first checks if the thread exists in the thread_map dictionary.
If not, it creates a new thread using the _createtread function and assigns
it to the thread_map. Finally, it calls the 'addSample' method of the thread,
passing the thread name, stack, and timestamp.

Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
---
 .../perf/scripts/python/firefox-gecko-converter.py  | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
index ce663840d212..95b061a97cbc 100644
--- a/tools/perf/scripts/python/firefox-gecko-converter.py
+++ b/tools/perf/scripts/python/firefox-gecko-converter.py
@@ -18,7 +18,20 @@ sys.path.append(os.environ['PERF_EXEC_PATH'] + \
 from perf_trace_context import *
 from Core import *
 
+thread_map = {}
+start_time = None
+
 def process_event(param_dict):
+	global start_time
+	global thread_map
+
+	def _addThreadSample(pid, tid, threadName, time_stamp, stack):
+		thread = thread_map.get(tid)
+		if not thread:
+			thread = _createtread(threadName, pid, tid)
+			thread_map[tid] = thread
+		thread['addSample'](threadName, stack, time_stamp)
+
 	time_stamp = (param_dict['sample']['time'] // 1000) / 1000
 	pid = param_dict['sample']['pid']
 	tid = param_dict['sample']['tid']
-- 
2.34.1


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

* [PATCH v2 3/7] scripts: python: create threads with schemas
  2023-07-05 19:40 [PATCH v2 0/7] Add support for Firefox's gecko profile format Anup Sharma
  2023-07-05 19:42 ` [PATCH v2 1/7] scripts: python: Extact necessary information from process event Anup Sharma
  2023-07-05 19:44 ` [PATCH v2 2/7] scripts: python: Introduce thread sample processing to create thread Anup Sharma
@ 2023-07-05 19:47 ` Anup Sharma
  2023-07-06  5:46   ` Namhyung Kim
  2023-07-05 19:48 ` [PATCH v2 4/7] scripts: python: implement get or create stack function Anup Sharma
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Anup Sharma @ 2023-07-05 19:47 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Anup Sharma, linux-perf-users,
	linux-kernel

The markers structure defines the schema and data for
thread markers, including fields such as 'name',
'startTime', 'endTime', 'phase', 'category', and 'data'.

The samples structure defines the schema and data for thread
samples, including fields such as 'stack', 'time', and
'responsiveness'.

The frameTable structure defines the schema and data for frame
information, including fields such as 'location', 'relevantForJS',
'innerWindowID', 'implementation', 'optimizations', 'line',
'column', 'category', and 'subcategory'.

The purpose of this function is to create a new thread structure
These structures provide a framework for storing and organizing
information related to thread markers, samples, frame details,
and stack information.

Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
---
 .../scripts/python/firefox-gecko-converter.py | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
index 95b061a97cbc..e56864e78dc1 100644
--- a/tools/perf/scripts/python/firefox-gecko-converter.py
+++ b/tools/perf/scripts/python/firefox-gecko-converter.py
@@ -24,6 +24,47 @@ start_time = None
 def process_event(param_dict):
 	global start_time
 	global thread_map
+	def _createtread(name, pid, tid):
+		markers = {
+			'schema': {
+				'name': 0,
+				'startTime': 1,
+				'endTime': 2,
+				'phase': 3,
+				'category': 4,
+				'data': 5,
+			},
+			'data': [],
+		}
+		samples = {
+			'schema': {
+				'stack': 0,
+				'time': 1,
+				'responsiveness': 2,
+				},
+			'data': [],
+		}
+		frameTable = {
+			'schema': {
+				'location': 0,
+				'relevantForJS': 1,
+				'innerWindowID': 2,
+				'implementation': 3,
+				'optimizations': 4,
+				'line': 5,
+				'column': 6,
+				'category': 7,
+				'subcategory': 8,
+			},
+			'data': [],
+		}
+		stackTable = {
+			'schema': {
+				'prefix': 0,
+				'frame': 1,
+			},
+			'data': [],
+		}
 
 	def _addThreadSample(pid, tid, threadName, time_stamp, stack):
 		thread = thread_map.get(tid)
-- 
2.34.1


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

* [PATCH v2 4/7] scripts: python: implement get or create stack function
  2023-07-05 19:40 [PATCH v2 0/7] Add support for Firefox's gecko profile format Anup Sharma
                   ` (2 preceding siblings ...)
  2023-07-05 19:47 ` [PATCH v2 3/7] scripts: python: create threads with schemas Anup Sharma
@ 2023-07-05 19:48 ` Anup Sharma
  2023-07-06  5:55   ` Namhyung Kim
  2023-07-05 19:48 ` [PATCH v2 5/7] scripts: python: implement get or create frame function Anup Sharma
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Anup Sharma @ 2023-07-05 19:48 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Anup Sharma, linux-perf-users,
	linux-kernel

The get_or_create_stack function is responsible for retrieving
or creating a stack based on the provided frame and prefix.
It first generates a key using the frame and prefix values.
If the stack corresponding to the key is found in the stackMap,
it is returned. Otherwise, a new stack is created by appending
the prefix and frame to the stackTable's 'data' array. The key
and the index of the newly created stack are added to the
stackMap for future reference.

Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
---
 tools/perf/scripts/python/firefox-gecko-converter.py | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
index e56864e78dc1..6f69c083d3ff 100644
--- a/tools/perf/scripts/python/firefox-gecko-converter.py
+++ b/tools/perf/scripts/python/firefox-gecko-converter.py
@@ -65,6 +65,17 @@ def process_event(param_dict):
 			},
 			'data': [],
 		}
+		stringTable = []
+
+		stackMap = dict()
+		def get_or_create_stack(frame, prefix):
+			key = f"{frame}" if prefix is None else f"{frame},{prefix}"
+			stack = stackMap.get(key)
+			if stack is None:
+				stack = len(stackTable['data'])
+				stackTable['data'].append([prefix, frame])
+				stackMap[key] = stack
+			return stack
 
 	def _addThreadSample(pid, tid, threadName, time_stamp, stack):
 		thread = thread_map.get(tid)
-- 
2.34.1


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

* [PATCH v2 5/7] scripts: python: implement get or create frame function
  2023-07-05 19:40 [PATCH v2 0/7] Add support for Firefox's gecko profile format Anup Sharma
                   ` (3 preceding siblings ...)
  2023-07-05 19:48 ` [PATCH v2 4/7] scripts: python: implement get or create stack function Anup Sharma
@ 2023-07-05 19:48 ` Anup Sharma
  2023-07-06  6:06   ` Namhyung Kim
  2023-07-05 19:49 ` [PATCH v2 6/7] scripts: python: implement add sample function and return finish Anup Sharma
  2023-07-05 19:49 ` [PATCH v2 7/7] scripts: python: Add trace end processing and JSON output Anup Sharma
  6 siblings, 1 reply; 18+ messages in thread
From: Anup Sharma @ 2023-07-05 19:48 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Anup Sharma, linux-perf-users,
	linux-kernel

The get_or_create_frame function is responsible for retrieving or
creating a frame based on the provided frameString. If the frame
corresponding to the frameString is found in the frameMap, it is
returned. Otherwise, a new frame is created by appending relevant
information to the frameTable's 'data' array and adding the
frameString to the stringTable.

The index of the newly created frame is added to the frameMap.

Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
---
 .../scripts/python/firefox-gecko-converter.py | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
index 6f69c083d3ff..d5b9fb16e520 100644
--- a/tools/perf/scripts/python/firefox-gecko-converter.py
+++ b/tools/perf/scripts/python/firefox-gecko-converter.py
@@ -77,6 +77,39 @@ def process_event(param_dict):
 				stackMap[key] = stack
 			return stack
 
+		frameMap = dict()
+		def get_or_create_frame(frameString):
+			frame = frameMap.get(frameString)
+			if frame is None:
+				frame = len(frameTable['data'])
+				location = len(stringTable)
+				stringTable.append(frameString)
+				category = KERNEL_CATEGORY_INDEX if frameString.find('kallsyms') != -1 \
+						or frameString.find('/vmlinux') != -1 \
+						or frameString.endswith('.ko)') \
+						else USER_CATEGORY_INDEX
+				implementation = None
+				optimizations = None
+				line = None
+				relevantForJS = False
+				subcategory = None
+				innerWindowID = 0
+				column = None
+
+				frameTable['data'].append([
+					location,
+					relevantForJS,
+					innerWindowID,
+					implementation,
+					optimizations,
+					line,
+					column,
+					category,
+					subcategory,
+				])
+				frameMap[frameString] = frame
+			return frame
+
 	def _addThreadSample(pid, tid, threadName, time_stamp, stack):
 		thread = thread_map.get(tid)
 		if not thread:
-- 
2.34.1


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

* [PATCH v2 6/7] scripts: python: implement add sample function and return finish
  2023-07-05 19:40 [PATCH v2 0/7] Add support for Firefox's gecko profile format Anup Sharma
                   ` (4 preceding siblings ...)
  2023-07-05 19:48 ` [PATCH v2 5/7] scripts: python: implement get or create frame function Anup Sharma
@ 2023-07-05 19:49 ` Anup Sharma
  2023-07-05 19:49 ` [PATCH v2 7/7] scripts: python: Add trace end processing and JSON output Anup Sharma
  6 siblings, 0 replies; 18+ messages in thread
From: Anup Sharma @ 2023-07-05 19:49 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Anup Sharma, linux-perf-users,
	linux-kernel

The addSample function appends a new entry to the 'samples' data structure.
It takes the thread name, stack array, and time as input parameters and
if the thread name differs from the current name, it updates the name.
The function utilizes the get_or_create_stack and get_or_create_frame
methods to construct the stack structure. Finally, it adds the stack,
time, and responsiveness values to the 'data' list within 'samples'.

The finish function generates a dictionary containing various profile
information such as 'tid', 'pid', 'name', 'markers', 'samples',
'frameTable', 'stackTable', 'stringTable', 'registerTime',
'unregisterTime', and 'processType'.

Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
---
 .../scripts/python/firefox-gecko-converter.py | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
index d5b9fb16e520..910e598c743f 100644
--- a/tools/perf/scripts/python/firefox-gecko-converter.py
+++ b/tools/perf/scripts/python/firefox-gecko-converter.py
@@ -110,6 +110,35 @@ def process_event(param_dict):
 				frameMap[frameString] = frame
 			return frame
 
+		def addSample(threadName, stackArray, time):
+			nonlocal name
+			if name != threadName:
+				name = threadName
+			stack = reduce(lambda prefix, stackFrame: get_or_create_stack
+					(get_or_create_frame(stackFrame), prefix), stackArray, None)
+			responsiveness = 0
+			samples['data'].append([stack, time, responsiveness])
+
+		def finish():
+			return {
+				"tid": tid,
+				"pid": pid,
+				"name": name,
+				"markers": markers,
+				"samples": samples,
+				"frameTable": frameTable,
+				"stackTable": stackTable,
+				"stringTable": stringTable,
+				"registerTime": 0,
+				"unregisterTime": None,
+				"processType": 'default'
+			}
+
+		return {
+			"addSample": addSample,
+			"finish": finish
+		}
+
 	def _addThreadSample(pid, tid, threadName, time_stamp, stack):
 		thread = thread_map.get(tid)
 		if not thread:
-- 
2.34.1


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

* [PATCH v2 7/7] scripts: python: Add trace end processing and JSON output
  2023-07-05 19:40 [PATCH v2 0/7] Add support for Firefox's gecko profile format Anup Sharma
                   ` (5 preceding siblings ...)
  2023-07-05 19:49 ` [PATCH v2 6/7] scripts: python: implement add sample function and return finish Anup Sharma
@ 2023-07-05 19:49 ` Anup Sharma
  6 siblings, 0 replies; 18+ messages in thread
From: Anup Sharma @ 2023-07-05 19:49 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, Anup Sharma, linux-perf-users,
	linux-kernel

Inside the trace end function the final output will be dumped
to standard output in JSON gecko format. Additionally, constants
such as USER_CATEGORY_INDEX, KERNEL_CATEGORY_INDEX, CATEGORIES, and
PRODUCT are defined to provide contextual information.
Also added _addThreadSample call which was missing.

Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
---
 .../scripts/python/firefox-gecko-converter.py | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
index 910e598c743f..6a2a4d816799 100644
--- a/tools/perf/scripts/python/firefox-gecko-converter.py
+++ b/tools/perf/scripts/python/firefox-gecko-converter.py
@@ -18,9 +18,47 @@ sys.path.append(os.environ['PERF_EXEC_PATH'] + \
 from perf_trace_context import *
 from Core import *
 
+USER_CATEGORY_INDEX = 0
+KERNEL_CATEGORY_INDEX = 1
 thread_map = {}
 start_time = None
 
+CATEGORIES = [
+    {'name': 'User', 'color': 'yellow', 'subcategories': ['Other']},
+    {'name': 'Kernel', 'color': 'orange', 'subcategories': ['Other']}
+]
+
+PRODUCT = os.popen('uname -op').read().strip()
+
+def trace_end():
+    thread_array = list(map(lambda thread: thread['finish'](), thread_map.values()))
+    for thread in thread_array:
+        key = thread['samples']['schema']['time']
+        thread['samples']['data'].sort(key=lambda data : float(data[key]))
+
+    result = {
+        'meta': {
+            'interval': 1,
+            'processType': 0,
+            'product': PRODUCT,
+            'stackwalk': 1,
+            'debug': 0,
+            'gcpoison': 0,
+            'asyncstack': 1,
+            'startTime': start_time,
+            'shutdownTime': None,
+            'version': 24,
+            'presymbolicated': True,
+            'categories': CATEGORIES,
+            'markerSchema': []
+            },
+        'libs': [],
+        'threads': thread_array,
+        'processes': [],
+        'pausedRanges': []
+    }
+    json.dump(result, sys.stdout, indent=2)
+
 def process_event(param_dict):
 	global start_time
 	global thread_map
@@ -159,6 +197,8 @@ def process_event(param_dict):
 			stack.append(call['sym']['name'] + f' (in {call["dso"]})')
 		if len(stack) != 0:
 			stack = stack[::-1]
+			_addThreadSample(pid, tid, thread_name, time_stamp, stack)
 	else:
 		mod = param_dict['symbol'] if 'symbol' in param_dict else '[unknown]'
 		dso = param_dict['dso'] if 'dso' in param_dict else '[unknown]'
+		_addThreadSample(pid, tid, thread_name, time_stamp, [mod + f' (in {dso})'])
-- 
2.34.1


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

* Re: [PATCH v2 1/7] scripts: python: Extact necessary information from process event
  2023-07-05 19:42 ` [PATCH v2 1/7] scripts: python: Extact necessary information from process event Anup Sharma
@ 2023-07-06  5:35   ` Namhyung Kim
  2023-07-07 21:12     ` Anup Sharma
  0 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2023-07-06  5:35 UTC (permalink / raw)
  To: Anup Sharma
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, linux-perf-users, linux-kernel

Hi Anup,

On Wed, Jul 5, 2023 at 12:42 PM Anup Sharma <anupnewsmail@gmail.com> wrote:
>
> The script takes in a sample event dictionary(param_dict) and retrieves
> relevant data such as time stamp, PID, TID, thread name, and call stack
> information. If available, the call stack is parsed to include function
> names and the associated DSO, which are requires for creating json schema.
> Also few libaries has been included which will be used in later commit.
>
> Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
> ---
>  .../scripts/python/firefox-gecko-converter.py | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 tools/perf/scripts/python/firefox-gecko-converter.py
>
> diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
> new file mode 100644
> index 000000000000..ce663840d212
> --- /dev/null
> +++ b/tools/perf/scripts/python/firefox-gecko-converter.py
> @@ -0,0 +1,37 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Usage:
> +#
> +#     perf record -a -g -F 99 sleep 1
> +#     perf script firefox-gecko-converter.py
> +
> +from __future__ import print_function

Is this needed for python3?

> +import os
> +import sys
> +import json
> +from functools import reduce
> +
> +sys.path.append(os.environ['PERF_EXEC_PATH'] + \
> +       '/scripts/python/Perf-Trace-Util/lib/Perf/Trace')
> +
> +from perf_trace_context import *
> +from Core import *
> +
> +def process_event(param_dict):
> +       time_stamp = (param_dict['sample']['time'] // 1000) / 1000
> +       pid = param_dict['sample']['pid']
> +       tid = param_dict['sample']['tid']
> +       thread_name = param_dict['comm']
> +       start_time = time_stamp if not start_time else start_time
> +       if param_dict['callchain']:
> +               stack = []
> +               for call in param_dict['callchain']:
> +                       if 'sym' not in call:
> +                               continue
> +                       stack.append(call['sym']['name'] + f' (in {call["dso"]})')
> +               if len(stack) != 0:
> +                       stack = stack[::-1]
> +       else:
> +               mod = param_dict['symbol'] if 'symbol' in param_dict else '[unknown]'

Why is it 'mod' instead of 'sym' or 'func'?

Thanks,
Namhyung


> +               dso = param_dict['dso'] if 'dso' in param_dict else '[unknown]'
> --
> 2.34.1
>

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

* Re: [PATCH v2 2/7] scripts: python: Introduce thread sample processing to create thread
  2023-07-05 19:44 ` [PATCH v2 2/7] scripts: python: Introduce thread sample processing to create thread Anup Sharma
@ 2023-07-06  5:42   ` Namhyung Kim
  2023-07-10 21:51     ` Anup Sharma
  0 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2023-07-06  5:42 UTC (permalink / raw)
  To: Anup Sharma
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, linux-perf-users, linux-kernel

On Wed, Jul 5, 2023 at 12:44 PM Anup Sharma <anupnewsmail@gmail.com> wrote:
>
> The _addThreadSample function is responsible for adding a sample to a specific
> thread. It first checks if the thread exists in the thread_map dictionary.
> If not, it creates a new thread using the _createtread function and assigns
> it to the thread_map. Finally, it calls the 'addSample' method of the thread,
> passing the thread name, stack, and timestamp.
>
> Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
> ---
>  .../perf/scripts/python/firefox-gecko-converter.py  | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
> index ce663840d212..95b061a97cbc 100644
> --- a/tools/perf/scripts/python/firefox-gecko-converter.py
> +++ b/tools/perf/scripts/python/firefox-gecko-converter.py
> @@ -18,7 +18,20 @@ sys.path.append(os.environ['PERF_EXEC_PATH'] + \
>  from perf_trace_context import *
>  from Core import *
>
> +thread_map = {}
> +start_time = None
> +
>  def process_event(param_dict):
> +       global start_time
> +       global thread_map
> +
> +       def _addThreadSample(pid, tid, threadName, time_stamp, stack):
> +               thread = thread_map.get(tid)
> +               if not thread:
> +                       thread = _createtread(threadName, pid, tid)

Shouldn't it be '_createThread'?

> +                       thread_map[tid] = thread
> +               thread['addSample'](threadName, stack, time_stamp)

Why is it like this?  What do you intend with the thread['addSample']
method?  Can it be simpler like a direct function call?

And more importantly, you'd better make each patch work properly.
AFAICS it won't do the job because both _createtread() and
thread['addSample'] are not implemented yet.

You can either move those definitions to this commit or have the
commit implementing them before this one.

Thanks,
Namhyung


> +
>         time_stamp = (param_dict['sample']['time'] // 1000) / 1000
>         pid = param_dict['sample']['pid']
>         tid = param_dict['sample']['tid']
> --
> 2.34.1
>

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

* Re: [PATCH v2 3/7] scripts: python: create threads with schemas
  2023-07-05 19:47 ` [PATCH v2 3/7] scripts: python: create threads with schemas Anup Sharma
@ 2023-07-06  5:46   ` Namhyung Kim
  2023-07-10 21:54     ` Anup Sharma
  0 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2023-07-06  5:46 UTC (permalink / raw)
  To: Anup Sharma
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, linux-perf-users, linux-kernel

On Wed, Jul 5, 2023 at 12:47 PM Anup Sharma <anupnewsmail@gmail.com> wrote:
>
> The markers structure defines the schema and data for
> thread markers, including fields such as 'name',
> 'startTime', 'endTime', 'phase', 'category', and 'data'.
>
> The samples structure defines the schema and data for thread
> samples, including fields such as 'stack', 'time', and
> 'responsiveness'.
>
> The frameTable structure defines the schema and data for frame
> information, including fields such as 'location', 'relevantForJS',
> 'innerWindowID', 'implementation', 'optimizations', 'line',
> 'column', 'category', and 'subcategory'.
>
> The purpose of this function is to create a new thread structure
> These structures provide a framework for storing and organizing
> information related to thread markers, samples, frame details,
> and stack information.
>
> Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
> ---
>  .../scripts/python/firefox-gecko-converter.py | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
> index 95b061a97cbc..e56864e78dc1 100644
> --- a/tools/perf/scripts/python/firefox-gecko-converter.py
> +++ b/tools/perf/scripts/python/firefox-gecko-converter.py
> @@ -24,6 +24,47 @@ start_time = None
>  def process_event(param_dict):
>         global start_time
>         global thread_map
> +       def _createtread(name, pid, tid):
> +               markers = {
> +                       'schema': {
> +                               'name': 0,
> +                               'startTime': 1,
> +                               'endTime': 2,
> +                               'phase': 3,
> +                               'category': 4,
> +                               'data': 5,
> +                       },
> +                       'data': [],
> +               }
> +               samples = {
> +                       'schema': {
> +                               'stack': 0,
> +                               'time': 1,
> +                               'responsiveness': 2,
> +                               },
> +                       'data': [],
> +               }
> +               frameTable = {
> +                       'schema': {
> +                               'location': 0,
> +                               'relevantForJS': 1,
> +                               'innerWindowID': 2,
> +                               'implementation': 3,
> +                               'optimizations': 4,
> +                               'line': 5,
> +                               'column': 6,
> +                               'category': 7,
> +                               'subcategory': 8,
> +                       },
> +                       'data': [],
> +               }
> +               stackTable = {
> +                       'schema': {
> +                               'prefix': 0,
> +                               'frame': 1,
> +                       },
> +                       'data': [],
> +               }

It seems this function doesn't return anything.
Can we have a complete definition?  Otherwise it's hard to
know how these tables are used.

Thanks,
Namhyung


>
>         def _addThreadSample(pid, tid, threadName, time_stamp, stack):
>                 thread = thread_map.get(tid)
> --
> 2.34.1
>

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

* Re: [PATCH v2 4/7] scripts: python: implement get or create stack function
  2023-07-05 19:48 ` [PATCH v2 4/7] scripts: python: implement get or create stack function Anup Sharma
@ 2023-07-06  5:55   ` Namhyung Kim
  2023-07-10 22:01     ` Anup Sharma
  0 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2023-07-06  5:55 UTC (permalink / raw)
  To: Anup Sharma
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, linux-perf-users, linux-kernel

On Wed, Jul 5, 2023 at 12:48 PM Anup Sharma <anupnewsmail@gmail.com> wrote:
>
> The get_or_create_stack function is responsible for retrieving
> or creating a stack based on the provided frame and prefix.
> It first generates a key using the frame and prefix values.
> If the stack corresponding to the key is found in the stackMap,
> it is returned. Otherwise, a new stack is created by appending
> the prefix and frame to the stackTable's 'data' array. The key
> and the index of the newly created stack are added to the
> stackMap for future reference.
>
> Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
> ---
>  tools/perf/scripts/python/firefox-gecko-converter.py | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
> index e56864e78dc1..6f69c083d3ff 100644
> --- a/tools/perf/scripts/python/firefox-gecko-converter.py
> +++ b/tools/perf/scripts/python/firefox-gecko-converter.py
> @@ -65,6 +65,17 @@ def process_event(param_dict):
>                         },
>                         'data': [],
>                 }
> +               stringTable = []
> +
> +               stackMap = dict()
> +               def get_or_create_stack(frame, prefix):
> +                       key = f"{frame}" if prefix is None else f"{frame},{prefix}"
> +                       stack = stackMap.get(key)

I'm confused about what the 'stack' is.

> +                       if stack is None:
> +                               stack = len(stackTable['data'])

It seems like a kind of index for the stackTable's data.

> +                               stackTable['data'].append([prefix, frame])
> +                               stackMap[key] = stack
> +                       return stack

So this function returns an index, right?

Thanks,
Namhyung


>
>         def _addThreadSample(pid, tid, threadName, time_stamp, stack):
>                 thread = thread_map.get(tid)
> --
> 2.34.1
>

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

* Re: [PATCH v2 5/7] scripts: python: implement get or create frame function
  2023-07-05 19:48 ` [PATCH v2 5/7] scripts: python: implement get or create frame function Anup Sharma
@ 2023-07-06  6:06   ` Namhyung Kim
  2023-07-10 22:42     ` Anup Sharma
  0 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2023-07-06  6:06 UTC (permalink / raw)
  To: Anup Sharma
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, linux-perf-users, linux-kernel

On Wed, Jul 5, 2023 at 12:48 PM Anup Sharma <anupnewsmail@gmail.com> wrote:
>
> The get_or_create_frame function is responsible for retrieving or
> creating a frame based on the provided frameString. If the frame
> corresponding to the frameString is found in the frameMap, it is
> returned. Otherwise, a new frame is created by appending relevant
> information to the frameTable's 'data' array and adding the
> frameString to the stringTable.
>
> The index of the newly created frame is added to the frameMap.
>
> Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
> ---
>  .../scripts/python/firefox-gecko-converter.py | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
> index 6f69c083d3ff..d5b9fb16e520 100644
> --- a/tools/perf/scripts/python/firefox-gecko-converter.py
> +++ b/tools/perf/scripts/python/firefox-gecko-converter.py
> @@ -77,6 +77,39 @@ def process_event(param_dict):
>                                 stackMap[key] = stack
>                         return stack
>
> +               frameMap = dict()
> +               def get_or_create_frame(frameString):
> +                       frame = frameMap.get(frameString)
> +                       if frame is None:
> +                               frame = len(frameTable['data'])
> +                               location = len(stringTable)
> +                               stringTable.append(frameString)

Looks like it just always appending a new string.
Any deduplication work later?

> +                               category = KERNEL_CATEGORY_INDEX if frameString.find('kallsyms') != -1 \
> +                                               or frameString.find('/vmlinux') != -1 \
> +                                               or frameString.endswith('.ko)') \
> +                                               else USER_CATEGORY_INDEX

I think you can use param_dict['sample']['cpumode'].
Please see include/uapi/linux/perf_event.h for cpumode
values.

> +                               implementation = None
> +                               optimizations = None
> +                               line = None
> +                               relevantForJS = False
> +                               subcategory = None
> +                               innerWindowID = 0
> +                               column = None
> +
> +                               frameTable['data'].append([
> +                                       location,
> +                                       relevantForJS,
> +                                       innerWindowID,
> +                                       implementation,
> +                                       optimizations,
> +                                       line,
> +                                       column,
> +                                       category,
> +                                       subcategory,
> +                               ])
> +                               frameMap[frameString] = frame

I think it'd be better if you define the frameTable in this
commit.

Thanks,
Namhyung


> +                       return frame
> +
>         def _addThreadSample(pid, tid, threadName, time_stamp, stack):
>                 thread = thread_map.get(tid)
>                 if not thread:
> --
> 2.34.1
>

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

* Re: [PATCH v2 1/7] scripts: python: Extact necessary information from process event
  2023-07-06  5:35   ` Namhyung Kim
@ 2023-07-07 21:12     ` Anup Sharma
  0 siblings, 0 replies; 18+ messages in thread
From: Anup Sharma @ 2023-07-07 21:12 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Anup Sharma, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Ian Rogers, Adrian Hunter, linux-perf-users,
	linux-kernel

On Wed, Jul 05, 2023 at 10:35:02PM -0700, Namhyung Kim wrote:
> Hi Anup,
> 
> On Wed, Jul 5, 2023 at 12:42 PM Anup Sharma <anupnewsmail@gmail.com> wrote:
> >
> > The script takes in a sample event dictionary(param_dict) and retrieves
> > relevant data such as time stamp, PID, TID, thread name, and call stack
> > information. If available, the call stack is parsed to include function
> > names and the associated DSO, which are requires for creating json schema.
> > Also few libaries has been included which will be used in later commit.
> >
> > Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
> > ---
> >  .../scripts/python/firefox-gecko-converter.py | 37 +++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >  create mode 100644 tools/perf/scripts/python/firefox-gecko-converter.py
> >
> > diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
> > new file mode 100644
> > index 000000000000..ce663840d212
> > --- /dev/null
> > +++ b/tools/perf/scripts/python/firefox-gecko-converter.py
> > @@ -0,0 +1,37 @@
> > +#!/usr/bin/env python3

I believe this is not needed as we are using perf-script-python interface.

> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Usage:
> > +#
> > +#     perf record -a -g -F 99 sleep 1
> > +#     perf script firefox-gecko-converter.py
> > +
> > +from __future__ import print_function
> 
> Is this needed for python3?

No, it is not needed. I will remove it.

>
> > +import os
> > +import sys
> > +import json
> > +from functools import reduce
> > +
> > +sys.path.append(os.environ['PERF_EXEC_PATH'] + \
> > +       '/scripts/python/Perf-Trace-Util/lib/Perf/Trace')
> > +
> > +from perf_trace_context import *
> > +from Core import *
> > +
> > +def process_event(param_dict):
> > +       time_stamp = (param_dict['sample']['time'] // 1000) / 1000
> > +       pid = param_dict['sample']['pid']
> > +       tid = param_dict['sample']['tid']
> > +       thread_name = param_dict['comm']
> > +       start_time = time_stamp if not start_time else start_time
> > +       if param_dict['callchain']:
> > +               stack = []
> > +               for call in param_dict['callchain']:
> > +                       if 'sym' not in call:
> > +                               continue
> > +                       stack.append(call['sym']['name'] + f' (in {call["dso"]})')
> > +               if len(stack) != 0:
> > +                       stack = stack[::-1]
> > +       else:
> > +               mod = param_dict['symbol'] if 'symbol' in param_dict else '[unknown]'
> 
> Why is it 'mod' instead of 'sym' or 'func'?

I will change it to 'func'. It will be more meaningful.

> 
> Thanks,
> Namhyung
> 
> 
> > +               dso = param_dict['dso'] if 'dso' in param_dict else '[unknown]'
> > --
> > 2.34.1
> >

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

* Re: [PATCH v2 2/7] scripts: python: Introduce thread sample processing to create thread
  2023-07-06  5:42   ` Namhyung Kim
@ 2023-07-10 21:51     ` Anup Sharma
  0 siblings, 0 replies; 18+ messages in thread
From: Anup Sharma @ 2023-07-10 21:51 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Anup Sharma, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Ian Rogers, Adrian Hunter, linux-perf-users,
	linux-kernel

On Wed, Jul 05, 2023 at 10:42:47PM -0700, Namhyung Kim wrote:
> On Wed, Jul 5, 2023 at 12:44 PM Anup Sharma <anupnewsmail@gmail.com> wrote:
> >
> > The _addThreadSample function is responsible for adding a sample to a specific
> > thread. It first checks if the thread exists in the thread_map dictionary.
> > If not, it creates a new thread using the _createtread function and assigns
> > it to the thread_map. Finally, it calls the 'addSample' method of the thread,
> > passing the thread name, stack, and timestamp.
> >
> > Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
> > ---
> >  .../perf/scripts/python/firefox-gecko-converter.py  | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
> > index ce663840d212..95b061a97cbc 100644
> > --- a/tools/perf/scripts/python/firefox-gecko-converter.py
> > +++ b/tools/perf/scripts/python/firefox-gecko-converter.py
> > @@ -18,7 +18,20 @@ sys.path.append(os.environ['PERF_EXEC_PATH'] + \
> >  from perf_trace_context import *
> >  from Core import *
> >
> > +thread_map = {}
> > +start_time = None
> > +
> >  def process_event(param_dict):
> > +       global start_time
> > +       global thread_map
> > +
> > +       def _addThreadSample(pid, tid, threadName, time_stamp, stack):
> > +               thread = thread_map.get(tid)
> > +               if not thread:
> > +                       thread = _createtread(threadName, pid, tid)
> 
> Shouldn't it be '_createThread'?

Yes, it should be '_createThread'. I will fix it in the next version.

> > +                       thread_map[tid] = thread
> > +               thread['addSample'](threadName, stack, time_stamp)
> 
> Why is it like this?  What do you intend with the thread['addSample']
> method?  Can it be simpler like a direct function call?

The purpose of the addSample function is to append stack frames to the
samples['data'] collection. While it could be implemented as a standalone
function, doing so would increase complexity due to shared properties
among threads such as pid, tid, and threadName. Although a decorator
could potentially address this, it would likely result in code that
is functionally and structurally similar. Alternatively, if addSample
were implemented as a separate function, the shared elements would need
to be repeatedly passed to the function.

> And more importantly, you'd better make each patch work properly.
> AFAICS it won't do the job because both _createtread() and
> thread['addSample'] are not implemented yet.
>
> You can either move those definitions to this commit or have the
> commit implementing them before this one.

Thanks, Preparing commit in series is new to me. I will try to fix
it in the next version.

> Thanks,
> Namhyung
> 
> 
> > +
> >         time_stamp = (param_dict['sample']['time'] // 1000) / 1000
> >         pid = param_dict['sample']['pid']
> >         tid = param_dict['sample']['tid']
> > --
> > 2.34.1
> >

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

* Re: [PATCH v2 3/7] scripts: python: create threads with schemas
  2023-07-06  5:46   ` Namhyung Kim
@ 2023-07-10 21:54     ` Anup Sharma
  0 siblings, 0 replies; 18+ messages in thread
From: Anup Sharma @ 2023-07-10 21:54 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Anup Sharma, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Ian Rogers, Adrian Hunter, linux-perf-users,
	linux-kernel

On Wed, Jul 05, 2023 at 10:46:51PM -0700, Namhyung Kim wrote:
> On Wed, Jul 5, 2023 at 12:47 PM Anup Sharma <anupnewsmail@gmail.com> wrote:
> >
> > The markers structure defines the schema and data for
> > thread markers, including fields such as 'name',
> > 'startTime', 'endTime', 'phase', 'category', and 'data'.
> >
> > The samples structure defines the schema and data for thread
> > samples, including fields such as 'stack', 'time', and
> > 'responsiveness'.
> >
> > The frameTable structure defines the schema and data for frame
> > information, including fields such as 'location', 'relevantForJS',
> > 'innerWindowID', 'implementation', 'optimizations', 'line',
> > 'column', 'category', and 'subcategory'.
> >
> > The purpose of this function is to create a new thread structure
> > These structures provide a framework for storing and organizing
> > information related to thread markers, samples, frame details,
> > and stack information.
> >
> > Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
> > ---
> >  .../scripts/python/firefox-gecko-converter.py | 41 +++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >
> > diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
> > index 95b061a97cbc..e56864e78dc1 100644
> > --- a/tools/perf/scripts/python/firefox-gecko-converter.py
> > +++ b/tools/perf/scripts/python/firefox-gecko-converter.py
> > @@ -24,6 +24,47 @@ start_time = None
> >  def process_event(param_dict):
> >         global start_time
> >         global thread_map
> > +       def _createtread(name, pid, tid):
> > +               markers = {
> > +                       'schema': {
> > +                               'name': 0,
> > +                               'startTime': 1,
> > +                               'endTime': 2,
> > +                               'phase': 3,
> > +                               'category': 4,
> > +                               'data': 5,
> > +                       },
> > +                       'data': [],
> > +               }
> > +               samples = {
> > +                       'schema': {
> > +                               'stack': 0,
> > +                               'time': 1,
> > +                               'responsiveness': 2,
> > +                               },
> > +                       'data': [],
> > +               }
> > +               frameTable = {
> > +                       'schema': {
> > +                               'location': 0,
> > +                               'relevantForJS': 1,
> > +                               'innerWindowID': 2,
> > +                               'implementation': 3,
> > +                               'optimizations': 4,
> > +                               'line': 5,
> > +                               'column': 6,
> > +                               'category': 7,
> > +                               'subcategory': 8,
> > +                       },
> > +                       'data': [],
> > +               }
> > +               stackTable = {
> > +                       'schema': {
> > +                               'prefix': 0,
> > +                               'frame': 1,
> > +                       },
> > +                       'data': [],
> > +               }
> 
> It seems this function doesn't return anything.
> Can we have a complete definition?  Otherwise it's hard to
> know how these tables are used.

I will add the complete definition in the next version.

> Thanks,
> Namhyung
> 
> 
> >
> >         def _addThreadSample(pid, tid, threadName, time_stamp, stack):
> >                 thread = thread_map.get(tid)
> > --
> > 2.34.1
> >

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

* Re: [PATCH v2 4/7] scripts: python: implement get or create stack function
  2023-07-06  5:55   ` Namhyung Kim
@ 2023-07-10 22:01     ` Anup Sharma
  0 siblings, 0 replies; 18+ messages in thread
From: Anup Sharma @ 2023-07-10 22:01 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Anup Sharma, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Ian Rogers, Adrian Hunter, linux-perf-users,
	linux-kernel

On Wed, Jul 05, 2023 at 10:55:52PM -0700, Namhyung Kim wrote:
> On Wed, Jul 5, 2023 at 12:48 PM Anup Sharma <anupnewsmail@gmail.com> wrote:
> >
> > The get_or_create_stack function is responsible for retrieving
> > or creating a stack based on the provided frame and prefix.
> > It first generates a key using the frame and prefix values.
> > If the stack corresponding to the key is found in the stackMap,
> > it is returned. Otherwise, a new stack is created by appending
> > the prefix and frame to the stackTable's 'data' array. The key
> > and the index of the newly created stack are added to the
> > stackMap for future reference.
> >
> > Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
> > ---
> >  tools/perf/scripts/python/firefox-gecko-converter.py | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
> > index e56864e78dc1..6f69c083d3ff 100644
> > --- a/tools/perf/scripts/python/firefox-gecko-converter.py
> > +++ b/tools/perf/scripts/python/firefox-gecko-converter.py
> > @@ -65,6 +65,17 @@ def process_event(param_dict):
> >                         },
> >                         'data': [],
> >                 }
> > +               stringTable = []
> > +
> > +               stackMap = dict()
> > +               def get_or_create_stack(frame, prefix):
> > +                       key = f"{frame}" if prefix is None else f"{frame},{prefix}"
> > +                       stack = stackMap.get(key)
> 
> I'm confused about what the 'stack' is.

The stack is a list of frames.

> > +                       if stack is None:
> > +                               stack = len(stackTable['data'])
> 
> It seems like a kind of index for the stackTable's data.

Yes, it is an index.

> > +                               stackTable['data'].append([prefix, frame])
> > +                               stackMap[key] = stack
> > +                       return stack
> 
> So this function returns an index, right?

Correct, it returns an index. This index allows to access or identify
a specific stack within the table.

> Thanks,
> Namhyung
> 
> 
> >
> >         def _addThreadSample(pid, tid, threadName, time_stamp, stack):
> >                 thread = thread_map.get(tid)
> > --
> > 2.34.1
> >

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

* Re: [PATCH v2 5/7] scripts: python: implement get or create frame function
  2023-07-06  6:06   ` Namhyung Kim
@ 2023-07-10 22:42     ` Anup Sharma
  0 siblings, 0 replies; 18+ messages in thread
From: Anup Sharma @ 2023-07-10 22:42 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Anup Sharma, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Ian Rogers, Adrian Hunter, linux-perf-users,
	linux-kernel

On Wed, Jul 05, 2023 at 11:06:58PM -0700, Namhyung Kim wrote:
> On Wed, Jul 5, 2023 at 12:48 PM Anup Sharma <anupnewsmail@gmail.com> wrote:
> >
> > The get_or_create_frame function is responsible for retrieving or
> > creating a frame based on the provided frameString. If the frame
> > corresponding to the frameString is found in the frameMap, it is
> > returned. Otherwise, a new frame is created by appending relevant
> > information to the frameTable's 'data' array and adding the
> > frameString to the stringTable.
> >
> > The index of the newly created frame is added to the frameMap.
> >
> > Signed-off-by: Anup Sharma <anupnewsmail@gmail.com>
> > ---
> >  .../scripts/python/firefox-gecko-converter.py | 33 +++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/tools/perf/scripts/python/firefox-gecko-converter.py b/tools/perf/scripts/python/firefox-gecko-converter.py
> > index 6f69c083d3ff..d5b9fb16e520 100644
> > --- a/tools/perf/scripts/python/firefox-gecko-converter.py
> > +++ b/tools/perf/scripts/python/firefox-gecko-converter.py
> > @@ -77,6 +77,39 @@ def process_event(param_dict):
> >                                 stackMap[key] = stack
> >                         return stack
> >
> > +               frameMap = dict()
> > +               def get_or_create_frame(frameString):
> > +                       frame = frameMap.get(frameString)
> > +                       if frame is None:
> > +                               frame = len(frameTable['data'])
> > +                               location = len(stringTable)
> > +                               stringTable.append(frameString)
> 
> Looks like it just always appending a new string.
> Any deduplication work later?

Although this initially came to my mind and almost all stack frames
seem to be similar for a repeated call, but I am not sure if we can dedup
because some frames do differ as the call stack progresses. For example,
a process exits a function and then calls another function, resulting in
most of the stack frames being the same but the last one being different.

> > +                               category = KERNEL_CATEGORY_INDEX if frameString.find('kallsyms') != -1 \
> > +                                               or frameString.find('/vmlinux') != -1 \
> > +                                               or frameString.endswith('.ko)') \
> > +                                               else USER_CATEGORY_INDEX
> 
> I think you can use param_dict['sample']['cpumode'].
> Please see include/uapi/linux/perf_event.h for cpumode
> values.

I am actively working on incorporating the use of param_dict
['sample']['cpumode'] to determine the category in the
upcoming v4 update. I saw in param_dict['sample']['cpumode']
values exist as 1 and 2. Can you point me to exact line in
include/uapi/linux/perf_event.h . I am not able to find it.

> > +                               implementation = None
> > +                               optimizations = None
> > +                               line = None
> > +                               relevantForJS = False
> > +                               subcategory = None
> > +                               innerWindowID = 0
> > +                               column = None
> > +
> > +                               frameTable['data'].append([
> > +                                       location,
> > +                                       relevantForJS,
> > +                                       innerWindowID,
> > +                                       implementation,
> > +                                       optimizations,
> > +                                       line,
> > +                                       column,
> > +                                       category,
> > +                                       subcategory,
> > +                               ])
> > +                               frameMap[frameString] = frame
> 
> I think it'd be better if you define the frameTable in this
> commit.

Certainly, my apologies for the disorder.

> Thanks,
> Namhyung
> 
> 
> > +                       return frame
> > +
> >         def _addThreadSample(pid, tid, threadName, time_stamp, stack):
> >                 thread = thread_map.get(tid)
> >                 if not thread:
> > --
> > 2.34.1
> >

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

end of thread, other threads:[~2023-07-10 22:43 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-05 19:40 [PATCH v2 0/7] Add support for Firefox's gecko profile format Anup Sharma
2023-07-05 19:42 ` [PATCH v2 1/7] scripts: python: Extact necessary information from process event Anup Sharma
2023-07-06  5:35   ` Namhyung Kim
2023-07-07 21:12     ` Anup Sharma
2023-07-05 19:44 ` [PATCH v2 2/7] scripts: python: Introduce thread sample processing to create thread Anup Sharma
2023-07-06  5:42   ` Namhyung Kim
2023-07-10 21:51     ` Anup Sharma
2023-07-05 19:47 ` [PATCH v2 3/7] scripts: python: create threads with schemas Anup Sharma
2023-07-06  5:46   ` Namhyung Kim
2023-07-10 21:54     ` Anup Sharma
2023-07-05 19:48 ` [PATCH v2 4/7] scripts: python: implement get or create stack function Anup Sharma
2023-07-06  5:55   ` Namhyung Kim
2023-07-10 22:01     ` Anup Sharma
2023-07-05 19:48 ` [PATCH v2 5/7] scripts: python: implement get or create frame function Anup Sharma
2023-07-06  6:06   ` Namhyung Kim
2023-07-10 22:42     ` Anup Sharma
2023-07-05 19:49 ` [PATCH v2 6/7] scripts: python: implement add sample function and return finish Anup Sharma
2023-07-05 19:49 ` [PATCH v2 7/7] scripts: python: Add trace end processing and JSON output Anup Sharma

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.