process: Cache dlOpen lib for faster lib calls#1903
process: Cache dlOpen lib for faster lib calls#1903prashantv wants to merge 1 commit intoshirou:masterfrom
Conversation
|
Can I get a review for this optimization, it's especially helpful for use-cases that get the CPU time periodically. |
shirou
left a comment
There was a problem hiding this comment.
I see, indeed the initialization cost can be significant when it’s called frequently. The Process struct already caches some information to reduce overhead, so adding this as part of that caching aligns well with the existing implementation. Thank you.
However, I think the file name process_state_none.go is a bit unclear in terms of what "state" means. Could you consider renaming it to something like process_osprocess_fallback.go instead?
9816d91 to
afd2e88
Compare
|
@shirou Happy to, renamed |
On darwin, calls that rely on purego functions such as process times or memory info will open/close the library, which dlOpen/dlCloses the same library repeatedly. This causes them to be significantly slower when they're used for periodic metrics emission. Cache the library on the process to avoid the repeated dlOpen/dlClose. The added benchmark shows significant improvements: ``` BenchmarkProcessCPUTime-10 211495 5682 ns/op 2776 B/op 64 allocs/op BenchmarkProcessCPUTime-10 1159842 1021 ns/op 760 B/op 15 allocs/op ```
afd2e88 to
ee31621
Compare
| funcs, err := p.os.funcs() | ||
| if err != nil { | ||
| return "", err | ||
| return "", nil |
There was a problem hiding this comment.
Sorry for the late comment. I think we should return err when p.os.funcs() returns an error. Was there a specific reason you decided to return nil instead?
| funcs, err := p.os.funcs() | ||
| if err != nil { | ||
| return "", err | ||
| return "", nil |
On darwin, calls that rely on purego functions such as process times or memory info will open/close the library, which dlOpen/dlCloses the same library repeatedly. This causes them to be significantly slower when they're used for periodic metrics emission.
Cache the library on the process to avoid the repeated dlOpen/dlClose.
The added benchmark shows significant improvements: