Microprofiler can now output logs for benchmarks in the csv format. #1290

tanmaydas82 · 2022-07-30T01:43:22Z

BUG=240626703

paulinesho · 2022-08-01T15:19:19Z

tensorflow/lite/micro/micro_profiler.cc

+#endif
+}
+
+int MicroProfiler::FindExistingOrNextPosition(const char* tagName) {


Could we have some comments on what this function does?

Thank you for the suggestion Pauline! Adding a descriptive comment.

advaitjain

high level comment is that it might be worth implementing a generic Map class that is embedded friendly and then use that to perform the computation that we are doing to get to the total ticks per tag.

advaitjain · 2022-08-01T16:46:25Z

tensorflow/lite/micro/micro_profiler.h

+  // Prints the profiling information of each of the tags in CSV (Comma
+  // Separated Value) form.


We should expand this description.

Something like:

// Prints total ticks for each unique tag in CSV format. // // Output will have one row for each unique tag along with the total ticks summed across all events with that particular tag.

Done. Thanks for the suggestion!

advaitjain · 2022-08-01T16:48:15Z

tensorflow/lite/micro/micro_profiler.h

@@ -72,6 +76,14 @@ class MicroProfiler {
  uint32_t end_ticks_[kMaxEvents];
  int num_events_ = 0;

+  struct ticks_per_tag {


rename: TicksPerTag

https://google.github.io/styleguide/cppguide.html#Type_Names

Thank you for the link! Really helpful.

advaitjain · 2022-08-01T16:50:22Z

tensorflow/lite/micro/micro_profiler.h

+    const char* tag;
+    uint32_t ticks;
+  };
+  ticks_per_tag total_ticks_per_tag[kMaxEvents];


add a comment that in practice the number of unique tags will be less than the number of events but it is theoretically possible for each event to be unique and hence we allow for the TicksPerTag array to have kMaxEvents entrires.

Done. Thank you for the suggestion!

advaitjain · 2022-08-01T16:51:26Z

tensorflow/lite/micro/micro_profiler.h

+  };
+  ticks_per_tag total_ticks_per_tag[kMaxEvents];
+
+  int FindExistingOrNextPosition(const char* tagName);


nit: add a comment about what this function does.

Done. Thank you for the suggestion!

advaitjain · 2022-08-01T16:51:39Z

tensorflow/lite/micro/micro_profiler.h

+  };
+  ticks_per_tag total_ticks_per_tag[kMaxEvents];
+
+  int FindExistingOrNextPosition(const char* tagName);


tagName -> tag_name

Done. Thank you for the suggestion!

advaitjain · 2022-08-01T16:54:30Z

tensorflow/lite/micro/micro_profiler.cc

+void MicroProfiler::LogTicksPerTagCsv() {
+#if !defined(TF_LITE_STRIP_ERROR_STRINGS)
+  MicroPrintf(
+      "\"Unique Ops in the Graph\",\"Total Ticks (all instances of the Op)\"");


nit: technically may not be Ops. It could be any event that we profile with the MicroProfiler.

Unique Tag, Total ticks across all events with that tag.

advaitjain · 2022-08-01T16:54:39Z

tensorflow/lite/micro/micro_profiler.cc

+#if !defined(TF_LITE_STRIP_ERROR_STRINGS)
+  MicroPrintf(
+      "\"Unique Ops in the Graph\",\"Total Ticks (all instances of the Op)\"");
+  int totalTicks = 0;


total_ticks

Done. Thank you for the suggestion!

advaitjain · 2022-08-01T16:56:01Z

tensorflow/lite/micro/micro_profiler.cc

+    }
+    MicroPrintf("%s,%d", ticksPerTag.tag, ticksPerTag.ticks);
+  }
+  MicroPrintf("total,%d", totalTicks);


this seems to not strictly be a part of the csv format. I would say skip from the current function.

Got it. I was thinking to print the total count as well as I have seen that this is required for the calculation but if you think that is not required, I can remove it. I can probably change it to "total number of ticks" or something like that to make it more meaningful.

advaitjain · 2022-08-02T17:12:40Z

tensorflow/lite/micro/micro_profiler.cc

