Refactor function tick code and add test

runtime/vm/profiler_service.cc

Index: runtime/vm/profiler_service.cc
diff --git a/runtime/vm/profiler_service.cc b/runtime/vm/profiler_service.cc
index 2fc71dbc196a70b309cfb59c181025cf75e4e002..34f503103ac058936144dab7baa334d4fa4707dc 100644
--- a/runtime/vm/profiler_service.cc
+++ b/runtime/vm/profiler_service.cc
@@ -117,14 +117,14 @@ const char* ProfileFunction::Name() const {
 void ProfileFunction::Tick(bool exclusive, intptr_t inclusive_serial) {
   if (exclusive) {
     exclusive_ticks_++;
-  } else {
-    if (inclusive_serial_ == inclusive_serial) {
-      // Already ticket.
-      return;
-    }
-    inclusive_serial_ = inclusive_serial;
-    inclusive_ticks_++;
   }
+  // Fall through and tick inclusive count too.
+  if (inclusive_serial_ == inclusive_serial) {
+    // Already ticket.

[rmacnak, 2015/07/28 20:35:27]

ticked

[Cutch, 2015/07/28 21:27:36]

Done.

+    return;
+  }
+  inclusive_serial_ = inclusive_serial;
+  inclusive_ticks_++;
 }
 
 
@@ -963,6 +963,7 @@ class ProfileBuilder : public ValueObject {
         null_code_(Code::ZoneHandle()),
         null_function_(Function::ZoneHandle()),
         tick_functions_(false),
+        inclusive_tree_(false),
         samples_(NULL) {
     ASSERT(profile_ != NULL);
   }
@@ -1121,7 +1122,8 @@ class ProfileBuilder : public ValueObject {
   void BuildCodeTrie(Profile::TrieKind kind) {
     ProfileCodeTrieNode* root =
         new ProfileCodeTrieNode(GetProfileCodeTagIndex(VMTag::kRootTagId));
-    if (IsInclusiveTrie(kind)) {
+    inclusive_tree_ = IsInclusiveTrie(kind);
+    if (inclusive_tree_) {
       BuildInclusiveCodeTrie(root);
     } else {
       BuildExclusiveCodeTrie(root);
@@ -1195,9 +1197,10 @@ class ProfileBuilder : public ValueObject {
         new ProfileFunctionTrieNode(
             GetProfileFunctionTagIndex(VMTag::kRootTagId));
     // We tick the functions while building the trie, but, we don't want to do
-    // it for both tries, just one.
-    tick_functions_ = IsInclusiveTrie(kind);
-    if (IsInclusiveTrie(kind)) {
+    // it for both tries, just the exclusive trie.
+    inclusive_tree_ = IsInclusiveTrie(kind);
+    tick_functions_ = !inclusive_tree_;
+    if (inclusive_tree_) {
       BuildInclusiveFunctionTrie(root);
     } else {
       BuildExclusiveFunctionTrie(root);
@@ -1222,20 +1225,12 @@ class ProfileBuilder : public ValueObject {
       // Truncated tag.
       if (sample->truncated()) {
         current = AppendTruncatedTag(current);
-        InclusiveTickTruncatedTag();
       }
 
       // Walk the sampled PCs.
       for (intptr_t j = sample->length() - 1; j >= 0; j--) {
         ASSERT(sample->At(j) != 0);
-        current = ProcessFunctionPC(
-            sample->At(j),
-            sample->timestamp(),
-            current,
-            i,
-            (j == 0),
-            sample->first_frame_executing() || sample->IsAllocationSample(),
-            true);
+        current = ProcessFrame(current, i, sample, j);
       }
     }
   }
@@ -1256,38 +1251,26 @@ class ProfileBuilder : public ValueObject {
       // Walk the sampled PCs.
       for (intptr_t j = 0; j < sample->length(); j++) {

[rmacnak, 2015/07/28 20:35:27]

j -> frame_index

[Cutch, 2015/07/28 21:27:36]

Done here and elsewhere.

         ASSERT(sample->At(j) != 0);
-        current = ProcessFunctionPC(
-            sample->At(j),
-            sample->timestamp(),
-            current,
-            i,
-            (j == 0),
-            sample->first_frame_executing() || sample->IsAllocationSample(),
-            false);
+        current = ProcessFrame(current, i, sample, j);
       }
 
       // Truncated tag.
       if (sample->truncated()) {
         current = AppendTruncatedTag(current);
+        InclusiveTickTruncatedTag();
       }
     }
   }
 
-  ProfileFunctionTrieNode* ProcessFunctionPC(
-      uword pc,
-      int64_t timestamp,
+  ProfileFunctionTrieNode* ProcessFrame(
       ProfileFunctionTrieNode* current,
-      intptr_t inclusive_serial,
-      bool top_frame,
-      bool top_frame_executing,
-      bool inclusive_tree) {
-    ProfileCode* profile_code = GetProfileCode(pc, timestamp);
+      intptr_t sample_index,
+      ProcessedSample* sample,
+      intptr_t frame_index) {
+    const uword pc = sample->At(frame_index);
+    ProfileCode* profile_code = GetProfileCode(pc,
+                                               sample->timestamp());
     ASSERT(profile_code != NULL);
-    const char* code_name = profile_code->name();
-    if (code_name == NULL) {
-      code_name = "";
-    }
-    intptr_t code_index = profile_code->code_table_index();
     const Code& code = Code::ZoneHandle(profile_code->code());
     GrowableArray<Function*> inlined_functions;
     if (!code.IsNull()) {
@@ -1298,40 +1281,35 @@ class ProfileBuilder : public ValueObject {
       // No inlined functions.
       ProfileFunction* function = profile_code->function();
       ASSERT(function != NULL);
-      current = ProcessFunction(function,
-                                current,
-                                inclusive_serial,
-                                top_frame,
-                                top_frame_executing,
-                                code_index);
+      current = ProcessFunction(current,
+                                sample_index,
+                                sample,
+                                frame_index,
+                                function);
       return current;
     }
 
-    if (inclusive_tree) {
+    if (inclusive_tree_) {
       for (intptr_t i = inlined_functions.length() - 1; i >= 0; i--) {
         Function* inlined_function = inlined_functions[i];
         ASSERT(inlined_function != NULL);
         ASSERT(!inlined_function->IsNull());
-        current = ProcessInlinedFunction(inlined_function,
-                                         current,
-                                         inclusive_serial,
-                                         top_frame,
-                                         top_frame_executing,
-                                         code_index);
-        top_frame = false;
+        current = ProcessInlinedFunction(current,
+                                         sample_index,
+                                         sample,
+                                         frame_index,
+                                         inlined_function);
       }
     } else {
       for (intptr_t i = 0; i < inlined_functions.length(); i++) {
         Function* inlined_function = inlined_functions[i];
         ASSERT(inlined_function != NULL);
         ASSERT(!inlined_function->IsNull());
-        current = ProcessInlinedFunction(inlined_function,
-                                         current,
-                                         inclusive_serial,
-                                         top_frame,
-                                         top_frame_executing,
-                                         code_index);
-        top_frame = false;
+        current = ProcessInlinedFunction(current,
+                                         sample_index,
+                                         sample,
+                                         frame_index + i,
+                                         inlined_function);
       }
     }
 
@@ -1339,44 +1317,31 @@ class ProfileBuilder : public ValueObject {
   }
 
   ProfileFunctionTrieNode* ProcessInlinedFunction(
-      Function* inlined_function,
       ProfileFunctionTrieNode* current,
-      intptr_t inclusive_serial,
-      bool top_frame,
-      bool top_frame_executing,
-      intptr_t code_index) {
+      intptr_t sample_index,
+      ProcessedSample* sample,
+      intptr_t frame_index,
+      Function* inlined_function) {
     ProfileFunctionTable* function_table = profile_->functions_;
     ProfileFunction* function = function_table->LookupOrAdd(*inlined_function);
     ASSERT(function != NULL);
-    return ProcessFunction(function,
-                           current,
-                           inclusive_serial,
-                           top_frame,
-                           top_frame_executing,
-                           code_index);
-  }
-
-  ProfileFunctionTrieNode* ProcessFunction(ProfileFunction* function,
-                                           ProfileFunctionTrieNode* current,
-                                           intptr_t inclusive_serial,
-                                           bool top_frame,
-                                           bool top_frame_executing,
-                                           intptr_t code_index) {
-    const bool exclusive = top_frame && top_frame_executing;
+    return ProcessFunction(current,
+                           sample_index,
+                           sample,
+                           frame_index,
+                           function);
+  }
+
+  ProfileFunctionTrieNode* ProcessFunction(ProfileFunctionTrieNode* current,
+                                           intptr_t sample_index,
+                                           ProcessedSample* sample,
+                                           intptr_t frame_index,
+                                           ProfileFunction* function) {
     if (tick_functions_) {
-      function->Tick(exclusive, exclusive ? -1 : inclusive_serial);
+      function->Tick(IsExecutingFrame(sample, frame_index), sample_index);
     }
-    function->AddProfileCode(code_index);
     current = current->GetChild(function->table_index());
-    current->AddCodeObjectIndex(code_index);
-    if (top_frame) {
-      if (top_frame_executing || vm_tags_emitted()) {
-        // Only tick if this function is using CPU time or VM tags are emitted.
-        current->Tick();
-      }
-    } else {
-      current->Tick();
-    }
+    current->Tick();
     return current;
   }
 
@@ -1510,7 +1475,7 @@ class ProfileBuilder : public ValueObject {
   }
 
   ProfileFunctionTrieNode* AppendVMTag(uword vm_tag,
-                                   ProfileFunctionTrieNode* current) {
+                                       ProfileFunctionTrieNode* current) {
     if (VMTag::IsNativeEntryTag(vm_tag)) {
       // Insert a dummy kNativeTagId node.
       intptr_t tag_index = GetProfileFunctionTagIndex(VMTag::kNativeTagId);
@@ -1781,6 +1746,7 @@ class ProfileBuilder : public ValueObject {
   const Code& null_code_;
   const Function& null_function_;
   bool tick_functions_;
+  bool inclusive_tree_;
 
   ProcessedSampleBuffer* samples_;
 };