rust: add more APIs to the new high-level ittapi crate
#48
Conversation
This change refactors the `ittapi` crate to add ITT APIs for domains, tasks, events, and string handles. The JIT notification code is moved to a submodule.
This requires a working installation of VTune--hence the integration test naming.
| if let Some(create_fn) = unsafe { ittapi_sys::__itt_task_begin_ptr__3_0 } { | ||
| // Currently the `taskid` and `parentid` parameters are unimplemented (TODO). | ||
| unsafe { create_fn(domain.as_ptr(), ITT_NULL, ITT_NULL, name.into().as_ptr()) }; | ||
| } |
There was a problem hiding this comment.
@ekovanova, from our previous discussions about initialization, I think this is the right thing to do here. If the static part of ittnotify has no way to actually register tasks (e.g., no collector loaded), then we just pretend to start the task. My thought was that we don't want the user to have to worry about handling this kind of error--the ITT APIs should "just work." We do this kind of trickery in a few more places (e.g., events, string handles). What do you think--does this seem correct?
There was a problem hiding this comment.
I'm agree that ITT API should just work and do nothing in case of actual collector is absent. We do not need to notify customer about dynamic library's status (absence/present) automatically - only in case of direct call to "__itt_get_collection_state" API.
This adds a `--measure vtune` option to the `benchmark` command that informs the VTune engine of the various benchmarking phases (compilation, instantiation, execution). This new measurement option does not output any data (like the `noop` measure); instead, VTune receives "task" markers that indicate when each phase starts and ends. This facilitates performance analysis by allowing us to focus in on the phase we're interested in. This change still has some WIP pieces (TODO): - the `ittapi` crate is currently a local path which should be fixed once intel/ittapi#48 is merged and a new published - the `wasmtime-bench-api` crate does not dump its JIT-compiled code, which is necessary for VTune to inspect the code in question; this functionality exists (see `ProfilingStrategy` in the [VTune docs]) but we must find a way to configure this from Sightglass [VTune docs]: https://docs.wasmtime.dev/examples-profiling-vtune.html#turn-on-vtune-support
| let task1 = Task::begin(&domain, "task#1"); | ||
| println!("Running task#1..."); | ||
| sleep(task_duration); | ||
| task1.end(); |
There was a problem hiding this comment.
if the ITT dynamic library (actual collector) is absent, the domain and the task1 will be invalid.
Correct?
Will this code work correct in case of invalid domain and task1?
Do we need additional checks to filter our the invalid situation or something?
There was a problem hiding this comment.
if the ITT dynamic library (actual collector) is absent, the domain and the task1 will be invalid. Correct?
Yes, correct.
Will this code work correct in case of invalid domain and task1? Do we need additional checks to filter our the invalid situation or something?
Right now the check that is done is "if the ITT function pointer is None, then don't call any ITT function." From your previous explanations, I thought this is the most general way to make this library never fail. But this assumption would be invalid if, e.g., the collector later wires up some of those function pointers but the user has already created an invalid domain--then, the next function call would go through with an invalid domain. Is this ever possible?
But I think this would still work, since the C side will check if the parameters are null pointers and discard the call. Am I right?
| if let Some(create_fn) = unsafe { ittapi_sys::__itt_task_begin_ptr__3_0 } { | ||
| // Currently the `taskid` and `parentid` parameters are unimplemented (TODO). | ||
| unsafe { create_fn(domain.as_ptr(), ITT_NULL, ITT_NULL, name.into().as_ptr()) }; | ||
| } |
There was a problem hiding this comment.
I'm agree that ITT API should just work and do nothing in case of actual collector is absent. We do not need to notify customer about dynamic library's status (absence/present) automatically - only in case of direct call to "__itt_get_collection_state" API.
This change refactors the code @bnjbvr added to the
ittapicrate to support a few new ITT APIs: domains, tasks, events, string handles (a full list is available here). It also improves the documentation and adds tests for the new APIs. Another addition is anintegration-testcrate that checks that a system configured with VTune will be able to actually collect data using the Rust crate.