ze: make sampling interval configurable - #525
Conversation
| @test "sampling_interval_help" { | ||
| run iprof --help | ||
|
|
||
| [ "$status" -eq 0 ] |
There was a problem hiding this comment.
We can use run !
If invoked with one of the following as the first argument, run will perform an implicit check on the exit status of the invoked command:
-N expect exit status N (0-255), fail if otherwise
! expect nonzero exit status (1-255), fail if command succeeds
(https://bats-core.readthedocs.io/en/stable/writing-tests.html#run-test-other-commands)
Lol I choose the wrong tests to do that. In the test where we do state neq 0 I mean :)
| [ "$status" -eq 0 ] | ||
| [[ "$output" == *"-i, --sample-interval MS"* ]] | ||
| [[ "$output" == *"Default: 50"* ]] | ||
| [[ "$output" == *"frequency, energy, engine, fabric-port, and memory"* ]] |
There was a problem hiding this comment.
Do we do this kind of test any where else? I think it's bad idea to put the default on the help. If we change the default, we will need to change it here too.
| @test "sampling_interval_is_passed_to_ze_sampler" { | ||
| for option in -i --sample-interval; do | ||
| trace="sampling_interval_trace_${option#-}" | ||
| rm -rf "$trace" | ||
|
|
||
| LTTNG_UST_ZE_LIBZE_LOADER=/dev/null \ | ||
| iprof --no-analysis --sample --backends ze "$option" 125 \ | ||
| --trace-output "$trace" -- \ | ||
| bash -c 'test "$LTTNG_UST_ZE_SAMPLING_ENERGY_PERIOD_MS" = 125' | ||
| done | ||
| } |
There was a problem hiding this comment.
Not sur what this is doing, we can remove it I think
| @test "sampling_heartbeat" { | ||
| rm -rf heartbeat_trace | ||
|
|
||
| LTTNG_UST_ZE_SAMPLING_ENERGY=0 LTTNG_UST_SAMPLING_HEARTBEAT=1 \ | ||
| iprof --no-analysis --sample --trace-output heartbeat_trace -- bash -c 'sleep 2' | ||
|
|
||
| babeltrace_thapi --no-restrict heartbeat_trace | grep "{foo: 16}" | ||
| [ $(babeltrace_thapi --no-restrict heartbeat_trace | grep -c "{foo: 32}") == 1 ] | ||
| } |
There was a problem hiding this comment.
We should have a test like thi. Where we sleep and change the LTTNG_UST_ZE_SAMPLING_HEARBEAT_PERIOD_MS and assert the correct count.
Maybe we can have a macro
void thapi_register_sampling_with_end("STR", default, f) {
if (getenv("LTTNG_UST_SAMPLING_{str}_MS")) {
struct timespec interval;
interval.tv_sec = 1;
interval.tv_nsec = 0;
Read_env
plugin_handle_heartbeat = thapi_register_sampling(&f, &interval);
}
And we can use that for all the sampling?
| begin | ||
| parser.parse!(into: options) | ||
| options[:'backend-names'] = options[:backends].map { |name_level| name_level.split(':').first } | ||
| if options.include?(:'sample-interval') |
There was a problem hiding this comment.
We sampling other thing like CXI should be do like name_level? sample_interval ze:121012, cxi:22323, heart_bit:1212 and if user just do --sample-interval *:1000 we apply it everywhere?
There was a problem hiding this comment.
Should do multiple mappings in the format of --sample-interval <sample>:<MS>, for example --sample-interval ze:125,cxi:250? and apply to all if --sample-interval '*:100'?
This could lead to better control over the sampling granularity
There was a problem hiding this comment.
Exactly. We do that for the backend:level already (like if you want mpi + level zero on the same table you can do --backend mpi:0,ze:0
Oh I guess we can do that for --sampling to a --sampling BACKEND:MS who enable it. So no need for another option, we just extend the current one.
Sorry , I make you do big change :D But it seem far better API wize
(and --sample versus --sampling I have no clue which one is best...)
2 participants
The Level Zero sampling is currently hardcoded every 50 ms. This change lets users select the interval at runtime through
iprof -i MSoriprof --sample-interval MS.-i MSand--sample-interval MSto theiprofCLI.--sampleand thezebackend when an interval is supplied.LTTNG_UST_ZE_SAMPLING_ENERGY_PERIOD_MS.timespecseconds/nanoseconds in the ZE sampler.