tqchen commented on a change in pull request #7919:
URL: https://github.com/apache/tvm/pull/7919#discussion_r624536898
##########
File path: include/tvm/runtime/registry.h
##########
@@ -52,6 +52,45 @@
namespace tvm {
namespace runtime {
+/*!
+ * \brief Check if signals have been sent to the process and if so
+ * invoke the registered signal handler in the frontend environment.
+ *
+ * When runnning TVM in another langugage(python), the signal handler
+ * may not be immediately executed, but instead the signal is marked
+ * in the interpreter state(to ensure non-blocking of the signal handler).
+ *
+ * This function can be explicitly invoked to check the cached signal
+ * and run the related processing if a signal is marked.
+ *
+ * Invoke this function periodically in a long running C++ function
+ * to check if KeyboardInterrupt happens in a python execution environment.
+ *
+ * Not inserting this function will not cause any correctness
+ * issue, but will delay the KeyboardInterrupt until the function returns
+ * to the python side. So this function is not needed in most API
+ * functions can finish quickly in a reasonable amount of time.
+ *
+ * \code
+ *
+ * int check_signal_every_kiter = 10;
+ *
+ * for (int iter = 0; iter < very_large_number; ++iter) {
+ * if (iter % check_signal_every_kiter == 0) {
+ * tvm::runtime::EnvCheckSignals();
+ * }
+ * // do work here
+ * }
+ *
+ * \endcode
+ *
+ * \note This function is a nop when no signal checking function is registered.
+ *
+ * \throws This function throws approperiate exception if an error happens,
Review comment:
The main complexity of pushing the handling to FFI layer is that we will
need to introduce specific compiled functions in the FFI layer(via cython and
others) complicates the implementation(we cannot use python to implement it
because the handling have to go through the C API, which is doable but
introduces complication to implement such function in cython without any
support from the python land).
With additional effort we might be able to recover the exact exception
raised. Given KeyboardInterrupt would be the most common one. We could change
that to a generic exception indicating that an error happens during signal
handling.
I have also tried to propagate the exact error to the python side would be
harder because it involves complications in the python calling boundary(we
cannot do so in the python land and it have to be done in a C++ land). Directly
normal return could raise an SystemError that also shows the original error
message.
Finally, back to the generic function. There are two ways to think about the
handling. Given the only usecase so far is python I trid to avoid generalizing
and centralizes the implementation in the runtime(to reduce indirection). We
can refactor once we see more needs and given there is no API change such
refactor should be easy
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
