Fix ProfileCode ticking and add unit test

- Fix wrong loop indexer (two i variables...) use when ticking ProfileCode.
- Factor out exclusive / inclusive tick decision into a helper function.
- Adjust ProfileCode::Tick's semantics.
- Add unit test verifying ProfileCode tick semantics.

R=rmacnak@google.com

Review URL: https://codereview.chromium.org//1260793003 .

runtime/vm/profiler_service.cc

@@ -268,17 +268,19 @@ void ProfileCode::GenerateAndSetSymbolName(const char* prefix) {
 
 
 void ProfileCode::Tick(uword pc, bool exclusive, intptr_t serial) {
+  // If exclusive is set, tick it.
   if (exclusive) {
     exclusive_ticks_++;
-  } else {
-    if (inclusive_serial_ == serial) {
-      // Already ticked for this sample.
-      return;
-    }
-    inclusive_serial_ = serial;
-    inclusive_ticks_++;
+    TickAddress(pc, true);
   }
-  TickAddress(pc, exclusive);
+  // Fall through and tick inclusive count too.
+  if (inclusive_serial_ == serial) {
+    // Already gave inclusive tick for this sample.
+    return;
+  }
+  inclusive_serial_ = serial;
+  inclusive_ticks_++;
+  TickAddress(pc, false);
 }
 
 
@@ -982,6 +984,12 @@ class ProfileBuilder : public ValueObject {
   }
 
  private:
+  // Returns true if |frame_index| in |sample| is using CPU.
+  static bool IsExecutingFrame(ProcessedSample* sample, intptr_t frame_index) {
+    return (frame_index == 0) && (sample->first_frame_executing() ||
+                                  sample->IsAllocationSample());
+  }
+
   static bool IsInclusiveTrie(Profile::TrieKind kind) {
     return (kind == Profile::kInclusiveFunction) ||
            (kind == Profile::kInclusiveCode);
@@ -1041,12 +1049,12 @@ class ProfileBuilder : public ValueObject {
 
       // Make sure that a ProfileCode objects exist for all pcs in the sample
       // and tick each one.
-      for (intptr_t i = 0; i < sample->length(); i++) {
-        const uword pc = sample->At(i);
+      for (intptr_t j = 0; j < sample->length(); j++) {
+        const uword pc = sample->At(j);
         ASSERT(pc != 0);
         ProfileCode* code = RegisterProfileCode(pc, timestamp);
         ASSERT(code != NULL);
-        code->Tick(pc, (i == 0), i);
+        code->Tick(pc, IsExecutingFrame(sample, j), i);
       }
     }
   }
@@ -1937,6 +1945,44 @@ const char* ProfileTrieWalker::CurrentName() {
 }
 
 
+intptr_t ProfileTrieWalker::CurrentNodeTickCount() {
+  if (current_ == NULL) {
+    return -1;
+  }
+  return current_->count();
+}
+
+
+intptr_t ProfileTrieWalker::CurrentInclusiveTicks() {
+  if (current_ == NULL) {
+    return -1;
+  }
+  if (code_trie_) {
+    ProfileCode* code = profile_->GetCode(current_->table_index());
+    return code->inclusive_ticks();
+  } else {
+    ProfileFunction* func = profile_->GetFunction(current_->table_index());
+    return func->inclusive_ticks();
+  }
+  UNREACHABLE();
+}
+
+
+intptr_t ProfileTrieWalker::CurrentExclusiveTicks() {
+  if (current_ == NULL) {
+    return -1;
+  }
+  if (code_trie_) {
+    ProfileCode* code = profile_->GetCode(current_->table_index());
+    return code->exclusive_ticks();
+  } else {
+    ProfileFunction* func = profile_->GetFunction(current_->table_index());
+    return func->exclusive_ticks();
+  }
+  UNREACHABLE();
+}
+
+
 bool ProfileTrieWalker::Down() {
   if ((current_ == NULL) || (current_->NumChildren() == 0)) {
     return false;

runtime/vm/profiler_service.h

@@ -349,6 +349,12 @@ class ProfileTrieWalker : public ValueObject {
   void Reset(Profile::TrieKind trie_kind);
 
   const char* CurrentName();
+  // Return the current node's peer's inclusive tick count.
+  intptr_t CurrentInclusiveTicks();
+  // Return the current node's peer's exclusive tick count.
+  intptr_t CurrentExclusiveTicks();
+  // Return the current node's tick count.
+  intptr_t CurrentNodeTickCount();
 
   bool Down();
   bool NextSibling();

runtime/vm/profiler_test.cc

@@ -314,6 +314,81 @@ TEST_CASE(Profiler_ToggleRecordAllocation) {
 }
 
 
+TEST_CASE(Profiler_CodeTicks) {
+  const char* kScript =
+      "class A {\n"
+      "  var a;\n"
+      "  var b;\n"
+      "}\n"
+      "class B {\n"
+      "  static boo() {\n"
+      "    return new A();\n"
+      "  }\n"
+      "}\n"
+      "main() {\n"
+      "  B.boo();\n"
+      "}\n";
+
+  Dart_Handle lib = TestCase::LoadTestScript(kScript, NULL);
+  EXPECT_VALID(lib);
+  Library& root_library = Library::Handle();
+  root_library ^= Api::UnwrapHandle(lib);
+
+  const Class& class_a = Class::Handle(GetClass(root_library, "A"));
+  EXPECT(!class_a.IsNull());
+
+  Dart_Handle result = Dart_Invoke(lib, NewString("main"), 0, NULL);
+  EXPECT_VALID(result);
+
+  {
+    Isolate* isolate = Isolate::Current();
+    StackZone zone(isolate);
+    HANDLESCOPE(isolate);
+    Profile profile(isolate);
+    AllocationFilter filter(isolate, class_a.id());
+    profile.Build(&filter, Profile::kNoTags);
+    // We should have no allocation samples.
+    EXPECT_EQ(0, profile.sample_count());
+  }
+
+  // Turn on allocation tracing for A.
+  class_a.SetTraceAllocation(true);
+
+  // Allocate three times.
+  result = Dart_Invoke(lib, NewString("main"), 0, NULL);
+  EXPECT_VALID(result);
+  result = Dart_Invoke(lib, NewString("main"), 0, NULL);
+  EXPECT_VALID(result);
+  result = Dart_Invoke(lib, NewString("main"), 0, NULL);
+  EXPECT_VALID(result);
+
+  {
+    Isolate* isolate = Isolate::Current();
+    StackZone zone(isolate);
+    HANDLESCOPE(isolate);
+    Profile profile(isolate);
+    AllocationFilter filter(isolate, class_a.id());
+    profile.Build(&filter, Profile::kNoTags);
+    // We should have one allocation sample.
+    EXPECT_EQ(3, profile.sample_count());
+    ProfileTrieWalker walker(&profile);
+
+    // Exclusive code: B.boo -> main.
+    walker.Reset(Profile::kExclusiveCode);
+    // Move down from the root.
+    EXPECT(walker.Down());
+    EXPECT_STREQ("B.boo", walker.CurrentName());
+    EXPECT_EQ(3, walker.CurrentInclusiveTicks());
+    EXPECT_EQ(3, walker.CurrentExclusiveTicks());
+    EXPECT(walker.Down());
+    EXPECT_STREQ("main", walker.CurrentName());
+    EXPECT_EQ(3, walker.CurrentInclusiveTicks());
+    EXPECT_EQ(0, walker.CurrentExclusiveTicks());
+    EXPECT(!walker.Down());
+  }
+}
+
+
 TEST_CASE(Profiler_IntrinsicAllocation) {
   const char* kScript = "double foo(double a, double b) => a + b;";
   Dart_Handle lib = TestCase::LoadTestScript(kScript, NULL);