Revision: 15317
Author: [email protected]
Date: Tue Jun 25 03:09:19 2013
Log: Get rid of Isolate::safe_stack_iterator_counter
This change removes per-isolate counter of active SafeStackFrameIterators.
The counter is used by stack frames implementations to avoid accessing
pointers to heap objects when traversing stack for CPU profiler (so
called "safe" mode). Each StackFrame instance is owned by single iterator
and has a pointer to it so we can simply mark the iterator as "safe" or not
and read the field in the stack frames instead of going into the isolate.
BUG=None
[email protected], [email protected]
Review URL: https://codereview.chromium.org/17585008
http://code.google.com/p/v8/source/detail?r=15317
Modified:
/branches/bleeding_edge/src/frames.cc
/branches/bleeding_edge/src/frames.h
/branches/bleeding_edge/src/isolate.h
=======================================
--- /branches/bleeding_edge/src/frames.cc Tue Jun 25 00:14:06 2013
+++ /branches/bleeding_edge/src/frames.cc Tue Jun 25 03:09:19 2013
@@ -93,14 +93,16 @@
STACK_FRAME_TYPE_LIST(INITIALIZE_SINGLETON)
frame_(NULL), handler_(NULL),
thread_(isolate_->thread_local_top()),
- fp_(NULL), sp_(NULL),
advance_(&StackFrameIterator::AdvanceWithHandler) {
+ fp_(NULL), sp_(NULL),
advance_(&StackFrameIterator::AdvanceWithHandler),
+ can_access_heap_objects_(true) {
Reset();
}
StackFrameIterator::StackFrameIterator(Isolate* isolate, ThreadLocalTop* t)
: isolate_(isolate),
STACK_FRAME_TYPE_LIST(INITIALIZE_SINGLETON)
frame_(NULL), handler_(NULL), thread_(t),
- fp_(NULL), sp_(NULL),
advance_(&StackFrameIterator::AdvanceWithHandler) {
+ fp_(NULL), sp_(NULL),
advance_(&StackFrameIterator::AdvanceWithHandler),
+ can_access_heap_objects_(true) {
Reset();
}
StackFrameIterator::StackFrameIterator(Isolate* isolate,
@@ -111,7 +113,8 @@
thread_(use_top ? isolate_->thread_local_top() : NULL),
fp_(use_top ? NULL : fp), sp_(sp),
advance_(use_top ? &StackFrameIterator::AdvanceWithHandler :
- &StackFrameIterator::AdvanceWithoutHandler) {
+ &StackFrameIterator::AdvanceWithoutHandler),
+ can_access_heap_objects_(false) {
if (use_top || fp != NULL) {
Reset();
}
@@ -166,7 +169,7 @@
state.sp = sp_;
state.pc_address = ResolveReturnAddressLocation(
reinterpret_cast<Address*>(StandardFrame::ComputePCAddress(fp_)));
- type = StackFrame::ComputeType(isolate(), &state);
+ type = StackFrame::ComputeType(this, &state);
}
if (SingletonFor(type) == NULL) return;
frame_ = SingletonFor(type, &state);
@@ -266,26 +269,11 @@
}
return *state.pc_address != NULL;
}
-
-
-SafeStackFrameIterator::ActiveCountMaintainer::ActiveCountMaintainer(
- Isolate* isolate)
- : isolate_(isolate) {
- isolate_->set_safe_stack_iterator_counter(
- isolate_->safe_stack_iterator_counter() + 1);
-}
-
-
-SafeStackFrameIterator::ActiveCountMaintainer::~ActiveCountMaintainer() {
- isolate_->set_safe_stack_iterator_counter(
- isolate_->safe_stack_iterator_counter() - 1);
-}
SafeStackFrameIterator::SafeStackFrameIterator(
Isolate* isolate,
Address fp, Address sp, Address low_bound, Address high_bound) :
- maintainer_(isolate),
stack_validator_(low_bound, high_bound),
is_valid_top_(IsValidTop(isolate, low_bound, high_bound)),
is_valid_fp_(IsWithinBounds(low_bound, high_bound, fp)),
@@ -294,10 +282,6 @@
if (!done()) Advance();
}
-bool SafeStackFrameIterator::is_active(Isolate* isolate) {
- return isolate->safe_stack_iterator_counter() > 0;
-}
-
bool SafeStackFrameIterator::IsValidTop(Isolate* isolate,
Address low_bound, Address
high_bound) {
@@ -435,7 +419,8 @@
}
-StackFrame::Type StackFrame::ComputeType(Isolate* isolate, State* state) {
+StackFrame::Type StackFrame::ComputeType(const StackFrameIterator*
iterator,
+ State* state) {
ASSERT(state->fp != NULL);
if (StandardFrame::IsArgumentsAdaptorFrame(state->fp)) {
return ARGUMENTS_ADAPTOR;
@@ -450,8 +435,9 @@
// frames as normal JavaScript frames to avoid having to look
// into the heap to determine the state. This is safe as long
// as nobody tries to GC...
- if (SafeStackFrameIterator::is_active(isolate)) return JAVA_SCRIPT;
- Code::Kind kind = GetContainingCode(isolate,
*(state->pc_address))->kind();
+ if (!iterator->can_access_heap_objects_) return JAVA_SCRIPT;
+ Code::Kind kind = GetContainingCode(iterator->isolate(),
+ *(state->pc_address))->kind();
ASSERT(kind == Code::FUNCTION || kind == Code::OPTIMIZED_FUNCTION);
return (kind == Code::OPTIMIZED_FUNCTION) ? OPTIMIZED : JAVA_SCRIPT;
}
@@ -459,10 +445,16 @@
}
+#ifdef DEBUG
+bool StackFrame::can_access_heap_objects() const {
+ return iterator_->can_access_heap_objects_;
+}
+#endif
+
StackFrame::Type StackFrame::GetCallerState(State* state) const {
ComputeCallerState(state);
- return ComputeType(isolate(), state);
+ return ComputeType(iterator_, state);
}
@@ -622,7 +614,7 @@
void StandardFrame::IterateCompiledFrame(ObjectVisitor* v) const {
// Make sure that we're not doing "safe" stack frame iteration. We cannot
// possibly find pointers in optimized frames in that state.
- ASSERT(!SafeStackFrameIterator::is_active(isolate()));
+ ASSERT(can_access_heap_objects());
// Compute the safepoint information.
unsigned stack_slots = 0;
@@ -754,7 +746,7 @@
int JavaScriptFrame::GetNumberOfIncomingArguments() const {
- ASSERT(!SafeStackFrameIterator::is_active(isolate()) &&
+ ASSERT(can_access_heap_objects() &&
isolate()->heap()->gc_state() == Heap::NOT_IN_GC);
JSFunction* function = JSFunction::cast(this->function());
=======================================
--- /branches/bleeding_edge/src/frames.h Tue Jun 25 00:14:06 2013
+++ /branches/bleeding_edge/src/frames.h Tue Jun 25 03:09:19 2013
@@ -321,7 +321,11 @@
inline StackHandler* top_handler() const;
// Compute the stack frame type for the given state.
- static Type ComputeType(Isolate* isolate, State* state);
+ static Type ComputeType(const StackFrameIterator* iterator, State*
state);
+
+#ifdef DEBUG
+ bool can_access_heap_objects() const;
+#endif
private:
const StackFrameIterator* iterator_;
@@ -782,11 +786,6 @@
// An iterator that iterates over a given thread's stack.
StackFrameIterator(Isolate* isolate, ThreadLocalTop* t);
- // An iterator that can start from a given FP address.
- // If use_top, then work as usual, if fp isn't NULL, use it,
- // otherwise, do nothing.
- StackFrameIterator(Isolate* isolate, bool use_top, Address fp, Address
sp);
-
StackFrame* frame() const {
ASSERT(!done());
return frame_;
@@ -798,6 +797,12 @@
void Advance() { (this->*advance_)(); }
private:
+ // An iterator that can start from a given FP address.
+ // If use_top, then work as usual, if fp isn't NULL, use it,
+ // otherwise, do nothing. This constructor is used to create
+ // StackFrameIterator for "safe" stack iteration.
+ StackFrameIterator(Isolate* isolate, bool use_top, Address fp, Address
sp);
+
// Go back to the first frame.
void Reset();
@@ -811,6 +816,7 @@
Address fp_;
Address sp_;
void (StackFrameIterator::*advance_)();
+ const bool can_access_heap_objects_;
StackHandler* handler() const {
ASSERT(!done());
@@ -879,8 +885,6 @@
bool done() const { return iteration_done_ || iterator_.done(); }
void Advance();
- static bool is_active(Isolate* isolate);
-
private:
void AdvanceOneFrame();
@@ -921,20 +925,6 @@
static bool IsValidTop(Isolate* isolate,
Address low_bound, Address high_bound);
- // This is a nasty hack to make sure the active count is incremented
- // before the constructor for the embedded iterator is invoked. This
- // is needed because the constructor will start looking at frames
- // right away and we need to make sure it doesn't start inspecting
- // heap objects.
- class ActiveCountMaintainer BASE_EMBEDDED {
- public:
- explicit ActiveCountMaintainer(Isolate* isolate);
- ~ActiveCountMaintainer();
- private:
- Isolate* isolate_;
- };
-
- ActiveCountMaintainer maintainer_;
StackAddressValidator stack_validator_;
const bool is_valid_top_;
const bool is_valid_fp_;
=======================================
--- /branches/bleeding_edge/src/isolate.h Thu Jun 20 03:05:33 2013
+++ /branches/bleeding_edge/src/isolate.h Tue Jun 25 03:09:19 2013
@@ -368,8 +368,6 @@
/* AstNode state.
*/ \
V(int, ast_node_id,
0) \
V(unsigned, ast_node_count,
0) \
- /* SafeStackFrameIterator activations count.
*/ \
- V(int, safe_stack_iterator_counter,
0) \
V(bool, observer_delivery_pending,
false) \
V(HStatistics*, hstatistics,
NULL) \
V(HTracer*, htracer,
NULL) \
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
