Implemented "no heap access" mode for JSFrame which is used for stack sampling in...

As I discovered that JSFrame accesses SharedFunctionInfo only to calculate caller SP and the latter is not used in profiler's stack sampling, I disabled accessing heap objects in JSFrame when doing stack sampling. This finally made V8's profiling stable when used from Chrome on a real web app.

Review URL: http://codereview.chromium.org/73020

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

diff --git a/src/frames-arm.cc b/src/frames-arm.cc
index fe850a8..121fb75 100644 (file)
--- a/src/frames-arm.cc
+++ b/src/frames-arm.cc
@@ -80,7 +80,7 @@ int JavaScriptFrame::GetProvidedParametersCount() const {
 
 Address JavaScriptFrame::GetCallerStackPointer() const {
   int arguments;
-  if (Heap::gc_state() != Heap::NOT_IN_GC) {
+  if (Heap::gc_state() != Heap::NOT_IN_GC || disable_heap_access_) {
     // The arguments for cooked frames are traversed as if they were
     // expression stack elements of the calling frame. The reason for
     // this rather strange decision is that we cannot access the

diff --git a/src/frames-ia32.cc b/src/frames-ia32.cc
index 2b24777..1bc62ec 100644 (file)
--- a/src/frames-ia32.cc
+++ b/src/frames-ia32.cc
@@ -78,7 +78,7 @@ int JavaScriptFrame::GetProvidedParametersCount() const {
 
 Address JavaScriptFrame::GetCallerStackPointer() const {
   int arguments;
-  if (Heap::gc_state() != Heap::NOT_IN_GC) {
+  if (Heap::gc_state() != Heap::NOT_IN_GC || disable_heap_access_) {
     // The arguments for cooked frames are traversed as if they were
     // expression stack elements of the calling frame. The reason for
     // this rather strange decision is that we cannot access the

diff --git a/src/frames-inl.h b/src/frames-inl.h
index 07c8e4e..cb03e2f 100644 (file)
--- a/src/frames-inl.h
+++ b/src/frames-inl.h
@@ -169,19 +169,6 @@ inline bool JavaScriptFrame::has_adapted_arguments() const {
 }
 
 
-inline bool JavaScriptFrame::is_at_function() const {
-  Object* result = function_slot_object();
-  // Verify that frame points at correct JS function object.
-  // We are verifying that function object address and
-  // the underlying map object address are valid, and that
-  // function is really a function.
-  return Heap::Contains(reinterpret_cast<Address>(result)) &&
-      result->IsHeapObject() &&
-      Heap::Contains(HeapObject::cast(result)->map()) &&
-      result->IsJSFunction();
-}
-
-
 inline Object* JavaScriptFrame::function() const {
   Object* result = function_slot_object();
   ASSERT(result->IsJSFunction());

diff --git a/src/frames.cc b/src/frames.cc
index a9bbbed..88c723d 100644 (file)
--- a/src/frames.cc
+++ b/src/frames.cc
@@ -86,6 +86,7 @@ StackFrameIterator::StackFrameIterator(bool use_top, Address fp, Address sp)
   if (use_top || fp != NULL) {
     Reset();
   }
+  JavaScriptFrame_.DisableHeapAccess();
 }
 
 #undef INITIALIZE_SINGLETON
@@ -231,11 +232,7 @@ bool SafeStackFrameIterator::CanIterateHandles(StackFrame* frame,
 
 
 bool SafeStackFrameIterator::IsValidFrame(StackFrame* frame) const {
-  return IsValidStackAddress(frame->sp()) && IsValidStackAddress(frame->fp()) &&
-      // JavaScriptFrame uses function shared info to advance, hence it must
-      // point to a valid function object.
-      (!frame->is_java_script() ||
-       reinterpret_cast<JavaScriptFrame*>(frame)->is_at_function());
+  return IsValidStackAddress(frame->sp()) && IsValidStackAddress(frame->fp());
 }
 
 
@@ -281,7 +278,7 @@ void SafeStackFrameIterator::Reset() {
 SafeStackTraceFrameIterator::SafeStackTraceFrameIterator(
     Address fp, Address sp, Address low_bound, Address high_bound) :
     SafeJavaScriptFrameIterator(fp, sp, low_bound, high_bound) {
-  if (!done() && !frame()->is_at_function()) Advance();
+  if (!done() && !frame()->is_java_script()) Advance();
 }
 
 
@@ -289,7 +286,7 @@ void SafeStackTraceFrameIterator::Advance() {
   while (true) {
     SafeJavaScriptFrameIterator::Advance();
     if (done()) return;
-    if (frame()->is_at_function()) return;
+    if (frame()->is_java_script()) return;
   }
 }
 #endif

diff --git a/src/frames.h b/src/frames.h
index fbf99ff..8ab4be9 100644 (file)
--- a/src/frames.h
+++ b/src/frames.h
@@ -373,7 +373,6 @@ class JavaScriptFrame: public StandardFrame {
   virtual Type type() const { return JAVA_SCRIPT; }
 
   // Accessors.
-  inline bool is_at_function() const;
   inline Object* function() const;
   inline Object* receiver() const;
   inline void set_receiver(Object* value);
@@ -414,11 +413,19 @@ class JavaScriptFrame: public StandardFrame {
 
  protected:
   explicit JavaScriptFrame(StackFrameIterator* iterator)
-      : StandardFrame(iterator) { }
+      : StandardFrame(iterator), disable_heap_access_(false) { }
 
   virtual Address GetCallerStackPointer() const;
 
+  // When this mode is enabled it is not allowed to access heap objects.
+  // This is a special mode used when gathering stack samples in profiler.
+  // A shortcoming is that caller's SP value will be calculated incorrectly
+  // (see GetCallerStackPointer implementation), but it is not used for stack
+  // sampling.
+  void DisableHeapAccess() { disable_heap_access_ = true; }
+
  private:
+  bool disable_heap_access_;
   inline Object* function_slot_object() const;
 
   friend class StackFrameIterator;