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

Counter nanoseconds do not correspond to Vulkan pipeline statistics #40

Closed
chaoticbob opened this issue Apr 28, 2019 · 4 comments
Closed

Comments

@chaoticbob
Copy link

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
Copy link
Collaborator

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
Copy link
Author

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 added a commit that referenced this issue May 1, 2019
The code was misinterpreting the limits.timestampPeriod member of
VkPhysicalDeviceProperties.

This addresses the issue reported here:
#40
@chesik-amd
Copy link
Collaborator

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

@chaoticbob
Copy link
Author

Thank you!

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

No branches or pull requests

2 participants