Add C++17 header-only wrapper for ITT API#226
Conversation
| : m_domain(domain) | ||
| , m_active(true) | ||
| { | ||
| __itt_string_handle* h = detail::create_string_handle(std::string(name).c_str()); |
There was a problem hiding this comment.
Would this not create mutex contention problem when ScopedTasks are used from multiple threads and creating handles? Maybe consider thread local cache for handles?
There was a problem hiding this comment.
Good catch, done!
It's great that you're helping us :)
There was a problem hiding this comment.
NP, I am still subscribed to the repo and my inbox is being bombarded by GitHub on every update. Since this is a header-only code the issues that may be in it will be very difficult to fix without full rebuild of customer code so it is better to exercise extra caution when making changes.
Also to explore as a further improvement idea, it may be good to have a constexpr hashing implementation for strings somewhere inbetween the cache and StringHandle so that lookups could be more effective. The strings fed to StringHandle are assumed to be constexpr anyway and can be hashed at compile-time as well.
There was a problem hiding this comment.
Thank you. That's why I tried to make this wrapper code as simple as possible to have less potential problems as possible
There was a problem hiding this comment.
Would this not create mutex contention problem when ScopedTasks are used from multiple threads and creating handles? Maybe consider thread local cache for handles?
Wow! Look who's here! :)
There was a problem hiding this comment.
I am now worried if storing just a string_view is alright due to lifetime expectation of the original string. What if the original string passed by the user is somehow a temporary object and is destroyed upon returning from the initial call?
There was a problem hiding this comment.
I don't see a problem here since the implemented cache keys always point to the ittapi string copy: cache.emplace(h->strA, h);
There was a problem hiding this comment.
I see now. Nicely done! I only hope they won't invalidate on static to dynamic lib transition
There was a problem hiding this comment.
The original (static) __itt_string_handles remain valid for the whole lifetime of the profiled process and are not freed on the transition phase
4 participants
Added a modern C++17 header-only wrapper over the ITT C API with RAII scope guards for tasks, regions, frames, and collection control.
What was added:
ScopedTask,ScopedRegion,ScopedFrame,ScopedPause— RAII classes that call the matchingend/resumeon destructionDomain,StringHandle— lightweight value types wrapping__itt_domain*and__itt_string_handle*task_begin()/task_end()manual API,__itt_idsupport for parent/child relationshipsset_thread_name(),pause()/resume()free functionsittapi::cxxINTERFACE target (opt-in via-DITT_API_CPP_SUPPORT=ON)buildall.py -cppflag,