From 0eb4dea12d16eac04ad3500899144d49edebabc8 Mon Sep 17 00:00:00 2001 From: "mikhail.naganov@gmail.com" Date: Mon, 13 Apr 2009 23:12:04 +0000 Subject: [PATCH] Implemented "no heap access" mode for JSFrame which is used for stack sampling in profiler. 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 --- src/frames-arm.cc | 2 +- src/frames-ia32.cc | 2 +- src/frames-inl.h | 13 ------------- src/frames.cc | 11 ++++------- src/frames.h | 11 +++++++++-- 5 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/frames-arm.cc b/src/frames-arm.cc index fe850a8..121fb75 100644 --- 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 --- 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 --- 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
(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 --- 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(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 --- 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; -- 2.7.4