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

windows implementation based on typeperf tool #7

Closed
wants to merge 1 commit into from

Conversation

zukalt
Copy link

@zukalt zukalt commented Oct 6, 2014

Please review my changes.
It should do the same as as ps -o pcpu,rssize -p PID does.

The key rss (or rssize) looks same as "\Process(\Working Set" counter - amount of allocated memory by the process which will not result page fault when will be accessed. Similar thing is described in ps man page for rss.

The '% Processor Time' looks like similar to pcpu (not sure though). At least it shows the percentage of cpu usage by process between 2 measurements, the interval is in milliseconds.

Gurgen

@soyuka
Copy link
Owner

soyuka commented Oct 6, 2014

My build is working on windows by using wmic. You might want to test it but this refactor is too messy and does not pass tests.

@soyuka soyuka closed this Oct 6, 2014
@soyuka soyuka reopened this Oct 6, 2014
@soyuka
Copy link
Owner

soyuka commented Oct 6, 2014

wmic is the way to go in my opinion, it might be not enough tested but if you really want, you may improve it.
Did you test it as it is? Does it not work properly?

@zukalt
Copy link
Author

zukalt commented Oct 6, 2014

Regarding failing tests, actually the test itself is not correct. The \Process(*)\Working Set - Private as I also mentioned readme is not available in older versions of windows,I by mistake included it in test so it will fail under WinXP. I can fix the test if you like the approach.
In respect of using wmic or typeperf is actually up to you. I made an implication that you don't like wmic (based on you comment in readme.md) so decided to find other way for that and what I noticed is that typeperf is more popular and accurate tool for process monitoring in windows.

@soyuka
Copy link
Owner

soyuka commented Oct 6, 2014

I left some comments on the code.

Have you tested the wmic way to go that I made before? Does it return results that are consistent?

I'll test the code as soon as I get my hand on a windows computer ;).

@zukalt
Copy link
Author

zukalt commented Oct 6, 2014

with wmic I actually didn't spend much time to understand the difference with typeperf, but what I noticed that it always shows some cpu usage like 0.123 while typeperf shows 0 for same process.

Also I just checked under WinXP virtual machine and it didn't work. Looks like exec is waiting too long for wmic process to finish. I get timeout error when running test. This can be fixed by using spawn instead of exec.

@soyuka
Copy link
Owner

soyuka commented Oct 6, 2014

Interesting, so wmic is definitely less accurate, I'll take the time to test this!

soyuka pushed a commit that referenced this pull request Oct 20, 2014
@soyuka
Copy link
Owner

soyuka commented Oct 20, 2014

Please fork the windows branch if you want to have the same code as I have ;)

@soyuka
Copy link
Owner

soyuka commented Oct 21, 2014

I think I've found why it's not doing anything on my computer (and maybe appveyor). My computer is set in French and I get this output from typeperf -qx -o file.txt:
https://gist.github.com/soyuka/e3011d8b4ab9e9594569

It looks like Processor has been translated somehow, I'm lost again can't find any ID here.

@zukalt
Copy link
Author

zukalt commented Oct 21, 2014

That's tough :) It looks like there is some way to refer counters with IDs instead of names:
The

typeperf "\Process(*)\% Processor Time" -sc 1

can be also written with IDs like

typeperf \230(*)\6 -sc 1

There is some information here but still I can't get correct syntax of getting the "\Process(*)\ID Process" counter written with numeric values. I even wrote some little node program to check all possible values for \230(*)\{N} tested on N=1,10000 only works with value 6.

:( feel desperate, will try to figure this out this evening.

@soyuka
Copy link
Owner

soyuka commented Oct 21, 2014

Nice documentation thanks!

Yeah if we can't rely on strings because of the language variation it sucks :/.

I'll do some research on my own and let you know!

@soyuka
Copy link
Owner

soyuka commented Oct 21, 2014

My HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Perflib\009 is quite empty and I can't get any number to work.
I've found this blog article about cpu/mem checks in windows cli, might pick your interest.

@zukalt
Copy link
Author

zukalt commented Oct 21, 2014

I tried under win xp and it didn't work even though the registry path is there and values are identical.
Just can't believe that they made the shell command locale specific. I'll spend some time to understand how this can be fixed.
At this moment I tend to believe that I won't find anything with typeperf.

I have idea to put some script on npm postinstall handler and check windows locale. if it's English (or at least commands are working) then keep using typeperf otherwise print some warning and/or provide information based on some less accurate tools. What do you think of that?
Of course if we don't find better replacement for typeperf.

@soyuka
Copy link
Owner

soyuka commented Oct 21, 2014

Just can't believe that they made the shell command locale specific.

+1 indeed that's just crap!

Yeah but a postfix command makes the script a bit more dirty than it is (not really testable) and I'd like to keep this as simple as possible. What about wmic, could we improve it? It looks like it's the best portable tool.

@zukalt
Copy link
Author

zukalt commented Oct 22, 2014

Yes, i guess the wmic is last hope. Yesterday i didn't get how you
calculate cpu usage. Do you lnow where can i read more about that?

@soyuka
Copy link
Owner

soyuka commented Oct 22, 2014

https://github.com/soyuka/pidusage/blob/master/lib/stats.js#L125

It's well commented with references ;)

Basically:

'wmic PROCESS '+pid+' get workingsetsize,usermodetime,kernelmodetime'

See the third reply there and then pcpu:

      var total = (stats.usermodetime + stats.kernelmodetime) / 10000000 //seconds

See wmic doc

@zukalt
Copy link
Author

zukalt commented Oct 23, 2014

That comment refers to this link as for detailed explanation which is broken, so I can't be sure how accurate it is.
Apart from that I think to calculate the usage you need to have two measurements between known interval. So expect that you need to run that command twice and then calculate it (the same is written in comment).
CPU = (((New_UserModeTime - Old_UserModeTime) + (New_KernelModeTime - Old_KernelModeTime)) / TimeInterval) * 100;
But I see problem measuring the TimeInterval. We create child processes and get the results when they are written to stdout so our results are not so accurate ( I asses the accuracy is ~100ms). I guess if need to do 2 different measurements within interval of 2-3 seconds for more accuracy. But the value will be different than typeperf (its interval is in milliseconds) or task manager for example.

One more important thing in that cpu usage formula i guess MUST use the number of processors.

Maybe we can have small discussion with skype to understand what we can do. There are more questions for me than answers. My account is - zu_kalt.

@soyuka
Copy link
Owner

soyuka commented Oct 23, 2014

Interesting.
I should be able to add an history that stores time with old and new usermode, kernelmode. I think this might be better than the actual way of doing things.

How would the formula be altered by the number of processors? wmic isn't taking this parameter internally?

Come here to discuss: https://gitter.im/soyuka/pidusage?utm_source=share-link&utm_medium=link&utm_campaign=share-link
Haven't find you on skype but my acc is soyuka

@soyuka
Copy link
Owner

soyuka commented Nov 12, 2014

#10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants