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

when I List process info on win10, it running a long time(30s), cpu usage 30~40% #250

Closed
jindongyi011039 opened this issue Aug 25, 2016 · 24 comments

Comments

@jindongyi011039
Copy link

when I List processes info on win10, it running a long time(30s), cpu usage 30~40%
but on linux , it just need 100ms

@shirou
Copy link
Owner

shirou commented Aug 25, 2016

To get processes on Windows, gopsutil WMI. and WMI is slow.
We plan to change other method like https://github.com/lxn/win, but actually, not started yet.

@kondr1
Copy link

kondr1 commented Nov 23, 2016

When you fix this?

@shirou
Copy link
Owner

shirou commented Nov 23, 2016

Sorry, I haven't have a time for a while. But, PR is always welcome!

@pqant
Copy link

pqant commented Feb 10, 2018

is there any progress about it ? WMI is really slow..

@shirou
Copy link
Owner

shirou commented Feb 11, 2018

Implementation becomes slightly better from this issue first opened. Actually, which API is slow and/or could you give us a sample code and benchmark?

@pqant
Copy link

pqant commented Feb 12, 2018

@shirou

Sure :

This is the method that burst cpu usage from %2-5 to %60-80.

package main

import (
	"github.com/gin-gonic/gin"
	"github.com/bamzi/jobrunner"
	"github.com/matishsiao/goInfo"
	"github.com/shirou/gopsutil/process" 
        ...
)

func FindProcessByCommandLineInfo(processInfo string) bool {
	if processInfo == "" {
		log.Fatalf("processInfo not specified!")
	}
	v, _ := process.Processes()
	for _, value := range v {
		cmdLine, _ := value.Cmdline()
		if strings.Contains(cmdLine, processInfo) {
			return true
		}
	}
	return false
}

@shirou
Copy link
Owner

shirou commented Feb 13, 2018

func BenchmarkProcesses(b *testing.B){
	for i := 0; i < b.N; i++ {
		Pids()
    }
}
func BenchmarkCmdline(b *testing.B){
	v, _ := Processes()
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		v[0].Cmdline()
    }
}
func BenchmarkTimes(b *testing.B){
	v, _ := Processes()
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		v[0].Times()
    }
}

Then get this result

BenchmarkProcesses-4         500           2366454 ns/op
BenchmarkCmdline-4            50          36758234 ns/op
BenchmarkTimes-4         2000000               623 ns/op

This means Processes is slow and Cmdline is very very slow. Eventually GetWin32Proc uses WMI and it is a problem. In fact, Times which does not use WMI is very fast.

So, we should eliminate WMI especially GetWin32Proc.

@pqant
Copy link

pqant commented Feb 14, 2018

@shirou yes it seems true. Dou you need something to expand border of your investigation?

@shirou
Copy link
Owner

shirou commented Feb 14, 2018

Actually Windows functions are spreads widely to get various process information. So we should implement functions one by one. To implement those, some kind of Ki or fired up is required, so please just wait or I will appreciate if you send a PR!

@Lomanic
Copy link
Collaborator

Lomanic commented Feb 14, 2018

@shirou porting WMI calls to Go syscalls is on my TODO list since a very long time, I did not have enough time lately to tackle this big problem, but I will probably be less busy in the next following weeks. As far as I remember, retrieving command-line of a process is done in python's psutil by using the (quite undocumented and maybe subject to future breaking changes) PEB structure (psutil_get_cmdline() calls psutil_get_process_data() which retrieves and uses the PEB structure of the given process), which is not straightforward at all to use… Everything I read elsewhere relies on the PEB structure too.

@pqant
Copy link

pqant commented Feb 16, 2018

@shirou Thank you.

