[Profiling,VM] Profiling interface for VM and Graph Runtime #7624

tkonolige · 2021-03-10T00:03:08Z

This PR introduces a unified interface for profiling between the graph runtime and VM. It replaces the VM profiler, but adds a separate function to the graph runtime (because the debug graph runtime has more features than just profiling).

Right now profiling reports are just textual, but there should be a straight forward route from the PR to generating structured reports if users want them.

@jroesch @junrushao1994 @comaniac

tqchen · 2021-03-10T00:06:12Z

Thanks @tkonolige , it would be good to think about introducing src/runtime/profiler and have options to optionally turn off the profiler for resource constrained cases

tkonolige · 2021-03-10T00:12:00Z

I talked to @areusch and he said it wouldn't have any effect on embed environments. Furthermore, it should be dead code eliminated if neither the debug graph runtime or VM profiler is enabled. Also, the amount of code introduced here is minimal.

tqchen · 2021-03-10T00:15:46Z

Because of the nature of the shared library (things get linked together) the code won't be dead code eliminated, having an explicit option is still desirable. We are also still using the c++ runtime for android iOS and raspberry pi. Having clear isolation could be a net positive.

tkonolige · 2021-03-10T00:37:02Z

There are already a two flags to enable/disable profiling. Adding a third is even more confusing, so I'm in favor for leaving this as is. Also, the code size here is minimal. On my machine, the code size increase is 80216 bytes.

tqchen · 2021-03-11T18:16:58Z

can we unify the profiling flag into a single one perhaps? Since in the most time we just need an option to switch all profiling off.

tkonolige · 2021-03-16T16:32:55Z

@siju-samuel Do you think you could review this?

tqchen · 2021-03-18T22:04:20Z

cc @trevor-m @icemelon9 @yzhliu would be great if you can take a look as well

include/tvm/runtime/profiling.h

icemelon · 2021-03-19T00:53:18Z

include/tvm/runtime/profiling.h

