Skip to content

[RFC][RUNTIME] Introduce new object protocol. - #4115

Merged
zhiics merged 3 commits into
apache:masterfrom
tqchen:object
Oct 15, 2019
Merged

[RFC][RUNTIME] Introduce new object protocol.#4115
zhiics merged 3 commits into
apache:masterfrom
tqchen:object

Conversation

@tqchen

@tqchentqchen commented Oct 13, 2019

Copy link
Copy Markdown
Member

This PR introduces a new object protocol to unify the node and object.
We also updated the existing runtime::vm code to make use of the new system.

Update to the node will be done in a follow-up PR.

Other changes:

  • Remove object-related code in json serializer as that code logic was not complete
    and we have a separate serializer for VM, can revisit later.

Terminology

  • Object: the container class, like Node
  • ObjectPtr: the intrusive pointer
  • ObjectRef: the reference class

Improvement over Node and old Object System

  • Support both static and dynamic type_index(tag)
  • Optimize IsInstance check when class have final attribute
  • Make use of static type index range for sub-class checking if possible.

@tqchen

tqchen commented Oct 13, 2019

Copy link
Copy Markdown
MemberAuthor

cc @jroesch @icemelon9 @junrushao1994@zhiics@jermainewang

@tqchen
tqchenforce-pushed the object branch 3 times, most recently from 80d9573 to f8f6f50CompareOctober 13, 2019 19:15
This PR introduces a new object protocol to unify the node and object.
We also updated the existing runtime::vm code to make use of the new system.
Update to the node will be done in a follow up PR.
Other changes:
- Remove object related code in json serializer as that code logic was not complete
and we have a separate serializer for VM, can revisit later.
@tqchen

Copy link
Copy Markdown
MemberAuthor

@junrushao1994 @icemelon9 @yzh119 can you help review this PR?

@zhiicszhiics left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this protocol. It will greatly reduce developers's effort to add a new runtime object so that it can interoperate between python and c++.

So currently we have Object/ObjectRef and Node/NodeRef and we are gonna migrate Node system to Object since it's more generic, right?

Comment threadinclude/tvm/runtime/memory.h
* the type index will be assigned during runtime.
* Runtime type index can be accessed by ObjectType::type_index();
* - _type_key:
* The unique string identifier of tyep type.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: tyep

Comment threadinclude/tvm/runtime/object.h Outdated
* - Use TVM_DECLARE_FINAL_OBJECT_INFO for object classes that cannot be sub-classed.
*
* New objects can be created using make_object function.
* Which will automatically populate the type_index and deleterof the object.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/deleterof/deleter of

Comment threadinclude/tvm/runtime/object.h Outdated

protected:
// The fields of the base object cell.
/*! \brief Type index(tag) that indicate the type of the object. */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/indicate/indicates

Comment threadinclude/tvm/runtime/object.h Outdated
* When the function is first time called for a type,
* it will register the type to the type table in the runtime.
* If the static_tindex is TypeIndex::kDynamic, the function will
* will allocate a runtime type index.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/will//

Comment threadinclude/tvm/runtime/object.h Outdated
class ObjectRef {
public:
/*! \brief default constructor */
ObjectRef() = default;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ObjectRef() =default;
ObjectRef() =default;

size_t tag;
/*! \brief The fields of the structure. */
std::vector<Object> fields;
#define TVM_DEFINE_OBJECT_REF_METHODS(TypeName, ParentType, ObjectName) \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

align ""

TVM_REGISTER_OBJECT_TYPE(ObjBase);
TVM_REGISTER_OBJECT_TYPE(ObjA);
TVM_REGISTER_OBJECT_TYPE(ObjB);
TVM_REGISTER_OBJECT_TYPE(ObjAA);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TVM_DECLARE_FINAL_OBJECT_INFO? just to have a test for final object as well.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point

@tqchen

Copy link
Copy Markdown
MemberAuthor

Yes, the goal is to migrate noderef/node to be subclass of object/objectref. The migration needs to take a few steps to complete

@tqchen

Copy link
Copy Markdown
MemberAuthor

ping @zhiics for another round of review

@zhiicszhiics left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zhiics
zhiics merged commit a0bd378 into apache:masterOct 15, 2019
@zhiics

Copy link
Copy Markdown
Member

@tqchen thanks. This is now merged.

anijain2305 pushed a commit to anijain2305/tvm that referenced this pull request Oct 17, 2019
* [RUNTIME] Introduce new object protocol.
This PR introduces a new object protocol to unify the node and object.
We also updated the existing runtime::vm code to make use of the new system.
Update to the node will be done in a follow up PR.
Other changes:
- Remove object related code in json serializer as that code logic was not complete
and we have a separate serializer for VM, can revisit later.
* address review comment
* Fix the child slot logic
wweic pushed a commit to neo-ai/tvm that referenced this pull request Oct 18, 2019
* [RUNTIME] Introduce new object protocol.
This PR introduces a new object protocol to unify the node and object.
We also updated the existing runtime::vm code to make use of the new system.
Update to the node will be done in a follow up PR.
Other changes:
- Remove object related code in json serializer as that code logic was not complete
and we have a separate serializer for VM, can revisit later.
* address review comment
* Fix the child slot logic
petrex pushed a commit to petrex/tvm that referenced this pull request Oct 29, 2019
* master: (51 commits)
[QNN][TFLite] Parsing QNN Add op. Adding MobilenetV2. (apache#4142)
[CI] Pin NNPack pthreadtools version (apache#4152)
Fix typo (apache#4144)
[Relay][Frontend][TF] Add tensor array ops (apache#3798)
[relay][vm] Separate VM runtime with executable (apache#4100)
[PATCH] Fix undefined __floatdihf in libtvmruntime.so on aarch64. (apache#4119)
[DOCKER] Pin torchvision==0.4.1 (apache#4140)
[TOPI][x86] Cascade lake support. (apache#4123)
[Relay] Improve build error when no lowered funcs are produced (apache#4132)
[RUNTIME] Refactor object python FFI to new protocol. (apache#4128)
Update PULL_REQUEST_TEMPLATE.md
Adding support for dequantizing from int32 to float32. (apache#4130)
[Relay][Training] Add and fix gradients (apache#4126)
[QNN] Change default rouning to UPWARD. (apache#4131)
Fix infer type of kernel in dense. (apache#4125)
[Relay][AlterOpLayout] NHWC to NCHWc pad operator. (apache#4103)
[ARITH] Fix lowering of floormod(x, y) != 0 (apache#4127)
[RFC][RUNTIME] Introduce new object protocol. (apache#4115)
[Relay][Topi] Disable conv NHWC pack int8. (apache#4038)
Update task_cpp_unittest.sh
...
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@tqchen@zhiics