Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[FFI][RUNTIME] Introduce runtime boxed types for int/float/bool" #17252

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

tqchen
Copy link
Member

@tqchen tqchen commented Aug 7, 2024

Reverts #16183

see #16183 (comment) for context

Copy link
Contributor

@Lunderberg Lunderberg left a comment

Choose a reason for hiding this comment

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

Revert approved, and crossing fingers that resolving the performance overhead won't be too onerous.

@tqchen tqchen merged commit 11be832 into main Aug 7, 2024
31 checks passed
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Aug 8, 2024
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Aug 8, 2024
Initially introduced in apache#16183,
these changes were reverted in
apache#17252 due to performance
degredation in some Relax models.  This could occur when a model
contained a large number of calls to `"vm.builtin.tuple_getitem"`,
which may occur when model weights are provided as a tuple.

This PR re-applies the changes from
apache#16183, but with the performance
degredation resolved.  The root cause was unnecessary type-checking
when converting from an untyped `tvm::ArrayNode*` to the typed
`tvm::Array<T>`, in the case where `T` is `ObjectRef`.
tqchen pushed a commit that referenced this pull request Aug 12, 2024
* Revert "Revert "[FFI][RUNTIME] Introduce runtime boxed types for int/float/bool" (#17252)"

This reverts commit 11be832.

* [FFI] Re-introduce the boxed primitive values

Initially introduced in #16183,
these changes were reverted in
#17252 due to performance
degredation in some Relax models.  This could occur when a model
contained a large number of calls to `"vm.builtin.tuple_getitem"`,
which may occur when model weights are provided as a tuple.

This PR re-applies the changes from
#16183, but with the performance
degredation resolved.  The root cause was unnecessary type-checking
when converting from an untyped `tvm::ArrayNode*` to the typed
`tvm::Array<T>`, in the case where `T` is `ObjectRef`.

* Correct typo from T to U
@tqchen tqchen deleted the revert-16183-ffi_boxed_primitives_for_runtime branch September 5, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants