Skip to content

Conversation

@DeadSonger
Copy link

I use Kineto as a tracer connected to applications via injection. By default, a time converter is used that does not correspond to the real time on the execution machine, and can also have a multiple accelerated time flow (for example, the time indicator in cupti can grow twice as fast as the time indicator on the executable machine). To solve this problem, this patch has been prepared, which corrects the previously indicated inaccuracies. As a result, the corrected version allows you to get after conversion the time matched with the real one with a small but constant offset (the offset is the same for all events)

@sraikund16
Copy link
Contributor

sraikund16 commented Mar 26, 2025

Seems reasonable to me. Typically we do this conversion in PyTorch profiler and then we set the kineto converter there. See: https://github.com/pytorch/pytorch/blob/3a8171efadd54c79453451156cf982f45d36ed66/c10/util/ApproximateClock.cpp#L41

If you are only using Kineto you will need this change but I am wondering if it would be more accurate to use a sampling approach like the one I linked above.

@facebook-github-bot
Copy link
Contributor

@sraikund16 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@sraikund16
Copy link
Contributor

Hi, so it seems like this change is causing several tests to fail. We can fix them but I was wondering if instead of changing the default within kineto you can change it using the get_time_converter call like we do in pytorch?

https://github.com/pytorch/pytorch/blob/dfcd98e684123b0cb0a143d8718b0672c58ec268/torch/csrc/autograd/profiler_kineto.cpp#L418

@DeadSonger
Copy link
Author

DeadSonger commented Apr 2, 2025

Hi, we compile KINETO as shared library and start it using CUDA_INJECTION64_PATH, there is no way to change converter in runtime in this case
May be we can select time converter using environment variable? for example KINETO_CUPTI_TIME_CONVERTER=[SIMPLE, SCALE]
And by default use current realisation of time converter

@sraikund16
Copy link
Contributor

That works for me! Feel free to update with env var instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants