Revert "Correctly report callstack when current function is FunctionCall builtin"
authoryurys@chromium.org <yurys@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 2 Jul 2013 06:26:07 +0000 (06:26 +0000)
committeryurys@chromium.org <yurys@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 2 Jul 2013 06:26:07 +0000 (06:26 +0000)
This reverts commit r15426. The new test fails in Debug mode.

BUG=None
TBR=jkummerow@chromium.org

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15434 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/cpu-profiler-inl.h
src/cpu-profiler.cc
src/cpu-profiler.h
src/profile-generator-inl.h
src/profile-generator.cc
src/profile-generator.h
test/cctest/test-cpu-profiler.cc

index 925a28e..408d46b 100644 (file)
@@ -56,17 +56,6 @@ void SharedFunctionInfoMoveEventRecord::UpdateCodeMap(CodeMap* code_map) {
 }
 
 
-void ReportBuiltinEventRecord::UpdateCodeMap(CodeMap* code_map) {
-  CodeEntry* entry = code_map->FindEntry(start);
-  if (!entry) {
-    // Code objects for builtins should already have been added to the map.
-    UNREACHABLE();
-    return;
-  }
-  entry->SetBuiltinId(builtin_id);
-}
-
-
 TickSample* ProfilerEventsProcessor::TickSampleEvent() {
   generator_->Tick();
   TickSampleEventRecord* evt =
index be64dd7..e75e05d 100644 (file)
@@ -454,7 +454,6 @@ void CpuProfiler::StartProcessorIfNotStarted() {
     }
     logger->LogCompiledFunctions();
     logger->LogAccessorCallbacks();
-    LogBuiltins();
     // Enable stack sampling.
     Sampler* sampler = logger->sampler();
     sampler->IncreaseProfilingDepth();
@@ -515,18 +514,4 @@ void CpuProfiler::StopProcessor() {
 }
 
 
-void CpuProfiler::LogBuiltins() {
-  Builtins* builtins = isolate_->builtins();
-  ASSERT(builtins->is_initialized());
-  for (int i = 0; i < Builtins::builtin_count; i++) {
-    CodeEventsContainer evt_rec(CodeEventRecord::REPORT_BUILTIN);
-    ReportBuiltinEventRecord* rec = &evt_rec.ReportBuiltinEventRecord_;
-    Builtins::Name id = static_cast<Builtins::Name>(i);
-    rec->start = builtins->builtin(id)->address();
-    rec->builtin_id = id;
-    processor_->Enqueue(evt_rec);
-  }
-}
-
-
 } }  // namespace v8::internal
index 61d40f0..98096be 100644 (file)
@@ -49,8 +49,7 @@ class TokenEnumerator;
 #define CODE_EVENTS_TYPE_LIST(V)                                   \
   V(CODE_CREATION,    CodeCreateEventRecord)                       \
   V(CODE_MOVE,        CodeMoveEventRecord)                         \
