google / UIforETW

User interface for recording and managing ETW traces
https://randomascii.wordpress.com/2015/04/14/uiforetw-windows-performance-made-easier/
Apache License 2.0
1.59k stars 201 forks source link

If IntelEnergyLibInitialize fails, pretend that the library does not exist #45

Closed mwinterb closed 9 years ago

mwinterb commented 9 years ago

This is primarily to fix an access violation when initially calling ReadSample I experienced with two different Xeon processors, but generally seems like good behavior.

mwinterb commented 9 years ago

In this scenario, Intel Power Gadget just exits silently, but I don't know if you'd want to emit an event that reports the initialization failure.

I'm also uncertain about calling GetMaxTemperature prior to IntelEnergyLibInitialize, but it wasn't causing any crashing issues, so I left it out of this PR. However, I've been getting random results each run if I don't wait to call GetMaxTemperature until after IntelEnergyLibInitialize completes successfully. Calling GetMaxTemperature again returns the same "random" number, but if I always call IntelEnergyLibInitialize prior to GetMaxTemperature, I consistently get 100, which is what IPG is showing me right now. I'm assuming this isn't the cause of "I get results that are clearly Fahrenheit" from 556a82e44c0525201a081efbdee21b4a3dfddd63, but the random numbers do occasionally look plausibly Fahrenheit-esque. The bogus max temperature seems to influence the result returned by GetPowerData; I'm guessing the processor returns the temperature as a percentage of the max temperature, which either the driver or the library translates to a Celsius number.

randomascii commented 9 years ago

Looks good. Thanks. I made a follow-on change to add you to CONTRIBUTORS and to make your suggested GetMaxTemperature() change.

randomascii commented 9 years ago

I added your github name (rather than your real name) to CONTRIBUTORS and failed to add it at all to AUTHORS, which should also contain it. You can fix these with another pull request or tell me what name you want.

mwinterb commented 9 years ago

I feel a bit silly with a pull request for just that, but I have two more pull requests pending, so I'll include those in the one that's unambiguously fine so you won't need to split them in two if you're fine with one and not the other.