You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/02/22 08:52:27 UTC
[GitHub] [tvm] monklof opened a new pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
monklof opened a new pull request #7495:
URL: https://github.com/apache/tvm/pull/7495
cc @wweic @zhiics
VM Profiler only does PackedFunc-level profiling for now, this pr adds instruction-level profiling capability.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] monklof commented on a change in pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
monklof commented on a change in pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#discussion_r582578220
##########
File path: src/runtime/vm/vm.cc
##########
@@ -360,11 +360,34 @@ void VirtualMachine::RunLoop() {
ICHECK(this->code_);
pc_ = 0;
Index frame_start = frames_.size();
+
+#ifdef USE_VM_PROFILER
+ bool started = false;
+ Opcode last_inst;
+ auto inst_begin = std::chrono::high_resolution_clock::now();
+ auto inst_end = inst_begin;
+#endif
+
while (true) {
main_loop:
auto const& instr = code_[this->pc_];
DLOG(INFO) << "Executing(" << pc_ << "): " << instr;
+#ifdef USE_VM_PROFILER
+ if (started) {
+ inst_end = std::chrono::high_resolution_clock::now();
+ double inst_duration =
+ std::chrono::duration_cast<std::chrono::duration<double>>(inst_end - inst_begin)\
+ .count() * 1e6;
Review comment:
Thanks, updated
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] monklof commented on a change in pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
monklof commented on a change in pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#discussion_r582576100
##########
File path: src/runtime/vm/vm.cc
##########
@@ -567,7 +590,7 @@ void VirtualMachine::RunLoop() {
auto caller_return_register = frames_.back().caller_return_register;
if (PopFrame() == frame_start) {
- return;
+ goto vm_completed;
Review comment:
Makes sense, I'll update it later.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] FrozenGene commented on a change in pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#discussion_r580182673
##########
File path: include/tvm/runtime/vm/bytecode.h
##########
@@ -26,6 +26,7 @@
#include <tvm/runtime/data_type.h>
#include <tvm/support/logging.h>
+#include <string>
Review comment:
Consider using `tvm/runtime/container.h`'s `class String`, as in the runtime we want to use our TVM's object system like other runtime's components.
##########
File path: include/tvm/runtime/vm/bytecode.h
##########
@@ -392,6 +393,8 @@ struct Instruction {
friend std::ostream& operator<<(std::ostream& os, const Instruction&);
};
+std::string InstructionStrRepr(const Opcode op);
Review comment:
ditto
##########
File path: src/runtime/vm/bytecode.cc
##########
@@ -632,6 +632,77 @@ void InstructionPrint(std::ostream& os, const Instruction& instr) {
}
}
+std::string InstructionStrRepr(const Opcode op) {
Review comment:
ditto
##########
File path: src/runtime/vm/vm.cc
##########
@@ -567,7 +590,7 @@ void VirtualMachine::RunLoop() {
auto caller_return_register = frames_.back().caller_return_register;
if (PopFrame() == frame_start) {
- return;
+ goto vm_completed;
Review comment:
i think you could just simply move the sentence of `vm_completed` here and could avoid `goto`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#issuecomment-784326126
I tried the separate instruction approach. It works ok, but there are some challenges around the timing of Invoke and InvokeClosure. I ultimately dropped the separate instruction approach because if you want to profiling everything, you have to change all the instructions or insert a ton on profiling instructions. I think the approach in this PR is fine. However, I will point out that this PR does not time Invoke and InvokeClosure correctly. It just times how long it takes to start the function call, not the total time spent in the call.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] FrozenGene commented on pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#issuecomment-783310773
Thanks @monklof Personally, I don't like we have to turn on one option and rebuild TVM if we want to profile (this situation is the same as DebugGraphRuntime). Like TRT, we have function called as `reportLayerTime` to do it but we don't have to use one version like Debug TRT. (see https://github.com/NVIDIA/TensorRT/blob/master/samples/opensource/sampleNMT/sampleNMT.cpp#L1127).
Of course, we don't have to create one class named as Profiler in TRT to do this. But whether we could simplify our usage of profiling? I think we could consider this VM and GraphRuntime together (Do we need DebugGraphRuntime and have to rebuild TVM when we want to profile?). cc @comaniac @tqchen @jcf94 want to listen more voices.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] monklof commented on pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
monklof commented on pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#issuecomment-785661470
@tkonolige Thanks, you are right, it does not time the total execution of Invoke and InvokeClosure, I only implemented the instruction-level profiling, function-level is not included.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] monklof commented on a change in pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
monklof commented on a change in pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#discussion_r582574176
##########
File path: include/tvm/runtime/vm/bytecode.h
##########
@@ -26,6 +26,7 @@
#include <tvm/runtime/data_type.h>
#include <tvm/support/logging.h>
+#include <string>
Review comment:
I think it's just fine since the result of InstructionStrRepr() is not a part of runtime's state, it's just used for profiling, and we don't need the object-system. I'm new to tvm, please correct me if my understanding to the object-system is wrong.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on a change in pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#discussion_r580414040
##########
File path: src/runtime/vm/vm.cc
##########
@@ -360,11 +360,34 @@ void VirtualMachine::RunLoop() {
ICHECK(this->code_);
pc_ = 0;
Index frame_start = frames_.size();
+
+#ifdef USE_VM_PROFILER
+ bool started = false;
+ Opcode last_inst;
+ auto inst_begin = std::chrono::high_resolution_clock::now();
+ auto inst_end = inst_begin;
+#endif
+
while (true) {
main_loop:
auto const& instr = code_[this->pc_];
DLOG(INFO) << "Executing(" << pc_ << "): " << instr;
+#ifdef USE_VM_PROFILER
+ if (started) {
+ inst_end = std::chrono::high_resolution_clock::now();
+ double inst_duration =
+ std::chrono::duration_cast<std::chrono::duration<double>>(inst_end - inst_begin)\
+ .count() * 1e6;
Review comment:
```suggestion
std::chrono::duration<double, std::micro>(inst_end - inst_begin)\
.count();
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] comaniac commented on pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#issuecomment-783545512
Also cc @icemelon9 @zhiics
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jroesch closed pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
jroesch closed pull request #7495:
URL: https://github.com/apache/tvm/pull/7495
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] monklof commented on a change in pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
monklof commented on a change in pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#discussion_r582574176
##########
File path: include/tvm/runtime/vm/bytecode.h
##########
@@ -26,6 +26,7 @@
#include <tvm/runtime/data_type.h>
#include <tvm/support/logging.h>
+#include <string>
Review comment:
I think it's just fine since the result of InstructionStrRepr() is not a part of runtime's state, it's just used for profiling.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jroesch commented on pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#issuecomment-907395484
I am doing triage on old PRs, going to close this, please feel free to follow up if you would like to still merge these changes. Thanks for your contributions!
It looks like this has been subsumed by new profiling work, again feel free to reopen if I appear in the wrong.
--
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-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#issuecomment-783520576
@FrozenGene I've been working on profiling for the VM. It does seems like we could merge the profiling infrastructure for the VM and the graph runtime. However, the profilers for the VM and the graph runtime do act differently. The graph runtime does statistical profiling where it runs every op multiple times. The VM only runs each op once.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] zhiics commented on pull request #7495: [Relay VM] VM Profiler Supports Profiling Instruction Stats
Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #7495:
URL: https://github.com/apache/tvm/pull/7495#issuecomment-783823031
I had a similar implementation long time ago but never upstreamed it because I think having a separate instruction might work better. @jroesch @tkonolige could you guys share some thoughts/updates on the profiling that you have been working on?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org