-  V(SHARED_FUNC_MOVE, SharedFunctionInfoMoveEventRecord)           \
-  V(REPORT_BUILTIN,   ReportBuiltinEventRecord)
+  V(SHARED_FUNC_MOVE, SharedFunctionInfoMoveEventRecord)
 
 
 class CodeEventRecord {
@@ -97,15 +96,6 @@ class SharedFunctionInfoMoveEventRecord : public CodeEventRecord {
 };
 
 
-class ReportBuiltinEventRecord : public CodeEventRecord {
- public:
-  Address start;
-  Builtins::Name builtin_id;
-
-  INLINE(void UpdateCodeMap(CodeMap* code_map));
-};
-
-
 class TickSampleEventRecord {
  public:
   // The parameterless constructor is used when we dequeue data from
@@ -256,7 +246,6 @@ class CpuProfiler {
   void StopProcessorIfLastProfile(const char* title);
   void StopProcessor();
   void ResetProfiles();
-  void LogBuiltins();
 
   Isolate* isolate_;
   CpuProfilesCollection* profiles_;
index 20c1aec..cc927ad 100644 (file)
@@ -50,7 +50,6 @@ CodeEntry::CodeEntry(Logger::LogEventsAndTags tag,
                      const char* resource_name,
                      int line_number)
     : tag_(tag),
-      builtin_id_(Builtins::builtin_count),
       name_prefix_(name_prefix),
       name_(name),
       resource_name_(resource_name),
index aeb80a3..239cfba 100644 (file)
@@ -235,12 +235,6 @@ bool CodeEntry::IsSameAs(CodeEntry* entry) const {
 }
 
 
-void CodeEntry::SetBuiltinId(Builtins::Name id) {
-  tag_ = Logger::BUILTIN_TAG;
-  builtin_id_ = id;
-}
-
-
 ProfileNode* ProfileNode::FindChild(CodeEntry* entry) {
   HashMap::Entry* map_entry =
       children_.Lookup(entry, CodeEntryHash(entry), false);
@@ -854,8 +848,6 @@ const char* const ProfileGenerator::kProgramEntryName =
     "(program)";
 const char* const ProfileGenerator::kGarbageCollectorEntryName =
     "(garbage collector)";
-const char* const ProfileGenerator::kUnresolvedFunctionName =
-    "(unresolved function)";
 
 
 ProfileGenerator::ProfileGenerator(CpuProfilesCollection* profiles)
@@ -864,10 +856,7 @@ ProfileGenerator::ProfileGenerator(CpuProfilesCollection* profiles)
           profiles->NewCodeEntry(Logger::FUNCTION_TAG, kProgramEntryName)),
       gc_entry_(
           profiles->NewCodeEntry(Logger::BUILTIN_TAG,
-                                 kGarbageCollectorEntryName)),
-      unresolved_entry_(
-          profiles->NewCodeEntry(Logger::FUNCTION_TAG,
-                                 kUnresolvedFunctionName)) {
+                                 kGarbageCollectorEntryName)) {
 }
 
 
@@ -879,40 +868,33 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) {
   CodeEntry** entry = entries.start();
   memset(entry, 0, entries.length() * sizeof(*entry));
   if (sample.pc != NULL) {
+    Address start;
+    CodeEntry* pc_entry = code_map_.FindEntry(sample.pc, &start);
+    // If pc is in the function code before it set up stack frame or after the
+    // frame was destroyed SafeStackFrameIterator incorrectly thinks that
+    // ebp contains return address of the current function and skips caller's
+    // frame. Check for this case and just skip such samples.
+    if (pc_entry) {
+      List<OffsetRange>* ranges = pc_entry->no_frame_ranges();
+      if (ranges) {
+        Code* code = Code::cast(HeapObject::FromAddress(start));
+        int pc_offset = static_cast<int>(sample.pc - code->instruction_start());
+        for (int i = 0; i < ranges->length(); i++) {
+          OffsetRange& range = ranges->at(i);
+          if (range.from <= pc_offset && pc_offset < range.to) {
+            return;
+          }
+        }
+      }
+    }
+    *entry++ = pc_entry;
+
     if (sample.has_external_callback) {
       // Don't use PC when in external callback code, as it can point
       // inside callback's code, and we will erroneously report
       // that a callback calls itself.
+      *(entries.start()) = NULL;
       *entry++ = code_map_.FindEntry(sample.external_callback);
-    } else {
-      Address start;
-      CodeEntry* pc_entry = code_map_.FindEntry(sample.pc, &start);
-      // If pc is in the function code before it set up stack frame or after the
-      // frame was destroyed SafeStackFrameIterator incorrectly thinks that
-      // ebp contains return address of the current function and skips caller's
-      // frame. Check for this case and just skip such samples.
-      if (pc_entry) {
-        List<OffsetRange>* ranges = pc_entry->no_frame_ranges();
-        if (ranges) {
-          Code* code = Code::cast(HeapObject::FromAddress(start));
-          int pc_offset = static_cast<int>(
-              sample.pc - code->instruction_start());
-          for (int i = 0; i < ranges->length(); i++) {
-            OffsetRange& range = ranges->at(i);
-            if (range.from <= pc_offset && pc_offset < range.to) {
-              return;
-            }
-          }
-        }
-        *entry++ = pc_entry;
-
-        if (pc_entry->builtin_id() == Builtins::kFunctionCall) {
-          // When current function is FunctionCall builtin tos is sometimes
-          // address of the function that invoked it but sometimes it's one
-          // of the arguments. We simply replace the frame with 'unknown' entry.
-          *entry++ = unresolved_entry_;
-        }
-      }
     }
 
     for (const Address* stack_pos = sample.stack,
index 411cbdb..9d4a7e3 100644 (file)
@@ -122,9 +122,6 @@ class CodeEntry {
     no_frame_ranges_ = ranges;
   }
 
-  void SetBuiltinId(Builtins::Name id);
-  Builtins::Name builtin_id() const { return builtin_id_; }
-
   void CopyData(const CodeEntry& source);
   uint32_t GetCallUid() const;
   bool IsSameAs(CodeEntry* entry) const;
@@ -133,8 +130,7 @@ class CodeEntry {
   static const char* const kEmptyResourceName;
 
  private:
-  Logger::LogEventsAndTags tag_ : 8;
-  Builtins::Name builtin_id_ : 8;
+  Logger::LogEventsAndTags tag_;
   const char* name_prefix_;
   const char* name_;
   const char* resource_name_;
@@ -429,9 +425,6 @@ class ProfileGenerator {
   static const char* const kAnonymousFunctionName;
   static const char* const kProgramEntryName;
   static const char* const kGarbageCollectorEntryName;
-  // Used to represent frames for which we have no reliable way to
-  // detect function.
-  static const char* const kUnresolvedFunctionName;
 
  private:
   INLINE(CodeEntry* EntryForVMState(StateTag tag));
@@ -440,7 +433,6 @@ class ProfileGenerator {
   CodeMap code_map_;
   CodeEntry* program_entry_;
   CodeEntry* gc_entry_;
-  CodeEntry* unresolved_entry_;
   SampleRateCalculator sample_rate_calc_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileGenerator);
index f4d9936..251a4a5 100644 (file)
@@ -1023,80 +1023,3 @@ TEST(BoundFunctionCall) {
 
   cpu_profiler->DeleteAllCpuProfiles();
 }
-
-
-static const char* call_function_test_source = "function bar(iterations) {\n"
-"}\n"
-"function start(duration) {\n"
-"  var start = Date.now();\n"
-"  while (Date.now() - start < duration) {\n"
-"    try {\n"
-"      bar.call(this, 10 * 1000);\n"
-"    } catch(e) {}\n"
-"  }\n"
-"}";
-
-
-// Test that if we sampled thread when it was inside FunctionCall buitin then
-// its caller frame will be '(unresolved function)' as we have no reliable way
-// to resolve it.
-//
-// [Top down]:
-//    96     0   (root) [-1] #1
-//     1     1    (garbage collector) [-1] #4
-//     5     0    (unresolved function) [-1] #5
-//     5     5      call [-1] #6
-//    71    70    start [-1] #3
-//     1     1      bar [-1] #7
-//    19    19    (program) [-1] #2
-TEST(FunctionCallSample) {
-  LocalContext env;
-  v8::HandleScope scope(env->GetIsolate());
-
-  v8::Script::Compile(v8::String::New(call_function_test_source))->Run();
-  v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(
-      env->Global()->Get(v8::String::New("start")));
-
-  v8::CpuProfiler* cpu_profiler = env->GetIsolate()->GetCpuProfiler();
-  v8::Local<v8::String> profile_name = v8::String::New("my_profile");
-
-  cpu_profiler->StartCpuProfiling(profile_name);
-  int32_t duration_ms = 100;
-  v8::Handle<v8::Value> args[] = { v8::Integer::New(duration_ms) };
-  function->Call(env->Global(), ARRAY_SIZE(args), args);
-  const v8::CpuProfile* profile = cpu_profiler->StopCpuProfiling(profile_name);
-
-  CHECK_NE(NULL, profile);
-  // Dump collected profile to have a better diagnostic in case of failure.
-  reinterpret_cast<i::CpuProfile*>(
-      const_cast<v8::CpuProfile*>(profile))->Print();
-
-  const v8::CpuProfileNode* root = profile->GetTopDownRoot();
-  {
-    ScopedVector<v8::Handle<v8::String> > names(4);
-    names[0] = v8::String::New(ProfileGenerator::kGarbageCollectorEntryName);
-    names[1] = v8::String::New(ProfileGenerator::kProgramEntryName);
-    names[2] = v8::String::New("start");
-    names[3] = v8::String::New(i::ProfileGenerator::kUnresolvedFunctionName);
-    // Don't allow |bar| and |call| nodes to be at the top level.
-    CheckChildrenNames(root, names);
-  }
-
-  const v8::CpuProfileNode* startNode = GetChild(root, "start");
-  {
-    ScopedVector<v8::Handle<v8::String> > names(1);
-    names[0] = v8::String::New("bar");
-    CheckChildrenNames(startNode, names);
-  }
-
-  const v8::CpuProfileNode* unresolvedNode =
-      FindChild(root, i::ProfileGenerator::kUnresolvedFunctionName);
-  if (unresolvedNode) {
-    ScopedVector<v8::Handle<v8::String> > names(1);
-    names[0] = v8::String::New("call");
-    CheckChildrenNames(unresolvedNode, names);
-  }
-
-  cpu_profiler->DeleteAllCpuProfiles();
-}
-