bottom icon indicating copy to clipboard operation
bottom copied to clipboard

feature: show running time of processes

Open yshui opened this issue 2 years ago • 4 comments

Description

Add a column to the processes table to show the running time of a process

Testing

If relevant, please state how this was tested. All changes must be tested to work:

If this is a code change, please also indicate which platforms were tested:

  • [ ] Windows
  • [ ] macOS
  • [x] Linux

Checklist

If relevant, ensure the following have been met:

  • [ ] Areas your change affects have been linted using rustfmt (cargo fmt)
  • [ ] The change has been tested and doesn't appear to cause any unintended breakage
  • [ ] Documentation has been added/updated if needed (README.md, help menu, etc.)
  • [ ] The pull request passes the provided CI pipeline
  • [ ] There are no merge conflicts
  • [ ] If relevant, new tests were added (don't worry too much about coverage)

yshui avatar Sep 04 '22 00:09 yshui

I also ran it on Windows, it seems fine, though I got some... interesting results for some of the times. This might be because I'm on a VM though.

image

ClementTsang avatar Sep 05 '22 23:09 ClementTsang

interesting results for some of the times

hmm, did sysinfo return invalid run time in some cases?

yshui avatar Sep 06 '22 09:09 yshui

For example, if I have 10 firefox processes, do we show the uptime? Take the highest one?

I think we should show the sum. It's the case when a process have multiple threads, on Linux at least.

yshui avatar Sep 06 '22 09:09 yshui

I removed chrono from dependencies.

yshui avatar Sep 08 '22 21:09 yshui

Rebased a bunch of stuff and added tests. Will need to still add searching for time, but I can add that.

ClementTsang avatar Apr 30 '23 19:04 ClementTsang

Codecov Report

Patch coverage: 34.29% and project coverage change: +0.11 :tada:

Comparison is base (605314d) 28.02% compared to head (e927cd0) 28.13%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #801      +/-   ##
==========================================
+ Coverage   28.02%   28.13%   +0.11%     
==========================================
  Files          95       95              
  Lines       15923    16077     +154     
==========================================
+ Hits         4462     4524      +62     
- Misses      11461    11553      +92     
Flag Coverage Δ
macos-12 29.63% <35.85%> (+0.12%) :arrow_up:
ubuntu-latest 29.56% <34.97%> (+0.11%) :arrow_up:
windows-2019 29.68% <35.50%> (+0.12%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/app/data_harvester/processes.rs 0.00% <0.00%> (ø)
src/app/data_harvester/processes/linux.rs 19.04% <0.00%> (-0.47%) :arrow_down:
src/app/data_harvester/processes/macos_freebsd.rs 0.00% <0.00%> (ø)
src/app/data_harvester/processes/windows.rs 0.00% <0.00%> (ø)
src/app/query.rs 0.00% <0.00%> (ø)
src/utils/error.rs 9.75% <ø> (ø)
src/widgets/process_table/proc_widget_column.rs 59.82% <16.66%> (-2.45%) :arrow_down:
src/widgets/process_table.rs 61.58% <33.33%> (-0.10%) :arrow_down:
src/widgets/process_table/proc_widget_data.rs 36.32% <95.45%> (+26.73%) :arrow_up:
src/options/process_columns.rs 100.00% <100.00%> (ø)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Apr 30 '23 19:04 codecov[bot]

Will update documentation in a separate PR later.

ClementTsang avatar Apr 30 '23 19:04 ClementTsang

Tested on Windows - noticed that the ones with 19000 days were ~53 years, which lines up with the UNIX epoch - and yep, they were reporting a start time of 0.

I might just add a check to zero out anything that reports that (or maybe make the time field an option) to handle it.

ClementTsang avatar May 02 '23 03:05 ClementTsang

@all-contributors please add @yshui for code.

ClementTsang avatar May 02 '23 05:05 ClementTsang

@ClementTsang

I've put up a pull request to add @yshui! :tada:

allcontributors[bot] avatar May 02 '23 05:05 allcontributors[bot]

Is it possible to add a command-line option for this feature?

wdscxsj avatar Nov 27 '23 05:11 wdscxsj

@wdscxsj heads up that it might be preferable if you asked stuff like this in either a separate discussion or an issue, just to avoid pinging everyone who worked on this PR.

That said, it's currently supported via changing the config (e.g.):

[processes]
columns = ["PID", "Name", "CPU%", "Mem%", "R/s", "W/s", "T.Read", "T.Write", "User", "State", "Time"]

and I'm not really sure if I want to add it as a command-line flag for now (the command-line interface is already bloated as is). Ideally changing it on runtime will become doable whenever I get to in-app config changing.

ClementTsang avatar Nov 27 '23 08:11 ClementTsang