GPUOpen-Tools / gpu_performance_api

GPU Performance API for AMD GPUs
MIT License
250 stars 46 forks source link

Counter nanoseconds do not correspond to Vulkan pipeline statistics #40

Closed chaoticbob closed 5 years ago

chaoticbob commented 5 years ago

The documentation and the code both state "nanoseconds" for various derived counters. This doesn't seem to be correct when compared to Vulkan's pipeline statistics. A sample profiling primitive assembly shows the following with GPA: GPUTime = 2489.508102 ExecutionDuration = 2489.076102 ExecutionStart = 6431344343.079865 ExecutionEnd = 6431349323.608068

The exact same command buffer shows: GPU Time = 3.411704 (ms) GPU Start Time = 8822159.163976 (ms) GPU End Time = 8822162.575679 (ms) This is worked out by multiplying the timestamp query with the device's timestampPeriod and then converting nanoseconds to milliseconds.

Using start/end time as hint, the time unit that makes the most sense for GPA's values is possibly microseconds.

Am I somehow misinterpreting this?

chesik-amd commented 5 years ago

Looks like there is some incorrect math in the code that converts Vulkan's timestampPeriod to the clock frequency. See https://github.com/GPUOpen-Tools/GPA/blob/master/Src/GPUPerfAPIVk/VkUtils.cpp#L231

We believe this is the cause of the issue you're seeing.

Can you try modifying the code to the following:

        // Vulkan's timestampPeriod is expressed in nanoseconds per clock tick, convert to frequency in seconds
        float timestampPeriod = properties.limits.timestampPeriod;
        timestampFrequency    = static_cast<gpa_uint64>(1000000000.0f / (timestampPeriod));

We are currently testing this change, and if you agree it looks good, we will push this change to this repo.

Thanks, Chris

cc: @amitprakash07

chaoticbob commented 5 years ago

Thank you for looking into this so quickly. I just verified it against the tests that I have and it looks to line up the times that I'm seeing out of the Vulkan pipeline statistics.

chesik-amd commented 5 years ago

Thanks for confirming and for reporting this issue. I have pushed the fix.

chaoticbob commented 5 years ago

Thank you!