search

VerySleepy / verysleepy

Very Sleepy, a sampling CPU profiler for Windows
http://www.codersnotes.com/sleepy
GNU General Public License v2.0
1.07k stars 105 forks source link

Early profiling using debugging API and other improvements #20

Closed Ashod closed 9 years ago

Ashod commented 9 years ago

A handful of improvements, most notably using the Debugger API to hook into the profilee before it starts running. This is useful for short-lived processes.

Also improved 64-bit detection and error reporting. In addition, a number of const-correctness and optimizations.

Willing to split them into separate PRs if that's more convenient.

CyberShadow commented 9 years ago

Hi, thanks for the patch. A few questions,

Ashod commented 9 years ago

There seems to be some whitespace mix-up in your patch[...]

Yes. I stopped using tabs a few years back. Can convert and resubmit.

There is some currently-unused code [...] do you think it could be useful?

Most are there for completeness, but some will be useful. Didn't want to be cull prematurely.

As I understand, there is no option to use the old process starting method[...]

The old method could still be used as a fall-back method.

Do you think there can be any disadvantages to using the debugger API?[...]

Unfortunately, attaching at any time can miss some delay-loaded DLLs, late-start threads etc. So the same disadvantages apply for any method that doesn't get notifications on these events. But debugging API offer a solution.

Can this be used to profile new threads as they are created (issue #4)?

Correct. And it should. A debugger is notified of all major events including thread start/end and DLL loading/unloading. I'd like to see these added and used to their full extent, and had them in mind when I added the code above.

CyberShadow commented 9 years ago

Most are there for completeness, but some will be useful. Didn't want to be cull prematurely.

Sorry, to clarify: There already exists some similar but unused code in the Very Sleepy codebase in the file "src\profiler\debugger.cpp".

The old method could still be used as a fall-back method.

Would it make sense to expose this as a setting the user can toggle?

Ashod commented 9 years ago

Sorry, to clarify: There already exists some similar but unused code in the Very Sleepy codebase in the file "src\profiler\debugger.cpp".

My bad. Hadn't noticed this code (I wasn't even expecting it). I guess merging is in order.

Would it make sense to expose this as a setting the user can toggle?

Yes. That'd be ideal.

Perhaps it'd be best if I split this PR. First the 64-bit process detection code and misc changes can go in, then the debugging code (merged) then user option to control the hooking method. Thoughts?

CyberShadow commented 9 years ago

Perhaps it'd be best if I split this PR. First the 64-bit process detection code and misc changes can go in, then the debugging code (merged) then user option to control the hooking method. Thoughts?

I do not have a preference, so feel free to do as you find more comfortable.

Ashod commented 9 years ago

I'll close this PR and open another. Thanks.