Skip to content

Commit

Permalink
Improve default sort order
Browse files Browse the repository at this point in the history
Before this change, we used a magic formula.

With this change in place, a process will be considered most interesting
if it fulfills one of:
* It uses the most CPU right now
* It has used the most CPU over time
* It uses the most memory right now
* It was launched most recently
  • Loading branch information
walles committed Sep 30, 2023
1 parent 1a447c7 commit 723057b
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 56 deletions.
87 changes: 65 additions & 22 deletions px/px_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ def __init__(
if cpu_percent is not None:
self.cpu_percent_s = f"{cpu_percent:.0f}%"

# Setting the CPU time like this implicitly recomputes the score
self.set_cpu_time_seconds(cpu_time)

self.children: MutableSet[PxProcess] = set()
Expand All @@ -169,28 +168,13 @@ def __ne__(self, other):
def __hash__(self):
return self.pid

def _recompute_score(self):
self.score = 0.0
if self.memory_percent is None:
return
if self.cpu_time_seconds is None:
return

self.score = (
(self.cpu_time_seconds + 1.0)
* (self.memory_percent + 1.0)
/ (self.age_seconds + 1.0)
)

def set_cpu_time_seconds(self, seconds: Optional[float]) -> None:
self.cpu_time_s: str = "--"
self.cpu_time_seconds = None
if seconds is not None:
self.cpu_time_s = seconds_to_str(seconds)
self.cpu_time_seconds = seconds

self._recompute_score()

def match(self, string, require_exact_user=True):
"""
Returns True if this process matches the string.
Expand Down Expand Up @@ -458,7 +442,6 @@ def get_all() -> List[PxProcess]:
close_fds=close_fds,
env=px_exec_util.ENV,
) as ps:

stdout = ps.stdout
assert stdout
now = datetime.datetime.now().replace(tzinfo=TIMEZONE)
Expand All @@ -476,15 +459,75 @@ def get_all() -> List[PxProcess]:


def order_best_last(processes: Iterable[PxProcess]) -> List[PxProcess]:
"""Returns process list ordered with the most interesting one last"""
return sorted(processes, key=operator.attrgetter("score", "cmdline"))
"""
Returns process list ordered with the most interesting one last.
The interestingness of a process is determined by three factors:
* How recently it was started. More recently is more interesting.
* How much memory it is using. Higher memory usage is more interesting.
* How much CPU it is using right now. Higher CPU usage is more interesting.
* How much CPU time it has used. Higher CPU time is more interesting.
To sort by all three of these factors, we first make three lists. The score
of each process is determined by its best position in any of the lists.
"""

# Earlier in these lists = more interesting
by_age = sorted(
filter(lambda p: p.age_seconds is not None, processes),
key=operator.attrgetter("age_seconds"),
)
by_memory = sorted(
filter(lambda p: p.memory_percent is not None, processes),
key=operator.attrgetter("memory_percent"),
reverse=True,
)
by_cpu = sorted(
filter(lambda p: p.cpu_percent is not None, processes),
key=operator.attrgetter("cpu_percent"),
reverse=True,
)
by_cpu_time = sorted(
filter(lambda p: p.cpu_time_seconds is not None, processes),
key=operator.attrgetter("cpu_time_seconds"),
reverse=True,
)

age_to_position: Dict[int, int] = {}
for position, process in enumerate(by_age):
age_to_position[process.pid] = position

memory_to_position: Dict[float, int] = {}
for position, process in enumerate(by_memory):
memory_to_position[process.pid] = position

cpu_to_position: Dict[float, int] = {}
for position, process in enumerate(by_cpu):
cpu_to_position[process.pid] = position

cpu_time_to_position: Dict[float, int] = {}
for position, process in enumerate(by_cpu_time):
cpu_time_to_position[process.pid] = position

# Presumably higher than the number of processes on any system
NOT_FOUND = 99999

def score(process: PxProcess) -> int:
"""Higher score = more interesting"""
best_position = min(
age_to_position.get(process.pid, NOT_FOUND),
memory_to_position.get(process.pid, NOT_FOUND),
cpu_to_position.get(process.pid, NOT_FOUND),
cpu_time_to_position.get(process.pid, NOT_FOUND),
)
return -best_position

return sorted(processes, key=score)


def order_best_first(processes: Iterable[PxProcess]) -> List[PxProcess]:
"""Returns process list ordered with the most interesting one first"""
ordered = sorted(processes, key=operator.attrgetter("cmdline"))
ordered = sorted(ordered, key=operator.attrgetter("score"), reverse=True)
return ordered
return order_best_last(processes)[::-1]


def seconds_to_str(seconds: float) -> str:
Expand Down
34 changes: 0 additions & 34 deletions tests/px_process_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,6 @@ def _test_get_all():

now = testutils.local_now()
for process in all_processes:
# Scores should be computed via multiplications and divisions of
# positive numbers, if this value is negative something is wrong.
assert process.score >= 0

# Processes created in the future = fishy
assert process.age_seconds >= 0
assert process.start_time < now
Expand Down Expand Up @@ -234,36 +230,6 @@ def test_parse_time():
assert "Constantinople" in str(e.value)


def test_order_best_last():
p0 = testutils.create_process(cputime="0:10.00", mempercent="10.0")
p1 = testutils.create_process(
commandline="awk", cputime="0:11.00", mempercent="1.0"
)
p2 = testutils.create_process(
commandline="bash", cputime="0:01.00", mempercent="11.0"
)

# P0 should be last because its score is the highest, and p1 and p2 should
# be ordered alphabetically
assert px_process.order_best_last([p0, p1, p2]) == [p1, p2, p0]
assert px_process.order_best_last([p2, p1, p0]) == [p1, p2, p0]


def test_order_best_first():
p0 = testutils.create_process(cputime="0:10.00", mempercent="10.0")
p1 = testutils.create_process(
commandline="awk", cputime="0:11.00", mempercent="1.0"
)
p2 = testutils.create_process(
commandline="bash", cputime="0:01.00", mempercent="11.0"
)

# P0 should be first because its score is the highest, then p1 and p2 should
# be ordered alphabetically
assert px_process.order_best_first([p0, p1, p2]) == [p0, p1, p2]
assert px_process.order_best_first([p2, p1, p0]) == [p0, p1, p2]


def test_match():
p = testutils.create_process(uid=0, commandline="/usr/libexec/AirPlayXPCHelper")

Expand Down

0 comments on commit 723057b

Please sign in to comment.