[GitHub] [tvm] tkonolige commented on a change in pull request #7624: [Profiling,VM] Profiling interface for VM and Graph Runtime

[GitBox <gi...@apache.org>]

tkonolige commented on a change in pull request #7624:
URL: https://github.com/apache/tvm/pull/7624#discussion_r597933567



##########
File path: 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") {
+    return PackedFunc(
+        [sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { prof_ = profiling::Profiler(); });
+  } else if (name == "invoke") {
     return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) {
-      ICHECK_EQ(args.size(), 1U);
-      std::vector<std::pair<Index, double>> op_acc_time;
-      std::unordered_map<Index, std::vector<double>> op_durations;
-      for (auto kv : op_timers_) {
-        std::vector<double> durations_us;
-        for (auto t : kv.second) {
-          durations_us.push_back(t->SyncAndGetElapsedNanos() / 1e3);
+      std::vector<TVMContext> ctxs;
+      for (auto ctx : ctxs_) {
+        if (ctx.device_type > 0) {
+          ctxs.push_back(ctx);
         }
-        op_durations[kv.first] = durations_us;
-      }
-      for (auto kv : op_durations) {
-        auto val =
-            std::make_pair(kv.first, std::accumulate(kv.second.begin(), kv.second.end(), 0.0));
-        op_acc_time.push_back(val);
       }
-      bool sort_by_time = args[0];
-      if (sort_by_time) {
-        auto comp = [](const std::pair<Index, double>& lhs, const std::pair<Index, double>& rhs) {
-          return lhs.second > rhs.second;
-        };
-        std::sort(op_acc_time.begin(), op_acc_time.end(), comp);
+      auto invoke = VirtualMachine::GetFunction("invoke", sptr_to_self);
+      prof_.Start(ctxs);
+      invoke.CallPacked(args, rv);
+      prof_.Stop();
+    });
+  } else if (name == "profile") {
+    return TypedPackedFunc<String(String)>([sptr_to_self, this](String arg_name) {
+      std::vector<TVMContext> ctxs;
+      for (auto ctx : ctxs_) {
+        if (ctx.device_type > 0) {
+          ctxs.push_back(ctx);
+        }
       }
-      double total_duration = 0.0;
-      int64_t total_packed_funcs = 0;
-      std::ostringstream os;
-      os << std::setw(30) << std::left << "#OpName"
-         << "\t" << std::setw(10) << std::left << "#InvokeCount"
-         << "\t"
-         << "#Duration(us): Sum/Mean/Min/Max" << std::endl;
 
-      for (auto kv : op_acc_time) {
-        auto vals = op_durations[kv.first];
-        auto sum = kv.second;
-        auto mean = sum / static_cast<double>(vals.size());
-        auto min_value = *std::min_element(vals.begin(), vals.end());
-        auto max_value = *std::max_element(vals.begin(), vals.end());
-
-        os << std::setw(30) << std::left << packed_index_map_[kv.first] << "\t" << std::setw(10)
-           << std::left << op_invokes_[kv.first] << "\t" << sum << "/" << mean << "/" << min_value
-           << "/" << max_value << std::endl;
-
-        total_duration += sum;
-        total_packed_funcs += op_invokes_[kv.first];
+      auto invoke = VirtualMachine::GetFunction("invoke", sptr_to_self);
+      // warmup
+      for (int i = 0; i < 3; i++) {
+        invoke(arg_name);
       }
-      os << "\nTotal Duration: " << total_duration << " us.\t"
-         << "Total Packed Functions: " << total_packed_funcs << std::endl;
-      *rv = os.str();
-    });
-  } else if (name == "reset") {
-    return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) {
-      op_timers_.clear();
-      op_invokes_.clear();
+
+      prof_ = profiling::Profiler();  // reset profiler

Review comment:
       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 `reset`ing will throw an error now). Should I just get rid of the old API?




-- 
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