From 2ba762fb61dc139265e78a02bdd01ca3734e155a Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Sat, 23 Mar 2024 20:01:07 +0100 Subject: [PATCH 01/15] Everything except for computing the cumulative CPU times --- px/px_process.py | 9 +++++++ px/px_sort_order.py | 14 +++++++++++ px/px_terminal.py | 28 +++++++++++++++------ px/px_top.py | 59 ++++++++++++++++++++++++--------------------- 4 files changed, 75 insertions(+), 35 deletions(-) create mode 100644 px/px_sort_order.py diff --git a/px/px_process.py b/px/px_process.py index 06b9213..2ccad7e 100644 --- a/px/px_process.py +++ b/px/px_process.py @@ -92,6 +92,7 @@ def __init__( memory_percent: Optional[float] = None, cpu_percent: Optional[float] = None, cpu_time: Optional[float] = None, + cumulative_cpu_time: Optional[float] = None, ) -> None: self.pid: int = pid self.ppid: Optional[int] = ppid @@ -142,6 +143,7 @@ def __init__( self.cpu_percent_s = f"{cpu_percent:.0f}%" self.set_cpu_time_seconds(cpu_time) + self.set_cumulative_cpu_time_seconds(cumulative_cpu_time) self.children: MutableSet[PxProcess] = set() self.parent: Optional[PxProcess] = None @@ -173,6 +175,13 @@ def set_cpu_time_seconds(self, seconds: Optional[float]) -> None: self.cpu_time_s = seconds_to_str(seconds) self.cpu_time_seconds = seconds + def set_cumulative_cpu_time_seconds(self, seconds: Optional[float]) -> None: + self.cumulative_cpu_time_s: str = "--" + self.cumulative_cpu_time_seconds = None + if seconds is not None: + self.cumulative_cpu_time_s = seconds_to_str(seconds) + self.cumulative_cpu_time_seconds = seconds + def match(self, string, require_exact_user=True): """ Returns True if this process matches the string. diff --git a/px/px_sort_order.py b/px/px_sort_order.py new file mode 100644 index 0000000..583c28d --- /dev/null +++ b/px/px_sort_order.py @@ -0,0 +1,14 @@ +import enum + + +class SortOrder(enum.Enum): + CPU = 1 + MEMORY = 2 + CUMULATIVE_CPU = 3 + + def next(self): + if self == SortOrder.CPU: + return SortOrder.MEMORY + if self == SortOrder.MEMORY: + return SortOrder.CUMULATIVE_CPU + return SortOrder.CPU diff --git a/px/px_terminal.py b/px/px_terminal.py index 91fa0ee..b0d5387 100644 --- a/px/px_terminal.py +++ b/px/px_terminal.py @@ -13,6 +13,7 @@ from typing import Optional from typing import Iterable from . import px_process +from . import px_sort_order # NOTE: To work with this list it can be useful to find the text "Uncomment to @@ -314,7 +315,7 @@ def format_with_widths(widths: List[int], strings: List[str]) -> str: def to_screen_lines( procs: List[px_process.PxProcess], row_to_highlight: Optional[int], - highlight_heading: Optional[str], + sort_order: px_sort_order.SortOrder, with_username: bool = True, ) -> List[str]: """ @@ -325,18 +326,24 @@ def to_screen_lines( The column name must be from the hard coded list in this function, see below. """ + cputime_name = "CPUTIME" + if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + cputime_name = "CUMLCPU" headings = [ "PID", "COMMAND", "USERNAME", "CPU", - "CPUTIME", + cputime_name, "RAM", "COMMANDLINE", ] - highlight_column: Optional[int] = None - if highlight_heading is not None: - highlight_column = headings.index(highlight_heading) + highlight_column = 5 # "RAM" + if ( + sort_order == px_sort_order.SortOrder.CPU + or sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU + ): + highlight_column = 4 # "CPUTIME" or "CUMLCPU" # Compute widest width for pid, command, user, cpu and memory usage columns pid_width = len(headings[0]) @@ -350,7 +357,12 @@ def to_screen_lines( command_width = max(command_width, len(proc.command)) username_width = max(username_width, len(proc.username)) cpu_width = max(cpu_width, len(proc.cpu_percent_s)) - cputime_width = max(cputime_width, len(proc.cpu_time_s)) + + cputime_s = proc.cpu_time_s + if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + cputime_s = proc.cumulative_cpu_time_s + cputime_width = max(cputime_width, len(cputime_s)) + mem_width = max(mem_width, len(proc.memory_percent_s)) column_widths = [ @@ -363,8 +375,8 @@ def to_screen_lines( 0, # The command line can have any length ] + username_index = headings.index("USERNAME") if not with_username: - username_index = headings.index("USERNAME") del headings[username_index] del column_widths[username_index] @@ -412,6 +424,8 @@ def to_screen_lines( memory_percent_s = bold(memory_percent_s.rjust(mem_width)) cpu_time_s = proc.cpu_time_s + if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + cpu_time_s = proc.cumulative_cpu_time_s if not proc.cpu_time_seconds: # Zero or undefined cpu_time_s = faint(cpu_time_s.rjust(cputime_width)) diff --git a/px/px_top.py b/px/px_top.py index 26d63ef..e94945c 100644 --- a/px/px_top.py +++ b/px/px_top.py @@ -1,15 +1,15 @@ -import enum import sys import copy import logging import unicodedata import os +from . import px_poller from . import px_process from . import px_terminal +from . import px_sort_order from . import px_processinfo from . import px_process_menu -from . import px_poller from . import px_category_bar from typing import List @@ -46,17 +46,7 @@ highlight_has_moved: bool = False -class SortOrder(enum.Enum): - CPU = 1 - MEMORY = 2 - - def next(self): - if self == SortOrder.CPU: - return SortOrder.MEMORY - return SortOrder.CPU - - -sort_order = SortOrder.CPU +sort_order = px_sort_order.SortOrder.CPU def adjust_cpu_times( @@ -112,6 +102,13 @@ def get_notnone_cpu_time_seconds(proc: px_process.PxProcess) -> float: return 0 +def get_notnone_cumulative_cpu_time_seconds(proc: px_process.PxProcess) -> float: + seconds = proc.cumulative_cpu_time_seconds + if seconds is not None: + return seconds + return 0 + + def get_notnone_memory_percent(proc: px_process.PxProcess) -> float: percent = proc.memory_percent if percent is not None: @@ -119,18 +116,25 @@ def get_notnone_memory_percent(proc: px_process.PxProcess) -> float: return 0 -def sort_by_cpu_usage(toplist): - # type(List[px_process.PxProcess]) -> List[px_process.PxProcess] +def sort_by_cpu_usage( + toplist: List[px_process.PxProcess], cumulative=False +) -> List[px_process.PxProcess]: can_sort_by_time = False for process in toplist: - if process.cpu_time_seconds: + metric = process.cpu_time_seconds + if cumulative: + metric = process.cumulative_cpu_time_seconds + if metric: can_sort_by_time = True break if can_sort_by_time: # There is at least one > 0 time in the process list, so sorting by time # will be of some use - return sorted(toplist, key=get_notnone_cpu_time_seconds, reverse=True) + key = get_notnone_cpu_time_seconds + if cumulative: + key = get_notnone_cumulative_cpu_time_seconds + return sorted(toplist, key=key, reverse=True) # No > 0 time in the process list, try CPU percentage as an approximation of # that. This should happen on the first iteration when ptop has just been @@ -141,18 +145,18 @@ def sort_by_cpu_usage(toplist): def get_toplist( baseline: List[px_process.PxProcess], current: List[px_process.PxProcess], - sort_order: SortOrder = SortOrder.CPU, -): - # type(...) -> List[px_process.PxProcess] + sort_order=px_sort_order.SortOrder.CPU, +) -> List[px_process.PxProcess]: toplist = adjust_cpu_times(baseline, current) # Sort by interestingness last toplist = px_process.order_best_first(toplist) - if sort_order == SortOrder.MEMORY: + if sort_order == px_sort_order.SortOrder.MEMORY: toplist = sorted(toplist, key=get_notnone_memory_percent, reverse=True) - else: - # By CPU time, this is the default + elif sort_order == px_sort_order.SortOrder.CPU: toplist = sort_by_cpu_usage(toplist) + elif sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + toplist = sort_by_cpu_usage(toplist, True) return toplist @@ -359,11 +363,8 @@ def get_screen_lines( if top_mode == MODE_SEARCH: highlight_row = None - highlight_column = "CPUTIME" - if sort_order == SortOrder.MEMORY: - highlight_column = "RAM" toplist_table_lines = px_terminal.to_screen_lines( - toplist[:max_process_count], highlight_row, highlight_column + toplist[:max_process_count], highlight_row, sort_order ) # Ensure that we cover the whole screen, even if it's higher than the @@ -371,8 +372,10 @@ def get_screen_lines( toplist_table_lines += screen_rows * [""] top_what = "CPU" - if sort_order == SortOrder.MEMORY: + if sort_order == px_sort_order.SortOrder.MEMORY: top_what = "memory" + elif sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + top_what = "cumulative CPU" lines += [px_terminal.bold("Top " + top_what + " using processes")] if top_mode == MODE_SEARCH: From 10a8ee295e72764dbd4b6757ce19160ee5006405 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Sun, 24 Mar 2024 08:33:32 +0100 Subject: [PATCH 02/15] This should be it But some tests are failing. --- px/px_top.py | 24 ++++++++++++++++++++++++ tests/px_process_test.py | 22 ++++++++++++++++++++++ tests/px_top_test.py | 7 +++++-- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/px/px_top.py b/px/px_top.py index e94945c..9936f31 100644 --- a/px/px_top.py +++ b/px/px_top.py @@ -95,6 +95,29 @@ def adjust_cpu_times( return list(pid2proc.values()) +def compute_cumulative_cpu_times(toplist: List[px_process.PxProcess]) -> None: + """ + Compute cumulative CPU times for all processes in the toplist. + + This function modifies the toplist in place. + """ + + # First, find the root process + root_process = toplist[0] + while root_process.parent is not None: + root_process = root_process.parent + + # Now, walk the process tree and compute cumulative CPU times + def walk_tree(proc: px_process.PxProcess) -> float: + sum = proc.cpu_time_seconds or 0 + for child in proc.children: + sum += walk_tree(child) + proc.set_cumulative_cpu_time_seconds(sum) + return sum + + walk_tree(root_process) + + def get_notnone_cpu_time_seconds(proc: px_process.PxProcess) -> float: seconds = proc.cpu_time_seconds if seconds is not None: @@ -148,6 +171,7 @@ def get_toplist( sort_order=px_sort_order.SortOrder.CPU, ) -> List[px_process.PxProcess]: toplist = adjust_cpu_times(baseline, current) + compute_cumulative_cpu_times(toplist) # Sort by interestingness last toplist = px_process.order_best_first(toplist) diff --git a/tests/px_process_test.py b/tests/px_process_test.py index 0dec23c..3966565 100644 --- a/tests/px_process_test.py +++ b/tests/px_process_test.py @@ -353,4 +353,26 @@ def test_resolve_links(): assert p1.parent is None assert p2.parent is p1 + + # Verify both equality... assert p1.children == {p2} + # ... and identity of the child. + assert list(p1.children)[0] is p2 + + +def test_resolve_links_multiple_roots(): + """There can be only one. Root. Of the process tree.""" + processes = px_process.get_all() + process_map = {p.pid: p for p in processes} + px_process.resolve_links(process_map, testutils.local_now()) + + root0 = processes[0] + while root0.parent: + root0 = root0.parent + + for process in processes: + root = process + while root.parent: + root = root.parent + + assert root is root0 diff --git a/tests/px_top_test.py b/tests/px_top_test.py index d3f03dc..3f8714e 100644 --- a/tests/px_top_test.py +++ b/tests/px_top_test.py @@ -66,8 +66,11 @@ def test_adjust_cpu_times(): def test_get_toplist(): - # Just make sure this call doesn't crash - px_top.get_toplist(px_process.get_all(), px_process.get_all()) + toplist = px_top.get_toplist(px_process.get_all(), px_process.get_all()) + + for process in toplist: + assert process.cumulative_cpu_time_seconds is not None + assert process.cumulative_cpu_time_s != "--" def test_get_command(): From 314ef7603dc7f6720985134000bb1902e9fa003a Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Sun, 24 Mar 2024 12:42:25 +0100 Subject: [PATCH 03/15] Accept None as decorations hint Used for plain "px". --- px/px_terminal.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/px/px_terminal.py b/px/px_terminal.py index b0d5387..4c1bb25 100644 --- a/px/px_terminal.py +++ b/px/px_terminal.py @@ -315,7 +315,7 @@ def format_with_widths(widths: List[int], strings: List[str]) -> str: def to_screen_lines( procs: List[px_process.PxProcess], row_to_highlight: Optional[int], - sort_order: px_sort_order.SortOrder, + sort_order: Optional[px_sort_order.SortOrder], with_username: bool = True, ) -> List[str]: """ @@ -338,11 +338,13 @@ def to_screen_lines( "RAM", "COMMANDLINE", ] - highlight_column = 5 # "RAM" - if ( - sort_order == px_sort_order.SortOrder.CPU - or sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU - ): + highlight_column = None + if sort_order == px_sort_order.SortOrder.MEMORY: + highlight_column = 5 # "RAM" + elif sort_order in [ + px_sort_order.SortOrder.CPU, + px_sort_order.SortOrder.CUMULATIVE_CPU, + ]: highlight_column = 4 # "CPUTIME" or "CUMLCPU" # Compute widest width for pid, command, user, cpu and memory usage columns From 7a45d9c773598808d197a34ff7ccd1f4b79e5d2c Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Sun, 24 Mar 2024 16:29:22 +0100 Subject: [PATCH 04/15] Add another (failing) test --- tests/px_top_test.py | 60 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/px_top_test.py b/tests/px_top_test.py index 3f8714e..a13f418 100644 --- a/tests/px_top_test.py +++ b/tests/px_top_test.py @@ -65,6 +65,66 @@ def test_adjust_cpu_times(): assert actual == expected +def test_adjust_cpu_time_links(): + """ + Verify that adjust_cpu_time() doesn't mess up the links between parent and + child processes. + + Otherwise compute_cumulative_cpu_times() will modify the wrong processes. + """ + now = testutils.local_now() + + current = [ + px_process.create_kernel_process(now), + testutils.create_process( + pid=100, cputime="0:10.00", commandline="only in current", ppid=0 + ), + testutils.create_process( + pid=200, + cputime="0:20.00", + commandline="re-used PID baseline", + timestring="Mon May 7 09:33:11 2010", + ppid=0, + ), + testutils.create_process( + pid=300, cputime="0:30.00", commandline="relevant baseline", ppid=0 + ), + ] + pid_to_process = {p.pid: p for p in current} + px_process.resolve_links(pid_to_process, now) + + baseline = [ + px_process.create_kernel_process(now), + testutils.create_process( + pid=200, + cputime="0:02.00", + commandline="re-used PID baseline", + timestring="Mon Apr 7 09:33:11 2010", + ), + testutils.create_process( + pid=300, cputime="0:03.00", commandline="relevant baseline" + ), + testutils.create_process( + pid=400, cputime="0:03.00", commandline="only in baseline" + ), + ] + + with_adjusted_times = px_top.adjust_cpu_times(baseline, current) + parent = with_adjusted_times[0] + + # Verify that the links between parent and child processes are still intact. + assert with_adjusted_times[1].parent is parent + assert with_adjusted_times[2].parent is parent + assert with_adjusted_times[3].parent is parent + + children = sorted(parent.children, key=lambda p: p.pid) + + assert len(children) == 3 + assert children[0] is with_adjusted_times[1] + assert children[1] is with_adjusted_times[2] + assert children[2] is with_adjusted_times[3] + + def test_get_toplist(): toplist = px_top.get_toplist(px_process.get_all(), px_process.get_all()) From 8e368dbb83f000aecc77f97814a774dd52184099 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Mon, 25 Mar 2024 06:30:18 +0100 Subject: [PATCH 05/15] Make sure we're testing the right thing --- tests/px_top_test.py | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/tests/px_top_test.py b/tests/px_top_test.py index a13f418..9034580 100644 --- a/tests/px_top_test.py +++ b/tests/px_top_test.py @@ -72,6 +72,21 @@ def test_adjust_cpu_time_links(): Otherwise compute_cumulative_cpu_times() will modify the wrong processes. """ + + def verify_links(processes: list[px_process.PxProcess]): + parent = processes[0] + + assert processes[1].parent is parent + assert processes[2].parent is parent + assert processes[3].parent is parent + + children = sorted(parent.children, key=lambda p: p.pid) + + assert len(children) == 3 + assert children[0] is processes[1] + assert children[1] is processes[2] + assert children[2] is processes[3] + now = testutils.local_now() current = [ @@ -93,6 +108,9 @@ def test_adjust_cpu_time_links(): pid_to_process = {p.pid: p for p in current} px_process.resolve_links(pid_to_process, now) + # Verify links before adjust_cpu_times() + verify_links(current) + baseline = [ px_process.create_kernel_process(now), testutils.create_process( @@ -110,24 +128,13 @@ def test_adjust_cpu_time_links(): ] with_adjusted_times = px_top.adjust_cpu_times(baseline, current) - parent = with_adjusted_times[0] - - # Verify that the links between parent and child processes are still intact. - assert with_adjusted_times[1].parent is parent - assert with_adjusted_times[2].parent is parent - assert with_adjusted_times[3].parent is parent - children = sorted(parent.children, key=lambda p: p.pid) - - assert len(children) == 3 - assert children[0] is with_adjusted_times[1] - assert children[1] is with_adjusted_times[2] - assert children[2] is with_adjusted_times[3] + # Verify that the links are still good + verify_links(with_adjusted_times) def test_get_toplist(): toplist = px_top.get_toplist(px_process.get_all(), px_process.get_all()) - for process in toplist: assert process.cumulative_cpu_time_seconds is not None assert process.cumulative_cpu_time_s != "--" From 6e75b1d7d15eaf9ca66b0cb5ef05bf9159d07660 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Mon, 25 Mar 2024 07:12:42 +0100 Subject: [PATCH 06/15] Pass the cumulative times test --- px/px_top.py | 28 +++++++------ tests/px_top_test.py | 95 +++++--------------------------------------- 2 files changed, 26 insertions(+), 97 deletions(-) diff --git a/px/px_top.py b/px/px_top.py index 9936f31..416fedd 100644 --- a/px/px_top.py +++ b/px/px_top.py @@ -1,5 +1,5 @@ +import datetime import sys -import copy import logging import unicodedata @@ -14,6 +14,7 @@ from typing import List from typing import Dict +from typing import Tuple from typing import Optional LOG = logging.getLogger(__name__) @@ -50,7 +51,8 @@ def adjust_cpu_times( - baseline: List[px_process.PxProcess], current: List[px_process.PxProcess] + baseline: Dict[int, Tuple[datetime.datetime, float]], + current: List[px_process.PxProcess], ) -> List[px_process.PxProcess]: """ Identify processes in current that are also in baseline. @@ -67,13 +69,14 @@ def adjust_cpu_times( for proc in current: pid2proc[proc.pid] = proc - for baseline_proc in baseline: - current_proc = pid2proc.get(baseline_proc.pid) + for baseline_pid, baseline_times in baseline.items(): + baseline_start_time, baseline_cputime = baseline_times + current_proc = pid2proc.get(baseline_pid) if current_proc is None: # This process is newer than the baseline continue - if current_proc.start_time != baseline_proc.start_time: + if current_proc.start_time != baseline_start_time: # This PID has been reused continue @@ -81,16 +84,14 @@ def adjust_cpu_times( # We can't subtract from None continue - if baseline_proc.cpu_time_seconds is None: + if baseline_cputime is None: # We can't subtract None continue - current_proc = copy.copy(current_proc) - if current_proc.cpu_time_seconds and baseline_proc.cpu_time_seconds: + if current_proc.cpu_time_seconds and baseline_cputime: current_proc.set_cpu_time_seconds( - current_proc.cpu_time_seconds - baseline_proc.cpu_time_seconds + current_proc.cpu_time_seconds - baseline_cputime ) - pid2proc[current_proc.pid] = current_proc return list(pid2proc.values()) @@ -166,7 +167,7 @@ def sort_by_cpu_usage( def get_toplist( - baseline: List[px_process.PxProcess], + baseline: Dict[int, Tuple[datetime.datetime, float]], current: List[px_process.PxProcess], sort_order=px_sort_order.SortOrder.CPU, ) -> List[px_process.PxProcess]: @@ -560,7 +561,10 @@ def _top(search: str = "") -> None: poller = px_poller.PxPoller(px_terminal.SIGWINCH_PIPE[1]) poller.start() - baseline = poller.get_all_processes() + baseline = { + p.pid: (p.start_time, p.cpu_time_seconds or 0.0) + for p in poller.get_all_processes() + } current = poller.get_all_processes() toplist = get_toplist(baseline, current, sort_order) diff --git a/tests/px_top_test.py b/tests/px_top_test.py index 9034580..e71e864 100644 --- a/tests/px_top_test.py +++ b/tests/px_top_test.py @@ -27,21 +27,12 @@ def test_adjust_cpu_times(): pid=300, cputime="0:30.00", commandline="relevant baseline" ), ] - baseline = [ - px_process.create_kernel_process(now), - testutils.create_process( - pid=200, - cputime="0:02.00", - commandline="re-used PID baseline", - timestring="Mon Apr 7 09:33:11 2010", - ), - testutils.create_process( - pid=300, cputime="0:03.00", commandline="relevant baseline" - ), - testutils.create_process( - pid=400, cputime="0:03.00", commandline="only in baseline" - ), - ] + baseline = { + 0: (current[0].start_time, 0), + 200: (current[2].start_time, 2.0), + 300: (current[3].start_time, 3.0), + 400: (now, 3.0), + } actual = px_process.order_best_last(px_top.adjust_cpu_times(baseline, current)) expected = px_process.order_best_last( @@ -52,7 +43,7 @@ def test_adjust_cpu_times(): ), testutils.create_process( pid=200, - cputime="0:20.00", + cputime="0:18.00", commandline="re-used PID baseline", timestring="Mon May 7 09:33:11 2010", ), @@ -65,76 +56,10 @@ def test_adjust_cpu_times(): assert actual == expected -def test_adjust_cpu_time_links(): - """ - Verify that adjust_cpu_time() doesn't mess up the links between parent and - child processes. - - Otherwise compute_cumulative_cpu_times() will modify the wrong processes. - """ - - def verify_links(processes: list[px_process.PxProcess]): - parent = processes[0] - - assert processes[1].parent is parent - assert processes[2].parent is parent - assert processes[3].parent is parent - - children = sorted(parent.children, key=lambda p: p.pid) - - assert len(children) == 3 - assert children[0] is processes[1] - assert children[1] is processes[2] - assert children[2] is processes[3] - - now = testutils.local_now() - - current = [ - px_process.create_kernel_process(now), - testutils.create_process( - pid=100, cputime="0:10.00", commandline="only in current", ppid=0 - ), - testutils.create_process( - pid=200, - cputime="0:20.00", - commandline="re-used PID baseline", - timestring="Mon May 7 09:33:11 2010", - ppid=0, - ), - testutils.create_process( - pid=300, cputime="0:30.00", commandline="relevant baseline", ppid=0 - ), - ] - pid_to_process = {p.pid: p for p in current} - px_process.resolve_links(pid_to_process, now) - - # Verify links before adjust_cpu_times() - verify_links(current) - - baseline = [ - px_process.create_kernel_process(now), - testutils.create_process( - pid=200, - cputime="0:02.00", - commandline="re-used PID baseline", - timestring="Mon Apr 7 09:33:11 2010", - ), - testutils.create_process( - pid=300, cputime="0:03.00", commandline="relevant baseline" - ), - testutils.create_process( - pid=400, cputime="0:03.00", commandline="only in baseline" - ), - ] - - with_adjusted_times = px_top.adjust_cpu_times(baseline, current) - - # Verify that the links are still good - verify_links(with_adjusted_times) - - def test_get_toplist(): - toplist = px_top.get_toplist(px_process.get_all(), px_process.get_all()) + current = px_process.get_all() + baseline = {p.pid: (p.start_time, p.cpu_time_seconds or 0.0) for p in current} + toplist = px_top.get_toplist(baseline, px_process.get_all()) for process in toplist: assert process.cumulative_cpu_time_seconds is not None assert process.cumulative_cpu_time_s != "--" From bb307bf4120d151eddcc03c53583593dbb8a22d7 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Tue, 26 Mar 2024 21:20:57 +0100 Subject: [PATCH 07/15] Highlight the correct values --- px/px_terminal.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/px/px_terminal.py b/px/px_terminal.py index 4c1bb25..3533ff8 100644 --- a/px/px_terminal.py +++ b/px/px_terminal.py @@ -325,7 +325,6 @@ def to_screen_lines( If highligh_heading contains a column name, that column will be highlighted. The column name must be from the hard coded list in this function, see below. """ - cputime_name = "CPUTIME" if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: cputime_name = "CUMLCPU" @@ -405,11 +404,12 @@ def to_screen_lines( if proc.memory_percent is not None and proc.memory_percent > max_memory_percent: max_memory_percent = proc.memory_percent max_memory_percent_s = proc.memory_percent_s - if ( - proc.cpu_time_seconds is not None - and proc.cpu_time_seconds > max_cpu_time_seconds - ): - max_cpu_time_seconds = proc.cpu_time_seconds + + cpu_time_seconds = proc.cpu_time_seconds + if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + cpu_time_seconds = proc.cumulative_cpu_time_seconds + if cpu_time_seconds is not None and cpu_time_seconds > max_cpu_time_seconds: + max_cpu_time_seconds = cpu_time_seconds current_user = os.environ.get("SUDO_USER") or getpass.getuser() for line_number, proc in enumerate(procs): @@ -428,12 +428,17 @@ def to_screen_lines( cpu_time_s = proc.cpu_time_s if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: cpu_time_s = proc.cumulative_cpu_time_s - if not proc.cpu_time_seconds: + + cpu_time_seconds = proc.cpu_time_seconds + if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + cpu_time_seconds = proc.cumulative_cpu_time_seconds + + if not cpu_time_seconds: # Zero or undefined cpu_time_s = faint(cpu_time_s.rjust(cputime_width)) - elif proc.cpu_time_seconds > 0.75 * max_cpu_time_seconds: + elif cpu_time_seconds > 0.75 * max_cpu_time_seconds: cpu_time_s = bold(cpu_time_s.rjust(cputime_width)) - elif proc.cpu_time_seconds < 0.1 * max_cpu_time_seconds: + elif cpu_time_seconds < 0.1 * max_cpu_time_seconds: cpu_time_s = faint(cpu_time_s.rjust(cputime_width)) # NOTE: This logic should match its friend in From ede610a16e33d92d71f8c8a61313ae8cb568331f Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 27 Mar 2024 06:37:44 +0100 Subject: [PATCH 08/15] Self review fixes --- px/px_terminal.py | 8 +++----- px/px_top.py | 5 +---- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/px/px_terminal.py b/px/px_terminal.py index 3533ff8..83755ef 100644 --- a/px/px_terminal.py +++ b/px/px_terminal.py @@ -322,9 +322,9 @@ def to_screen_lines( Returns an array of lines that can be printed to screen. Lines are not cropped, so they can be longer than the screen width. - If highligh_heading contains a column name, that column will be highlighted. - The column name must be from the hard coded list in this function, see below. + If sort_order is set, the sort order column will be highlighted. """ + cputime_name = "CPUTIME" if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: cputime_name = "CUMLCPU" @@ -426,11 +426,9 @@ def to_screen_lines( memory_percent_s = bold(memory_percent_s.rjust(mem_width)) cpu_time_s = proc.cpu_time_s - if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: - cpu_time_s = proc.cumulative_cpu_time_s - cpu_time_seconds = proc.cpu_time_seconds if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + cpu_time_s = proc.cumulative_cpu_time_s cpu_time_seconds = proc.cumulative_cpu_time_seconds if not cpu_time_seconds: diff --git a/px/px_top.py b/px/px_top.py index 416fedd..013e897 100644 --- a/px/px_top.py +++ b/px/px_top.py @@ -561,11 +561,8 @@ def _top(search: str = "") -> None: poller = px_poller.PxPoller(px_terminal.SIGWINCH_PIPE[1]) poller.start() - baseline = { - p.pid: (p.start_time, p.cpu_time_seconds or 0.0) - for p in poller.get_all_processes() - } current = poller.get_all_processes() + baseline = {p.pid: (p.start_time, p.cpu_time_seconds or 0.0) for p in current} toplist = get_toplist(baseline, current, sort_order) From 1d531dc82ce8f12d76c7383447920f129c2b7016 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 27 Mar 2024 18:26:53 +0100 Subject: [PATCH 09/15] Improve CPU time highlighting In the cumulative case. --- px/px_terminal.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/px/px_terminal.py b/px/px_terminal.py index 83755ef..b9844be 100644 --- a/px/px_terminal.py +++ b/px/px_terminal.py @@ -407,6 +407,12 @@ def to_screen_lines( cpu_time_seconds = proc.cpu_time_seconds if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + if proc.pid <= 1: + # Both the kernel (PID 0) and the init process (PID 1) will just + # have contain the total time of all other processes. Since we + # only use this max value for highlighting (see below), if we + # include these only they will be highlighted. So we skip them. + continue cpu_time_seconds = proc.cumulative_cpu_time_seconds if cpu_time_seconds is not None and cpu_time_seconds > max_cpu_time_seconds: max_cpu_time_seconds = cpu_time_seconds From d8715a7fdaef358f4ab8dd1f14a26922b13a8311 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 27 Mar 2024 19:04:44 +0100 Subject: [PATCH 10/15] Order processes by cumulative CPU time But keep the tree structure. --- px/px_process.py | 5 ++--- px/px_top.py | 40 ++++++++++++++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/px/px_process.py b/px/px_process.py index 2ccad7e..1413d0e 100644 --- a/px/px_process.py +++ b/px/px_process.py @@ -13,7 +13,6 @@ from typing import Dict -from typing import MutableSet from typing import Optional from typing import List from typing import Iterable @@ -145,7 +144,7 @@ def __init__( self.set_cpu_time_seconds(cpu_time) self.set_cumulative_cpu_time_seconds(cumulative_cpu_time) - self.children: MutableSet[PxProcess] = set() + self.children: List[PxProcess] = [] self.parent: Optional[PxProcess] = None def __repr__(self): @@ -402,7 +401,7 @@ def resolve_links(processes: Dict[int, PxProcess], now: datetime.datetime) -> No process.parent = processes.get(process.ppid) if process.parent is not None: - process.parent.children.add(process) + process.parent.children.append(process) def remove_process_and_descendants(processes: Dict[int, PxProcess], pid: int) -> None: diff --git a/px/px_top.py b/px/px_top.py index 013e897..c531ca0 100644 --- a/px/px_top.py +++ b/px/px_top.py @@ -141,13 +141,11 @@ def get_notnone_memory_percent(proc: px_process.PxProcess) -> float: def sort_by_cpu_usage( - toplist: List[px_process.PxProcess], cumulative=False + toplist: List[px_process.PxProcess], ) -> List[px_process.PxProcess]: can_sort_by_time = False for process in toplist: metric = process.cpu_time_seconds - if cumulative: - metric = process.cumulative_cpu_time_seconds if metric: can_sort_by_time = True break @@ -156,8 +154,6 @@ def sort_by_cpu_usage( # There is at least one > 0 time in the process list, so sorting by time # will be of some use key = get_notnone_cpu_time_seconds - if cumulative: - key = get_notnone_cumulative_cpu_time_seconds return sorted(toplist, key=key, reverse=True) # No > 0 time in the process list, try CPU percentage as an approximation of @@ -166,6 +162,38 @@ def sort_by_cpu_usage( return sorted(toplist, key=lambda process: process.cpu_percent or 0, reverse=True) +def sort_by_cpu_usage_tree( + toplist: List[px_process.PxProcess], +) -> List[px_process.PxProcess]: + """ + Sort the process list by cumulative CPU time, but keep the tree structure. + """ + root_process = toplist[0] + while root_process.parent is not None: + root_process = root_process.parent + + def sort_children(proc: px_process.PxProcess) -> None: + proc.children = sorted( + proc.children, key=get_notnone_cumulative_cpu_time_seconds, reverse=True + ) + for child in proc.children: + sort_children(child) + + sort_children(root_process) + + # Now, recreate the list by flattening the tree + flat_list = [] + + def flatten(proc: px_process.PxProcess) -> None: + flat_list.append(proc) + for child in proc.children: + flatten(child) + + flatten(root_process) + + return flat_list + + def get_toplist( baseline: Dict[int, Tuple[datetime.datetime, float]], current: List[px_process.PxProcess], @@ -181,7 +209,7 @@ def get_toplist( elif sort_order == px_sort_order.SortOrder.CPU: toplist = sort_by_cpu_usage(toplist) elif sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: - toplist = sort_by_cpu_usage(toplist, True) + toplist = sort_by_cpu_usage_tree(toplist) return toplist From bbc71f0457984f80a5d95d6e71a192afb6abffb4 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 27 Mar 2024 19:19:49 +0100 Subject: [PATCH 11/15] Fix typing problems --- tests/px_process_test.py | 2 +- tests/px_processinfo_test.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/px_process_test.py b/tests/px_process_test.py index 3966565..5213814 100644 --- a/tests/px_process_test.py +++ b/tests/px_process_test.py @@ -355,7 +355,7 @@ def test_resolve_links(): assert p2.parent is p1 # Verify both equality... - assert p1.children == {p2} + assert p1.children == [p2] # ... and identity of the child. assert list(p1.children)[0] is p2 diff --git a/tests/px_processinfo_test.py b/tests/px_processinfo_test.py index 47c4634..60ab0ba 100644 --- a/tests/px_processinfo_test.py +++ b/tests/px_processinfo_test.py @@ -140,10 +140,10 @@ def test_print_process_subtree(): lines: List[Tuple[str, px_process.PxProcess]] = [] child_proc = testutils.create_process(pid=2, commandline="child") - child_proc.children = set() + child_proc.children = [] parent_proc = testutils.create_process(pid=1, commandline="parent") - parent_proc.children = {child_proc} + parent_proc.children = [child_proc] px_processinfo.print_process_subtree(sys.stdout.fileno(), parent_proc, 0, lines) From e07ff07042ee9e7a4896c3e183d2753314578d11 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 27 Mar 2024 19:21:51 +0100 Subject: [PATCH 12/15] Pass the tests again --- tests/px_process_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/px_process_test.py b/tests/px_process_test.py index 5213814..2cb94ee 100644 --- a/tests/px_process_test.py +++ b/tests/px_process_test.py @@ -133,7 +133,7 @@ def _validate_references(processes): else: assert process.parent is not None - assert isinstance(process.children, set) + assert isinstance(process.children, list) if process.parent: assert process.parent in processes assert process.parent.pid == process.ppid From e7d5de7b32728e592079b6bb30c72fa7601e47f2 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Thu, 28 Mar 2024 06:42:31 +0100 Subject: [PATCH 13/15] Make it look like a tree --- px/px_process.py | 7 +++++++ px/px_terminal.py | 8 ++++++-- px/px_top.py | 5 +++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/px/px_process.py b/px/px_process.py index 1413d0e..9b16e47 100644 --- a/px/px_process.py +++ b/px/px_process.py @@ -147,6 +147,10 @@ def __init__( self.children: List[PxProcess] = [] self.parent: Optional[PxProcess] = None + # How many levels down the tree this process is. Kernel is level 0, init + # level 1 and everything else 2 and up. + self.level = 0 + def __repr__(self): # I guess this is really what __str__ should be doing, but the point of # implementing this method is to make the py.test output more readable, @@ -404,6 +408,9 @@ def resolve_links(processes: Dict[int, PxProcess], now: datetime.datetime) -> No process.parent.children.append(process) +# FIXME: Make the tree look like a tree in cumulative CPU time mode + + def remove_process_and_descendants(processes: Dict[int, PxProcess], pid: int) -> None: process = processes[pid] if process.parent is not None: diff --git a/px/px_terminal.py b/px/px_terminal.py index b9844be..21e42b9 100644 --- a/px/px_terminal.py +++ b/px/px_terminal.py @@ -355,7 +355,7 @@ def to_screen_lines( mem_width = len(headings[5]) for proc in procs: pid_width = max(pid_width, len(str(proc.pid))) - command_width = max(command_width, len(proc.command)) + command_width = max(command_width, len(proc.command) + proc.level * 2) username_width = max(username_width, len(proc.username)) cpu_width = max(cpu_width, len(proc.cpu_percent_s)) @@ -454,9 +454,13 @@ def to_screen_lines( # Neither root nor ourselves, highlight! owner = bold(owner) + indent = "" + if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + indent = " " * proc.level * 2 + columns = [ str(proc.pid), - proc.command, + indent + proc.command, owner, cpu_percent_s, cpu_time_s, diff --git a/px/px_top.py b/px/px_top.py index c531ca0..5f67c30 100644 --- a/px/px_top.py +++ b/px/px_top.py @@ -184,10 +184,11 @@ def sort_children(proc: px_process.PxProcess) -> None: # Now, recreate the list by flattening the tree flat_list = [] - def flatten(proc: px_process.PxProcess) -> None: + def flatten(proc: px_process.PxProcess, level=0) -> None: + proc.level = level flat_list.append(proc) for child in proc.children: - flatten(child) + flatten(child, level + 1) flatten(root_process) From 2c81d80d872a9396153128dcd5c904cdf8f5d447 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Thu, 28 Mar 2024 06:49:09 +0100 Subject: [PATCH 14/15] The word is "aggregated" --- px/px_process.py | 17 +++++++---------- px/px_sort_order.py | 4 ++-- px/px_terminal.py | 24 ++++++++++++------------ px/px_top.py | 30 +++++++++++++++--------------- tests/px_top_test.py | 4 ++-- 5 files changed, 38 insertions(+), 41 deletions(-) diff --git a/px/px_process.py b/px/px_process.py index 9b16e47..fe65aba 100644 --- a/px/px_process.py +++ b/px/px_process.py @@ -91,7 +91,7 @@ def __init__( memory_percent: Optional[float] = None, cpu_percent: Optional[float] = None, cpu_time: Optional[float] = None, - cumulative_cpu_time: Optional[float] = None, + aggregated_cpu_time: Optional[float] = None, ) -> None: self.pid: int = pid self.ppid: Optional[int] = ppid @@ -142,7 +142,7 @@ def __init__( self.cpu_percent_s = f"{cpu_percent:.0f}%" self.set_cpu_time_seconds(cpu_time) - self.set_cumulative_cpu_time_seconds(cumulative_cpu_time) + self.set_aggregated_cpu_time_seconds(aggregated_cpu_time) self.children: List[PxProcess] = [] self.parent: Optional[PxProcess] = None @@ -178,12 +178,12 @@ def set_cpu_time_seconds(self, seconds: Optional[float]) -> None: self.cpu_time_s = seconds_to_str(seconds) self.cpu_time_seconds = seconds - def set_cumulative_cpu_time_seconds(self, seconds: Optional[float]) -> None: - self.cumulative_cpu_time_s: str = "--" - self.cumulative_cpu_time_seconds = None + def set_aggregated_cpu_time_seconds(self, seconds: Optional[float]) -> None: + self.aggregated_cpu_time_s: str = "--" + self.aggregated_cpu_time_seconds = None if seconds is not None: - self.cumulative_cpu_time_s = seconds_to_str(seconds) - self.cumulative_cpu_time_seconds = seconds + self.aggregated_cpu_time_s = seconds_to_str(seconds) + self.aggregated_cpu_time_seconds = seconds def match(self, string, require_exact_user=True): """ @@ -408,9 +408,6 @@ def resolve_links(processes: Dict[int, PxProcess], now: datetime.datetime) -> No process.parent.children.append(process) -# FIXME: Make the tree look like a tree in cumulative CPU time mode - - def remove_process_and_descendants(processes: Dict[int, PxProcess], pid: int) -> None: process = processes[pid] if process.parent is not None: diff --git a/px/px_sort_order.py b/px/px_sort_order.py index 583c28d..0c5c2df 100644 --- a/px/px_sort_order.py +++ b/px/px_sort_order.py @@ -4,11 +4,11 @@ class SortOrder(enum.Enum): CPU = 1 MEMORY = 2 - CUMULATIVE_CPU = 3 + AGGREGATED_CPU = 3 def next(self): if self == SortOrder.CPU: return SortOrder.MEMORY if self == SortOrder.MEMORY: - return SortOrder.CUMULATIVE_CPU + return SortOrder.AGGREGATED_CPU return SortOrder.CPU diff --git a/px/px_terminal.py b/px/px_terminal.py index 21e42b9..0cce65e 100644 --- a/px/px_terminal.py +++ b/px/px_terminal.py @@ -326,8 +326,8 @@ def to_screen_lines( """ cputime_name = "CPUTIME" - if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: - cputime_name = "CUMLCPU" + if sort_order == px_sort_order.SortOrder.AGGREGATED_CPU: + cputime_name = "AGGRCPU" headings = [ "PID", "COMMAND", @@ -342,9 +342,9 @@ def to_screen_lines( highlight_column = 5 # "RAM" elif sort_order in [ px_sort_order.SortOrder.CPU, - px_sort_order.SortOrder.CUMULATIVE_CPU, + px_sort_order.SortOrder.AGGREGATED_CPU, ]: - highlight_column = 4 # "CPUTIME" or "CUMLCPU" + highlight_column = 4 # "CPUTIME" or "AGGRCPU" # Compute widest width for pid, command, user, cpu and memory usage columns pid_width = len(headings[0]) @@ -360,8 +360,8 @@ def to_screen_lines( cpu_width = max(cpu_width, len(proc.cpu_percent_s)) cputime_s = proc.cpu_time_s - if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: - cputime_s = proc.cumulative_cpu_time_s + if sort_order == px_sort_order.SortOrder.AGGREGATED_CPU: + cputime_s = proc.aggregated_cpu_time_s cputime_width = max(cputime_width, len(cputime_s)) mem_width = max(mem_width, len(proc.memory_percent_s)) @@ -406,14 +406,14 @@ def to_screen_lines( max_memory_percent_s = proc.memory_percent_s cpu_time_seconds = proc.cpu_time_seconds - if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + if sort_order == px_sort_order.SortOrder.AGGREGATED_CPU: if proc.pid <= 1: # Both the kernel (PID 0) and the init process (PID 1) will just # have contain the total time of all other processes. Since we # only use this max value for highlighting (see below), if we # include these only they will be highlighted. So we skip them. continue - cpu_time_seconds = proc.cumulative_cpu_time_seconds + cpu_time_seconds = proc.aggregated_cpu_time_seconds if cpu_time_seconds is not None and cpu_time_seconds > max_cpu_time_seconds: max_cpu_time_seconds = cpu_time_seconds @@ -433,9 +433,9 @@ def to_screen_lines( cpu_time_s = proc.cpu_time_s cpu_time_seconds = proc.cpu_time_seconds - if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: - cpu_time_s = proc.cumulative_cpu_time_s - cpu_time_seconds = proc.cumulative_cpu_time_seconds + if sort_order == px_sort_order.SortOrder.AGGREGATED_CPU: + cpu_time_s = proc.aggregated_cpu_time_s + cpu_time_seconds = proc.aggregated_cpu_time_seconds if not cpu_time_seconds: # Zero or undefined @@ -455,7 +455,7 @@ def to_screen_lines( owner = bold(owner) indent = "" - if sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + if sort_order == px_sort_order.SortOrder.AGGREGATED_CPU: indent = " " * proc.level * 2 columns = [ diff --git a/px/px_top.py b/px/px_top.py index 5f67c30..835ace8 100644 --- a/px/px_top.py +++ b/px/px_top.py @@ -96,9 +96,9 @@ def adjust_cpu_times( return list(pid2proc.values()) -def compute_cumulative_cpu_times(toplist: List[px_process.PxProcess]) -> None: +def compute_aggregated_cpu_times(toplist: List[px_process.PxProcess]) -> None: """ - Compute cumulative CPU times for all processes in the toplist. + Compute aggregated CPU times for all processes in the toplist. This function modifies the toplist in place. """ @@ -108,12 +108,12 @@ def compute_cumulative_cpu_times(toplist: List[px_process.PxProcess]) -> None: while root_process.parent is not None: root_process = root_process.parent - # Now, walk the process tree and compute cumulative CPU times + # Now, walk the process tree and compute aggregated CPU times def walk_tree(proc: px_process.PxProcess) -> float: sum = proc.cpu_time_seconds or 0 for child in proc.children: sum += walk_tree(child) - proc.set_cumulative_cpu_time_seconds(sum) + proc.set_aggregated_cpu_time_seconds(sum) return sum walk_tree(root_process) @@ -126,8 +126,8 @@ def get_notnone_cpu_time_seconds(proc: px_process.PxProcess) -> float: return 0 -def get_notnone_cumulative_cpu_time_seconds(proc: px_process.PxProcess) -> float: - seconds = proc.cumulative_cpu_time_seconds +def get_notnone_aggregated_cpu_time_seconds(proc: px_process.PxProcess) -> float: + seconds = proc.aggregated_cpu_time_seconds if seconds is not None: return seconds return 0 @@ -166,7 +166,7 @@ def sort_by_cpu_usage_tree( toplist: List[px_process.PxProcess], ) -> List[px_process.PxProcess]: """ - Sort the process list by cumulative CPU time, but keep the tree structure. + Sort the process list by aggregated CPU time, but keep the tree structure. """ root_process = toplist[0] while root_process.parent is not None: @@ -174,7 +174,7 @@ def sort_by_cpu_usage_tree( def sort_children(proc: px_process.PxProcess) -> None: proc.children = sorted( - proc.children, key=get_notnone_cumulative_cpu_time_seconds, reverse=True + proc.children, key=get_notnone_aggregated_cpu_time_seconds, reverse=True ) for child in proc.children: sort_children(child) @@ -201,7 +201,7 @@ def get_toplist( sort_order=px_sort_order.SortOrder.CPU, ) -> List[px_process.PxProcess]: toplist = adjust_cpu_times(baseline, current) - compute_cumulative_cpu_times(toplist) + compute_aggregated_cpu_times(toplist) # Sort by interestingness last toplist = px_process.order_best_first(toplist) @@ -209,7 +209,7 @@ def get_toplist( toplist = sorted(toplist, key=get_notnone_memory_percent, reverse=True) elif sort_order == px_sort_order.SortOrder.CPU: toplist = sort_by_cpu_usage(toplist) - elif sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: + elif sort_order == px_sort_order.SortOrder.AGGREGATED_CPU: toplist = sort_by_cpu_usage_tree(toplist) return toplist @@ -425,12 +425,12 @@ def get_screen_lines( # number of processes toplist_table_lines += screen_rows * [""] - top_what = "CPU" + top_line = "Top processes by CPU usage" if sort_order == px_sort_order.SortOrder.MEMORY: - top_what = "memory" - elif sort_order == px_sort_order.SortOrder.CUMULATIVE_CPU: - top_what = "cumulative CPU" - lines += [px_terminal.bold("Top " + top_what + " using processes")] + top_line = "Top processes by memory usage" + elif sort_order == px_sort_order.SortOrder.AGGREGATED_CPU: + top_line = "Process tree ordered by aggregated CPU time" + lines += [px_terminal.bold(top_line)] if top_mode == MODE_SEARCH: lines += [SEARCH_PROMPT_ACTIVE + px_terminal.bold(search or "") + SEARCH_CURSOR] diff --git a/tests/px_top_test.py b/tests/px_top_test.py index e71e864..c894a9e 100644 --- a/tests/px_top_test.py +++ b/tests/px_top_test.py @@ -61,8 +61,8 @@ def test_get_toplist(): baseline = {p.pid: (p.start_time, p.cpu_time_seconds or 0.0) for p in current} toplist = px_top.get_toplist(baseline, px_process.get_all()) for process in toplist: - assert process.cumulative_cpu_time_seconds is not None - assert process.cumulative_cpu_time_s != "--" + assert process.aggregated_cpu_time_seconds is not None + assert process.aggregated_cpu_time_s != "--" def test_get_command(): From ee8603485b663226d2dff4217a62c566d5028055 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Thu, 28 Mar 2024 07:04:54 +0100 Subject: [PATCH 15/15] Self review --- px/px_process.py | 11 ++++------- px/px_terminal.py | 6 +++--- px/px_top.py | 11 +++-------- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/px/px_process.py b/px/px_process.py index fe65aba..6885f1d 100644 --- a/px/px_process.py +++ b/px/px_process.py @@ -91,7 +91,7 @@ def __init__( memory_percent: Optional[float] = None, cpu_percent: Optional[float] = None, cpu_time: Optional[float] = None, - aggregated_cpu_time: Optional[float] = None, + aggregated_cpu_time: float = 0.0, ) -> None: self.pid: int = pid self.ppid: Optional[int] = ppid @@ -178,12 +178,9 @@ def set_cpu_time_seconds(self, seconds: Optional[float]) -> None: self.cpu_time_s = seconds_to_str(seconds) self.cpu_time_seconds = seconds - def set_aggregated_cpu_time_seconds(self, seconds: Optional[float]) -> None: - self.aggregated_cpu_time_s: str = "--" - self.aggregated_cpu_time_seconds = None - if seconds is not None: - self.aggregated_cpu_time_s = seconds_to_str(seconds) - self.aggregated_cpu_time_seconds = seconds + def set_aggregated_cpu_time_seconds(self, seconds: float) -> None: + self.aggregated_cpu_time_s = seconds_to_str(seconds) + self.aggregated_cpu_time_seconds = seconds def match(self, string, require_exact_user=True): """ diff --git a/px/px_terminal.py b/px/px_terminal.py index 0cce65e..8194bde 100644 --- a/px/px_terminal.py +++ b/px/px_terminal.py @@ -409,9 +409,9 @@ def to_screen_lines( if sort_order == px_sort_order.SortOrder.AGGREGATED_CPU: if proc.pid <= 1: # Both the kernel (PID 0) and the init process (PID 1) will just - # have contain the total time of all other processes. Since we - # only use this max value for highlighting (see below), if we - # include these only they will be highlighted. So we skip them. + # contain the total time of all other processes. Since we only + # use this max value for highlighting (see below), if we include + # these only they will be highlighted. So we skip them. continue cpu_time_seconds = proc.aggregated_cpu_time_seconds if cpu_time_seconds is not None and cpu_time_seconds > max_cpu_time_seconds: diff --git a/px/px_top.py b/px/px_top.py index 835ace8..df1b6b1 100644 --- a/px/px_top.py +++ b/px/px_top.py @@ -126,13 +126,6 @@ def get_notnone_cpu_time_seconds(proc: px_process.PxProcess) -> float: return 0 -def get_notnone_aggregated_cpu_time_seconds(proc: px_process.PxProcess) -> float: - seconds = proc.aggregated_cpu_time_seconds - if seconds is not None: - return seconds - return 0 - - def get_notnone_memory_percent(proc: px_process.PxProcess) -> float: percent = proc.memory_percent if percent is not None: @@ -174,7 +167,9 @@ def sort_by_cpu_usage_tree( def sort_children(proc: px_process.PxProcess) -> None: proc.children = sorted( - proc.children, key=get_notnone_aggregated_cpu_time_seconds, reverse=True + proc.children, + key=lambda child: child.aggregated_cpu_time_seconds, + reverse=True, ) for child in proc.children: sort_children(child)