Icemist commented on code in PR #12382:
URL: https://github.com/apache/tvm/pull/12382#discussion_r943548530
##########
include/tvm/runtime/profiling.h:
##########
@@ -573,6 +573,8 @@ PackedFunc ProfileFunction(Module mod, std::string
func_name, int device_type, i
* minimum duration requirement of one `repeat`.
* i.e., When the run time of one `repeat` falls below this time,
* the `number` parameter will be automatically increased.
+ * \param max_repeat_ms The maximum number of repeats when measured time is
equal to 0.
Review Comment:
Let's change the word "repeat" in "max_repeat_num" or change the name
completely. It looks similar but does not apply to the other arguments(repeat,
min_repeat_ms, repeats_to_cooldown). It may confuse users.
##########
src/runtime/crt/common/crt_runtime_api.c:
##########
@@ -588,6 +591,8 @@ tvm_crt_error_t RunTimeEvaluator(tvm_function_index_t
function_index, TVMValue*
if (err != kTvmErrorNoError) {
goto release_and_return;
}
+ if (std::fpclassify(curr_res_seconds) == FP_ZERO) absolute_zero_times++;
+ if (absolute_zero_times >= max_repeat_num) break;
} while (curr_res_seconds < min_repeat_seconds);
Review Comment:
It seems logical to me to keep the conditions interrupting the cycle
together, unless they are doing something specific.
mb change
```
if (absolute_zero_times >= max_repeat_num) break;
} while (curr_res_seconds < min_repeat_seconds);
```
to
`} while (curr_res_seconds < min_repeat_seconds && absolute_zero_times <
max_repeat_num);`
And do it in all 3 places.
##########
src/runtime/profiling.cc:
##########
@@ -887,6 +887,8 @@ PackedFunc WrapTimeEvaluator(PackedFunc pf, Device dev, int
number, int repeat,
}
t->Stop();
int64_t t_nanos = t->SyncAndGetElapsedNanos();
+ if (t_nanos == 0) absolute_zero_times++;
+ if (absolute_zero_times >= max_repeat_num) break;
Review Comment:
It seems to me that in the case of repeating several zero measurements, we
could try to change the "number" using a different formula: for special case
when duration_ms is 0 and absolute_zero_times > 1. Otherwise I don't see the
sense of the default value of 100 for "max_repeat_num" argument.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
