vvchernov commented on a change in pull request #9980:
URL: https://github.com/apache/tvm/pull/9980#discussion_r790524396
##########
File path: src/runtime/vm/executable.cc
##########
@@ -86,7 +86,7 @@ PackedFunc Executable::GetFunction(const std::string& name,
const ObjectPtr<Obje
} else if (name == "vm_load_executable") {
return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) {
auto vm = make_object<VirtualMachine>();
- vm->LoadExecutable(this);
+ vm->LoadExecutable(ObjectRef(sptr_to_self));
Review comment:
As I know sptr_to_self is already `ObjectPtr<Object>`, if it is
immediatly `ObjectPtr<Executable>` or can be simply converted to
`ObjectPtr<Executable>` it will be nice, but it is not so. Unfortunately your
approach is bad due to GetObjectPtr(...) creates new ObjectPtr<> object which
contains copy of raw pointer. There is no any connection between origin and new
ObjectPtr. It means that if origin object was released (e.g. when python
session where it was generated is closed) we will have pointer to broken object
and can not use it. just my PR solves this problem, but your approach does not.
##########
File path: src/runtime/vm/executable.cc
##########
@@ -86,7 +86,7 @@ PackedFunc Executable::GetFunction(const std::string& name,
const ObjectPtr<Obje
} else if (name == "vm_load_executable") {
return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) {
auto vm = make_object<VirtualMachine>();
- vm->LoadExecutable(this);
+ vm->LoadExecutable(ObjectRef(sptr_to_self));
Review comment:
As I know sptr_to_self is already `ObjectPtr<Object>`, if it is
immediatly `ObjectPtr<Executable>` or can be simply converted to
`ObjectPtr<Executable>` it will be nice, but it is not so. Unfortunately your
approach is bad due to GetObjectPtr(...) creates new ObjectPtr<> object which
contains copy of raw pointer. There is no any connection between origin and new
ObjectPtr. It means that if origin object was released (e.g. when python
session where it was generated is closed) we will have pointer to broken object
and can not use it. just my PR solves this problem, but your approach does not
as I think.
--
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: commits-unsubscr...@tvm.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
