[VM] Memory Manager moved up to runtime #15833
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
srkreddy1238
changed the title
srkreddy1238
force-pushed
the
vm_mem_planner
branch
2 times, most recently
from
6562464 to
3f8b8ae
Compare
tqchen
requested changes
Sep 27, 2023
yongwww
reviewed
Sep 28, 2023
srkreddy1238
force-pushed
the
vm_mem_planner
branch
from
4dc673b to
60fa956
Compare
yongwww
reviewed
Sep 28, 2023
Member
yongwww
left a comment
yongwww
left a comment
There was a problem hiding this comment.
Thanks for the work, overall it looks good to me, have left some comments.
tqchen
approved these changes
Sep 28, 2023
echuraev
requested changes
Sep 29, 2023
Contributor
echuraev
left a comment
echuraev
left a comment
There was a problem hiding this comment.
In general LGTM. Thank you for your PR! One comment which should be applied.
srkreddy1238
force-pushed
the
vm_mem_planner
branch
2 times, most recently
from
36b9112 to
98fa479
Compare
echuraev
approved these changes
Oct 2, 2023
Now graph runtime also uses the same memory manager This acommodates a common memory manager with pooled and naive support. As a follow up we can move the WorkspacePool to use this common memory manager.
Co-authored-by: Egor Churaev <egor.churaev@gmail.com>
Using available allocator instead of requested is leading to an unpexpected crash
srkreddy1238
force-pushed
the
vm_mem_planner
branch
from
98fa479 to
b1ee28f
Compare
Member
|
This was merged, thanks for the effort! |
yongwww
pushed a commit
to yongwww/tvm
that referenced
this pull request
Oct 6, 2023
* [VM] memory Manager moved up to runtime Now graph runtime also uses the same memory manager This acommodates a common memory manager with pooled and naive support. As a follow up we can move the WorkspacePool to use this common memory manager. * * update dependents with new file addition. * * define memory_manager under new namespace * * use ShapeTuple across vm executor and memory_manager * * ShapeTuple across the Allocators * * GetDataSize is moved to DeviceAPI and memory_manager uses this interface. * * review comments * * Make compiler happy with unused variables * * lint * Update src/runtime/memory/memory_manager.cc Co-authored-by: Egor Churaev <egor.churaev@gmail.com> * * allow multiple allocators to coexist for the same device. Using available allocator instead of requested is leading to an unpexpected crash --------- Co-authored-by: Egor Churaev <egor.churaev@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now graph runtime also uses the same memory manager This acommodates a common memory manager with pooled and naive support.
As a follow up we can move the WorkspacePool to use this common memory manager.
This is a prerequisite to accommodate a common two stage memory allocation as described in #15058