gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735790881
##########
File path: src/runtime/rpc/rpc_module.cc
##########
@@ -164,7 +164,7 @@ class RPCModuleNode final : public ModuleNode {
~RPCModuleNode() {
if (module_handle_ != nullptr) {
try {
- sess_->FreeHandle(module_handle_, kTVMModuleHandle);
+ // sess_->FreeHandle(module_handle_, kTVMModuleHandle);
Review comment:
This isn't actually a nit :(
Uncommenting it amounts to revert
https://github.com/apache/tvm/pull/9229/commits/ce29d5b8be2941f68f2587d5ec218e988dcb11b6
This obviously can't get merged and needs a proper fix. But I don't have a
suggestion yet. As we discussed sometime ago we could flash every time before
we run a model but that seems overkill to me.
Although we've chatted a bit about potential memory leaks when running over
and over a model without resetting the device, on my experiments I never have
an issue when running multiple times with that workaround, (I let the models
run overnight, a hundred times in sequence, without resetting the device or
flashing the model again between the runs). But yeah I do agree a deeper
investigation is necessary here.
Another option I thought of is to introduce a new packet to reset the
device, but I'm not sure when exactly it would be send to the device (if in the
`RPCModuleNode` constructor or somewhere else).
Suggestions welcome :)
--
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]