+class DurationNode : public Object {
+ public:
+  /* The duration as a floating point number of microseconds. */
+  double value;


Suggest changing the name to microseconds to be more specific

src/runtime/profiling.cc

icemelon · 2021-03-19T00:58:26Z

src/runtime/vm/profiler/vm.cc

-      op_timers_.clear();
-      op_invokes_.clear();
+
+      prof_ = profiling::Profiler();  // reset profiler


Since we create the profiler each time, we should just remove it from the class field.

It is actually initialized each time we call reset. The old API is to call invoke and then get_stat to return the profiling information. I wanted to still support old api (though calling invoke twice without reseting will throw an error now). Should I just get rid of the old API?

icemelon · 2021-03-19T00:58:37Z

src/runtime/vm/profiler/vm.cc

@@ -42,59 +42,44 @@ namespace vm {
 PackedFunc VirtualMachineDebug::GetFunction(const std::string& name,
                                            const ObjectPtr<Object>& sptr_to_self) {
  if (name == "get_stat") {
+    return PackedFunc(
+        [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = prof_.Report(); });
+  } else if (name == "reset") {


no need for reset.

icemelon · 2021-03-19T00:58:48Z

src/runtime/vm/profiler/vm.h

@@ -52,8 +52,7 @@ class VirtualMachineDebug : public VirtualMachine {
                    const std::vector<ObjectRef>& args) final;

  std::unordered_map<Index, std::string> packed_index_map_;
-  std::unordered_map<Index, std::vector<Timer>> op_timers_;
-  std::unordered_map<Index, int> op_invokes_;
+  profiling::Profiler prof_;


we can remove it

tkonolige · 2021-03-25T17:22:52Z

@icemelon9 Could you re-review?

…e frame sorting optional.

jwfromm · 2021-03-31T17:20:09Z

@icemelon9 just wanted to ping you on this again, its come a long way since you last reviewed it.

icemelon · 2021-03-31T17:26:06Z

Overall LGTM. You can just remove the reset API in the profiler VM.

tkonolige · 2021-03-31T18:05:00Z

I've removed the reset api. The profiler still needs to be a field because it is used in multiple member functions.

icemelon

LGTM

icemelon · 2021-03-31T21:04:15Z

Could you fix the CI? @tkonolige

icemelon · 2021-04-01T16:34:44Z

Thanks @tkonolige

) * [Profiling,VM] Profiling interface for VM and Graph Runtime * lint * fix test * make profiling test actually run * Try to better match the graph runtime function names to vm * formatting * DurationNode.value -> microseconds; PercentNode.value -> percent; make frame sorting optional. * renaming for the tvmcontext -> device change * formatting * remove old vm profiler get_stat api * fix tests

tkonolige mentioned this pull request Mar 11, 2021

[RUNTIME] Switch time evaluator to use device specific timing. #7631

Merged

tkonolige mentioned this pull request Mar 11, 2021

[PROFILING] Combine USE_VM_PROFILER and USE_GRAPH_RUNTIME_DEBUG into a single flag USE_PROFILER #7637

Merged

tqchen added the status: need review label Mar 18, 2021

tqchen assigned icemelon Mar 18, 2021

icemelon reviewed Mar 19, 2021

View reviewed changes

tkonolige added 8 commits March 30, 2021 15:46

[Profiling,VM] Profiling interface for VM and Graph Runtime

fde4b96

lint

fa9d8b9

fix test

050f7f6

make profiling test actually run

aae53a8

Try to better match the graph runtime function names to vm

fb887e4

formatting

bfc8e26

DurationNode.value -> microseconds; PercentNode.value -> percent; mak…

1d8c1c7

…e frame sorting optional.

renaming for the tvmcontext -> device change

54a9e45

tkonolige force-pushed the profiler branch from df6de94 to 54a9e45 Compare March 30, 2021 22:54

formatting

10bd10d

remove old vm profiler get_stat api

96c96c2

icemelon approved these changes Mar 31, 2021

View reviewed changes

fix tests

8894729

icemelon merged commit b95803f into apache:main Apr 1, 2021

icemelon added status: accepted and removed status: need review labels Apr 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Profiling,VM] Profiling interface for VM and Graph Runtime #7624

[Profiling,VM] Profiling interface for VM and Graph Runtime #7624

tkonolige commented Mar 10, 2021

tqchen commented Mar 10, 2021

tkonolige commented Mar 10, 2021

tqchen commented Mar 10, 2021

tkonolige commented Mar 10, 2021

tqchen commented Mar 11, 2021 •

edited

Loading

tkonolige commented Mar 16, 2021

tqchen commented Mar 18, 2021

icemelon Mar 19, 2021

icemelon Mar 19, 2021

tkonolige Mar 19, 2021

icemelon Mar 19, 2021

icemelon Mar 19, 2021

tkonolige commented Mar 25, 2021

jwfromm commented Mar 31, 2021

icemelon commented Mar 31, 2021

tkonolige commented Mar 31, 2021

icemelon left a comment

icemelon commented Mar 31, 2021

icemelon commented Apr 1, 2021

[Profiling,VM] Profiling interface for VM and Graph Runtime #7624

[Profiling,VM] Profiling interface for VM and Graph Runtime #7624

Conversation

tkonolige commented Mar 10, 2021

tqchen commented Mar 10, 2021

tkonolige commented Mar 10, 2021

tqchen commented Mar 10, 2021

tkonolige commented Mar 10, 2021

tqchen commented Mar 11, 2021 • edited Loading

tkonolige commented Mar 16, 2021

tqchen commented Mar 18, 2021

icemelon Mar 19, 2021

Choose a reason for hiding this comment

icemelon Mar 19, 2021

Choose a reason for hiding this comment

tkonolige Mar 19, 2021

Choose a reason for hiding this comment

icemelon Mar 19, 2021

Choose a reason for hiding this comment

icemelon Mar 19, 2021

Choose a reason for hiding this comment

tkonolige commented Mar 25, 2021

jwfromm commented Mar 31, 2021

icemelon commented Mar 31, 2021

tkonolige commented Mar 31, 2021

icemelon left a comment

Choose a reason for hiding this comment

icemelon commented Mar 31, 2021

icemelon commented Apr 1, 2021

tqchen commented Mar 11, 2021 •

edited

Loading