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.

Sign up for GitHub

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

Jump to bottom

Remove MemoryPlan from VM passes #7361

Merged
merged 1 commit into from
Jan 29, 2021
Merged

Remove MemoryPlan from VM passes #7361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/relay/backend/vm/compiler.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -985,8 +985,11 @@ transform::Sequential MemoryOpt(tvm::Target host_target, TargetsMap targets) {
// Fuse the shape functions.
pass_seqs.push_back(transform::FuseOps());

// Perform memory planning in order to coalesce/reduce allocations.
pass_seqs.push_back(transform::MemoryPlan());
// TODO(mbrookhart, jroesch, masahi): this pass is very slow, and is
// incomplete to provide memory resuse optimizations. Disable it until we can
// rewrite it in C++ and complete it.
// // Perform memory planning in order to coalesce/reduce allocations.
// pass_seqs.push_back(transform::MemoryPlan());
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have some benchmark data from dynamic models, such as tf ssd/rcnn, to show the performance impact of disabling this pass?

Copy link
Member

Choose a reason for hiding this comment

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

Based on my experience, the performance difference with and without this pass is not evident, at least for the BERT case.

Copy link
Contributor

Choose a reason for hiding this comment

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

@icemelon9 Great to see you working on Bert. I've recently been trying to optimize bert with TVM, especially dynamic-batched case. I found that the Relay VM solution introduced a lot of small pieces of PackedFunc related to alloc_storage's size calculation, and it makes vm slower. Do you have any relevant experience or idea to share on this? Thanks. More detailed discussion: https://discuss.tvm.apache.org/t/guideline-relay-aot/5977/17?u=monklof


// Compute away constant computation introduced by coalescing allocations.
pass_seqs.push_back(transform::FoldConstant());
Expand Down
1 change: 0 additions & 1 deletion src/relay/backend/vm/lambda_lift.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ class LambdaLifter : public ExprMutator {
global = module_->GetGlobalVar(name);
} else {
// Add the lifted function to the module.
std::cout << AsText(lifted_func) << std::endl;
module_->Add(global, lifted_func);
}

Expand Down