btw , Unfortunately process.Processes() is not stable.
I have to write another method to validate it but different style.
The method that I wrote is below. Please just focus on CheckProcessForOnlyWindows. I wrote it because I had to. (There was a time pressure on the project. When I find a proper free time slot then I'll change it.) process.Processes() doesn't work properly.

screenshot -> Screenshot#1

func FindProcessByName(processInfo string, verification string, osType OSType) (bool, error, bool) {
	if processInfo == "" {
		log.Fatalf("processInfo not specified!")
	}
	CheckProcessForOnlyWindows := func(verification string) bool {
		if *commandLogActivePtr {
			color.Green("%v - >>> Deep Process Searching mode has been activated!", NowTr())
		}
		commandNew := `cmd /C wmic path win32_process get /format:list | findstr -i "@@APPINFO@@"`
		commandNew = strings.Replace(commandNew, "@@APPINFO@@", *commandFindPtr, -1)
		if FileExists(procFinderPath) {
			err := DeleteFile(procFinderPath)
			if err != nil {
				color.Red("%v - Process Finder File Deletion Error : %v", NowTr(), err)
				return false
			}
		}
		batFileContent := []byte(commandNew)
		err := ioutil.WriteFile(procFinderPath, batFileContent, 0755)
		if err != nil {
			color.Red("%v - Process Finder File Content Writing Error : %v", NowTr(), err)
			return false
		}
		result := executeDirectly(procFinderPath)

		if result == "" {
			return false
		} else {
			if strings.Contains(result, verification) {
				return true
			} else {
				return false
			}
		}
	}
	v, err := process.Processes()
	if err != nil {
		if osType != Windows {
			return false, err, false
		} else {
			ans := CheckProcessForOnlyWindows(verification)
			if ans {
				return ans, nil, true
			} else {
				return ans, err, true
			}
		}
	}
	for _, value := range v {
		cmdLine, errIn := value.Cmdline()
		if errIn != nil {
			if osType != Windows {
				return false, err, false
			} else {
				ans := CheckProcessForOnlyWindows(verification)
				if ans {
					return ans, nil, true
				} else {
					return ans, errIn, true
				}
			}
		}
		if strings.Contains(cmdLine, processInfo) {
			return true, nil, false
		}
	}
	if osType != Windows {
		return false, err, false
	} else {
		return CheckProcessForOnlyWindows(verification), nil, true
	}
}

@Lomanic
Copy link
Collaborator

Lomanic commented Aug 16, 2019

The only remaining functions still using WMI are now IOCounters() and Cmdline().

IOCounters would probably rely on GetProcessIoCounters. Cmdline is the most difficult one to port to win32 API as there is no other way to get this information but by calling NtQuerySystemInformation (which for the moment I'm not even able to do by reusing existing code in common_windows.go), with some wow64 subtleties according to psutil (from my previous comment in this issue) for parsing the PEB structure of the process. Since win8.1 there is a new parameter ProcessCommandLineInformation (=60) to NtQuerySystemInformation where parsing PEB structure is not needed.

@shirou
Copy link
Owner

shirou commented Aug 17, 2019

I am really amazed about your great work @Lomanic !
Parsing PEB sounds hard. I think we can use ProcessCommandLineInformation and if failed, fallback to wmi.

@Lomanic
Copy link
Collaborator

Lomanic commented Aug 17, 2019

Thanks @shirou. Yes, that was the idea, in a first step before implementing properly NtQuerySystemInformation with PEB parsing.

@Lomanic
Copy link
Collaborator

Lomanic commented Aug 18, 2019

NtQueryProcessBasicInformation in cloudfoundry/gosigar shows how to how to call NtQuerySystemInformation in Go with ProcessBasicInformation, which gives us the PebBaseAddress of a process.

@Lomanic
Copy link
Collaborator

Lomanic commented Aug 19, 2019

I pushed a commit in https://github.com/Lomanic/gopsutil/tree/issue250, but as said in the commit message this is functional albeit very hacky and will need some help regarding how to properly parse the result returned by NtQueryInformationProcess to a string (using an array of uint16 and converting it to string with windows.UTF16ToString like in other calls only works on GOARCH=386 on wow64 but not when compiled with GOARCH=amd64…?!). Edit: it's probably a UNICODE_STRING, see https://github.com/hillu/go-ntdll/blob/dd4204aa705ec72221a10165524a261ac3f5ef64/unicode_string.go#L23 for a conversion to a string

Speed improvement is by approx. of a factor of 10 as I remember (down to a few ms for each process on my win10 VM).

@Lomanic
Copy link
Collaborator

Lomanic commented Aug 20, 2019

Current progress: after implementing the UNICODE_STRING type, working properly when running as Administrator on Win10 but failing hard with a panic as a non-elevated user with this stacktrace (different when compiling with GOARCH=386 for even more fun)

panic: runtime error: growslice: cap out of range

goroutine 1 [running]:
fmt.(*buffer).WriteString(...)
        c:/go/src/fmt/print.go:83
fmt.(*fmt).padString(0xc00009a040, 0x700041002e0070, 0x6b007400580070)
        c:/go/src/fmt/format.go:110 +0x101
fmt.(*fmt).fmtS(0xc00009a040, 0x700041002e0070, 0x6b007400580070)
        c:/go/src/fmt/format.go:347 +0x68
fmt.(*pp).fmtString(0xc00009a000, 0x700041002e0070, 0x6b007400580070, 0x76)
        c:/go/src/fmt/print.go:448 +0x139
fmt.(*pp).printArg(0xc00009a000, 0x512c60, 0xc000142620, 0x76)
        c:/go/src/fmt/print.go:684 +0x887
fmt.(*pp).doPrintln(0xc00009a000, 0xc000069eb8, 0x2, 0x2)
        c:/go/src/fmt/print.go:1159 +0xb9
fmt.Fprintln(0x569000, 0xc000006020, 0xc000069eb8, 0x2, 0x2, 0x0, 0x0, 0x0)
        c:/go/src/fmt/print.go:265 +0x5f
fmt.Println(...)
        c:/go/src/fmt/print.go:275
main.main()
        C:/Users/user/go/src/github.com/Lomanic/gopsutiltest/proccmdline.go:21 +0x1b5
exit status 2

With proccmdline.go being this program

package main

import (
	"fmt"
	"os"
	"time"

	"github.com/shirou/gopsutil/process"
)

func main() {
	start := time.Now()
	procs, err := process.Processes()
	if err != nil {
		fmt.Println("A", err)
		os.Exit(1)
	}
	for _, proc := range procs {
		fmt.Println("PID", proc.Pid)
		// proc.Cmdline()
		fmt.Println(proc.Cmdline())
	}
	elapsed := time.Now().Sub(start)
	fmt.Println("Elapsed time", elapsed, "(", elapsed/(time.Duration(len(procs))), "per process).", len(procs), "processes")
}

@AtakanColak
Copy link
Contributor

I too have also observed the memory leak issue caused by Cmdline(), image attached below is divided into two sections with one being Cmdline() enabled, the other being disabled. May I ask why this memory leak might be happening?

image

Lomanic added a commit to Lomanic/gopsutil that referenced this issue Jan 26, 2020
shirou added a commit that referenced this issue Jan 27, 2020
[process][windows] Use win32 API in process.IOCounters() instead of slow WMI call #250
@Lomanic
Copy link
Collaborator

Lomanic commented Jan 27, 2020

The only remaining call to WMI is now Cmdline(), unfortunately this is the hardest one to port to native win32 API while it is needed by many users.

@AtakanColak it has to be the library StackExchange/wmi not freeing memory after use, or something on gopsutil part of course. I won't have time to debug this, I prefer to get rid off WMI instead in my limited free time. Maybe we could try the fix for StackExchange/wmi#27 (StackExchange/wmi#27 (comment)), as far as I see we don't have this in gopsutil. Would you try adding this in process/process_windows.go's init() function?

@AtakanColak
Copy link
Contributor

@Lomanic I added the comment you mentioned to the init() function;

func init() {

wmi.DefaultClient.AllowMissingFields = true

fmt.Println("hmm")

s, err := wmi.InitializeSWbemServices(wmi.DefaultClient)

if err == nil {
	wmi.DefaultClient.SWbemServicesClient = s
} else {
	fmt.Println("error", err.Error())
}

This is how I tested it;

func main() {
	for {
		ps, err := process.Processes()
		if err != nil {
			fmt.Println(err.Error())
			return
		}
		for _, p := range ps {
			p.Cmdline()
		}
	}
}

Unfortunately the memory leak persists. I have to leave it here.

@Lomanic
Copy link
Collaborator

Lomanic commented Jan 28, 2020

Thanks for testing, that could have been an easy fix.

@ghost
Copy link

ghost commented May 6, 2020

I just found this issue and it was a little late.
I have previously implemented the win32 process to read the command line parameters of the win64 process.
https://github.com/GameXG/gowindows/blob/master/process_windows.go

@Lomanic
Copy link
Collaborator

Lomanic commented May 17, 2020

Fixed by @mxmauro's awesome work at #862, I'm really happy to close this old issue 🍾

@Lomanic Lomanic closed this as completed May 17, 2020
@mxmauro
Copy link
Contributor

mxmauro commented May 17, 2020

Thanks @Lomanic, you are welcome.

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

No branches or pull requests

7 participants