+#endif
+}
+
+/**


super nit: let's use // for the comment style for consistency.

https://google.github.io/styleguide/cppguide.html#Comment_Style

Thank you for the suggestion.

advaitjain · 2022-08-02T17:13:47Z

tensorflow/lite/micro/micro_profiler.cc

+      const char* currentTagName_t = each_tag_entry.tag;
+      const char* newTagName_t = tag_name;


current_tag_name
new_tag_name

advaitjain · 2022-08-02T17:15:08Z

tensorflow/lite/micro/micro_profiler.cc

+      bool matched = true;
+      while ((*currentTagName_t != '\0') && (*newTagName_t != '\0')) {
+        if (((*currentTagName_t) == (*newTagName_t))) {
+          ++currentTagName_t;
+          ++newTagName_t;
+        } else {
+          matched = false;
+          break;
+        }


Let's use strcmp instead of this custom loop.

I was not aware of if strcmp is available in tflm Thank you for the suggestion.

advaitjain · 2022-08-02T17:15:57Z

tensorflow/lite/micro/micro_profiler.h

+  // hence we allow total_ticks_per_tag to have kMaxEvents entries.
+  TicksPerTag total_ticks_per_tag[kMaxEvents];
+
+  int FindExistingOrNextPosition(const char* tagName);


tag_name: https://google.github.io/styleguide/cppguide.html#Variable_Names

Done. Missed the header file last time.

advaitjain · 2022-08-05T20:57:05Z

tensorflow/lite/micro/micro_profiler.cc

+    if (tags_[i] == nullptr) {
+      MicroPrintf("Event does not have a tag name.");
+      continue;
+    }


can this happen? seems like this should be a DCHECK and not an error that we want to print and continue on.

Added a DCHECK. Thank you for the suggestion.

advaitjain · 2022-08-05T20:58:38Z

tensorflow/lite/micro/micro_profiler.cc

+    if (position == -1) {
+      MicroPrintf("Number of tags exceeds the memory allocation.");
+      return;
+    }


again, this should not happen, right? We explicitly se the total number of unique tags to be equal to the total number of permitted events.

DCHECK seems more appropriate.

Right. I was probably overthinking. Added DCHECK.

Microprofiler can now output logs for benchmarks in the csv format.

67d57de

tanmaydas82 added the ci:run label Jul 30, 2022

github-actions bot removed the ci:run label Jul 30, 2022

Fix failing tests.

e59bceb

tanmaydas82 added the ci:run label Jul 30, 2022

github-actions bot removed the ci:run label Jul 30, 2022

Renaming some variables.

f04b0ec

tanmaydas82 added the ci:run label Jul 30, 2022

github-actions bot removed the ci:run label Jul 30, 2022

tanmaydas82 marked this pull request as ready for review July 30, 2022 08:29

tanmaydas82 requested a review from a team as a code owner July 30, 2022 08:29

tanmaydas82 added the ci:run label Jul 30, 2022

github-actions bot removed the ci:run label Jul 30, 2022

paulinesho reviewed Aug 1, 2022

View reviewed changes

advaitjain requested changes Aug 1, 2022

View reviewed changes

Incorporating all the review comments.

decb722

tanmaydas82 added the ci:run label Aug 1, 2022

github-actions bot removed the ci:run label Aug 1, 2022

advaitjain requested changes Aug 2, 2022

View reviewed changes

Incorporating review comments.

483a5a4

tanmaydas82 added the ci:run label Aug 2, 2022

github-actions bot removed the ci:run label Aug 2, 2022

tanmaydas82 added the ci:run label Aug 2, 2022

github-actions bot removed the ci:run label Aug 2, 2022

advaitjain requested changes Aug 5, 2022

View reviewed changes

Replaced the error logs with DCHECK.

09736e6

tanmaydas82 added the ci:run label Aug 5, 2022

github-actions bot removed the ci:run label Aug 5, 2022

advaitjain approved these changes Aug 5, 2022

View reviewed changes

google-ml-butler bot added the ci:ready_to_merge label Aug 5, 2022

TFLM-bot added the ci:run label Aug 5, 2022

github-actions bot removed the ci:run label Aug 5, 2022

TFLM-bot added the ci:run label Aug 5, 2022

github-actions bot removed the ci:run label Aug 5, 2022

Merge branch 'main' into csv-logger

2100f8e

TFLM-bot added the ci:run label Aug 5, 2022

github-actions bot removed the ci:run label Aug 5, 2022

mergify bot merged commit 4a8320a into tensorflow:main Aug 5, 2022

mergify bot removed the ci:ready_to_merge label Aug 5, 2022

tanmaydas82 deleted the csv-logger branch November 16, 2022 03:58

andresovela mentioned this pull request Oct 19, 2024

Profiler improvements #2724

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Microprofiler can now output logs for benchmarks in the csv format. #1290

Microprofiler can now output logs for benchmarks in the csv format. #1290

tanmaydas82 commented Jul 30, 2022

paulinesho Aug 1, 2022

tanmaydas82 Aug 1, 2022

advaitjain left a comment

advaitjain Aug 1, 2022

tanmaydas82 Aug 1, 2022

advaitjain Aug 1, 2022

tanmaydas82 Aug 1, 2022

advaitjain Aug 1, 2022

tanmaydas82 Aug 1, 2022

advaitjain Aug 1, 2022

tanmaydas82 Aug 1, 2022

advaitjain Aug 1, 2022

tanmaydas82 Aug 1, 2022

advaitjain Aug 1, 2022

advaitjain Aug 1, 2022

tanmaydas82 Aug 1, 2022

advaitjain Aug 1, 2022

tanmaydas82 Aug 1, 2022

advaitjain Aug 2, 2022

tanmaydas82 Aug 2, 2022

advaitjain Aug 2, 2022

tanmaydas82 Aug 2, 2022

advaitjain Aug 2, 2022

tanmaydas82 Aug 2, 2022

advaitjain Aug 2, 2022

tanmaydas82 Aug 2, 2022

advaitjain Aug 5, 2022

tanmaydas82 Aug 5, 2022

advaitjain Aug 5, 2022

tanmaydas82 Aug 5, 2022

		// Prints the profiling information of each of the tags in CSV (Comma
		// Separated Value) form.

+              #endif
+              }
+              /**

		const char* currentTagName_t = each_tag_entry.tag;
		const char* newTagName_t = tag_name;

Microprofiler can now output logs for benchmarks in the csv format. #1290

Microprofiler can now output logs for benchmarks in the csv format. #1290

Conversation

tanmaydas82 commented Jul 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

advaitjain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment