Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement getloadavg on Windows. Fixes #604 and #1484 #1485

Merged
merged 4 commits into from Apr 11, 2019

Conversation

ammaraskar
Copy link
Contributor

@ammaraskar ammaraskar commented Apr 11, 2019

(ref. #604 and #1484)

I added some guards for Vista+ since I realized PdhAddEnglishCounterW is not available on XP.

There's a choice between calling init_loadavg_counter on import or on the first getloadavg call. I'm inclined to say the latter is better since then we don't introduce overhead when it isn't needed.

Not sure how to test properly this properly on the windows CI since there's nothing built in we can test against. There might be some external programs we can use but I'd like to get your take on it.

@ammaraskar ammaraskar force-pushed the master branch 2 times, most recently from 9fb91f3 to 0f16f77 Compare Apr 11, 2019
Copy link
Owner

@giampaolo giampaolo left a comment

Excellent code. I'll merge when you'll give me the OK.

psutil/tests/test_linux.py Outdated Show resolved Hide resolved
psutil/tests/test_memory_leaks.py Outdated Show resolved Hide resolved
psutil/tests/test_system.py Show resolved Hide resolved
psutil/arch/windows/wmi.c Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
HISTORY.rst Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
psutil/_psutil_windows.c Outdated Show resolved Hide resolved
psutil/_pswindows.py Show resolved Hide resolved
psutil/arch/windows/wmi.c Show resolved Hide resolved
@giampaolo
Copy link
Owner

giampaolo commented Apr 11, 2019

There's a choice between calling init_loadavg_counter on import or on the first getloadavg call. I'm inclined to say the latter is better since then we don't introduce overhead when it isn't needed.

I agree

Not sure how to test properly this properly on the windows CI since there's nothing built in we can test against. There might be some external programs we can use but I'd like to get your take on it.

Probably there will be something in wmic but I wouldn't bother (too boring =)).

setup.py Show resolved Hide resolved
psutil/tests/__init__.py Outdated Show resolved Hide resolved
@ammaraskar ammaraskar force-pushed the master branch 3 times, most recently from 3bb90fb to 3e82e54 Compare Apr 11, 2019
@ammaraskar ammaraskar force-pushed the master branch 4 times, most recently from 08d4ecb to 231ab22 Compare Apr 11, 2019
@ammaraskar
Copy link
Contributor Author

ammaraskar commented Apr 11, 2019

I've gone ahead and incorporated the feedback from the code review along with updating install.sh to install a more recent version of python2 (the old one was incompatible with the latest openssl) in order to fix the CI.

@giampaolo giampaolo merged commit 921870d into giampaolo:master Apr 11, 2019
@giampaolo
Copy link
Owner

giampaolo commented Apr 11, 2019

Nice, thanks Ammar.

@vstinner
Copy link

vstinner commented Apr 11, 2019

Cool stuff 👍

@vstinner
Copy link

vstinner commented Apr 11, 2019

Python also got its toy: https://github.com/python/cpython/pull/8357/files

@giampaolo
Copy link
Owner

giampaolo commented Apr 12, 2019

Cool stuff indeed. I had been floating around this for a while back in the days (the original proposal is old). There was/is a bunch of info on internet mentioning the bits with which it's theoretically possible to do this ("System Processor Queue Length") but I couldn't find any real implementation (e.g. knowing how to calculate LOADAVG_FACTOR_1F magic numbers is non trivial and requires some actual understanding of how this works).

nlevitt added a commit to nlevitt/psutil that referenced this pull request May 6, 2019
* origin/master:
  Fix giampaolo#1276: [AIX] use getargs to get process cmdline (giampaolo#1500) (patch by @wiggin15)
  Fix Process.ionice example using wrong keyword arg (giampaolo#1504)
  fix history syntax
  remove catching IOError; let the test fail and adjust it later
  Fix cpu freq (giampaolo#1496)
  pre release
  fix giampaolo#1493: [Linux] cpu_freq(): handle the case where /sys/devices/system/cpu/cpufreq/ exists but is empty.
  Revert "Fix cpu_freq (giampaolo#1493)" (giampaolo#1495)
  Fix cpu_freq (giampaolo#1493)
  Update cpu_freq to return 0 for max/min if not available (giampaolo#1487)
  give CREDITS to @agnewee for giampaolo#1491
  SunOS / net_if_addrs(): free() ifap struct on error (giampaolo#1491)
  fix giampaolo#1486: add wraps() decorator around wrap_exceptions
  refactor/move some utilities into _common.py
  update doc
  update HISTORY
  Implement getloadavg on Windows. Fixes giampaolo#604 and giampaolo#1484 (giampaolo#1485) (patch by Ammar Askar)
  give credits to @amanusk for giampaolo#1472
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants