Reviewers: loislo, Sven Panne,
Description:
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
Please review this at https://codereview.chromium.org/17585008/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/frames.h
M src/frames.cc
M src/isolate.h
Index: src/frames.cc
diff --git a/src/frames.cc b/src/frames.cc
index
3cf02f4007c076799324137d80406a3268ffefe3..e1c0892fdf4ae9d97e20672ac9afbdcddf2d8c1f
100644
--- a/src/frames.cc
+++ b/src/frames.cc
@@ -93,14 +93,16 @@ StackFrameIterator::StackFrameIterator(Isolate* isolate)
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),
+ is_safe_iterator_(false) {
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),
+ is_safe_iterator_(false) {
Reset();
}
StackFrameIterator::StackFrameIterator(Isolate* isolate,
@@ -111,7 +113,8 @@ StackFrameIterator::StackFrameIterator(Isolate* isolate,
thread_(use_top ? isolate_->thread_local_top() : NULL),
fp_(use_top ? NULL : fp), sp_(sp),
advance_(use_top ? &StackFrameIterator::AdvanceWithHandler :
- &StackFrameIterator::AdvanceWithoutHandler) {
+ &StackFrameIterator::AdvanceWithoutHandler),
+ is_safe_iterator_(true) {
if (use_top || fp != NULL) {
Reset();
}
@@ -166,7 +169,7 @@ void StackFrameIterator::Reset() {
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);
@@ -268,24 +271,9 @@ bool
SafeStackFrameIterator::ExitFrameValidator::IsValidFP(Address fp) {
}
-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 @@ SafeStackFrameIterator::SafeStackFrameIterator(
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 @@ void StackFrame::SetReturnAddressLocationResolver(
}
-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 @@ StackFrame::Type StackFrame::ComputeType(Isolate*
isolate, State* state) {
// 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->is_safe_iterator_) 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 @@ StackFrame::Type StackFrame::ComputeType(Isolate*
isolate, State* state) {
}
+#ifdef DEBUG
+bool StackFrame::is_safe_iteration() const {
+ return iterator_->is_safe_iterator_;
+}
+#endif
+
StackFrame::Type StackFrame::GetCallerState(State* state) const {
ComputeCallerState(state);
- return ComputeType(isolate(), state);
+ return ComputeType(iterator_, state);
}
@@ -622,7 +614,7 @@ bool StandardFrame::IsExpressionInsideHandler(int n)
const {
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(!is_safe_iteration());
// Compute the safepoint information.
unsigned stack_slots = 0;
@@ -754,7 +746,7 @@ Code* JavaScriptFrame::unchecked_code() const {
int JavaScriptFrame::GetNumberOfIncomingArguments() const {
- ASSERT(!SafeStackFrameIterator::is_active(isolate()) &&
+ ASSERT(!is_safe_iteration() &&
isolate()->heap()->gc_state() == Heap::NOT_IN_GC);
JSFunction* function = JSFunction::cast(this->function());
Index: src/frames.h
diff --git a/src/frames.h b/src/frames.h
index
1a78d09b15fc114eddde1b29b1ecf6fbe3e6c282..92ef45b31e1aa77f4f1aaed635c57d0cdf7c8e9a
100644
--- a/src/frames.h
+++ b/src/frames.h
@@ -321,7 +321,11 @@ class StackFrame BASE_EMBEDDED {
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 is_safe_iteration() const;
+#endif
private:
const StackFrameIterator* iterator_;
@@ -782,11 +786,6 @@ class StackFrameIterator BASE_EMBEDDED {
// 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 @@ class StackFrameIterator BASE_EMBEDDED {
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 @@ class StackFrameIterator BASE_EMBEDDED {
Address fp_;
Address sp_;
void (StackFrameIterator::*advance_)();
+ const bool is_safe_iterator_;
StackHandler* handler() const {
ASSERT(!done());
@@ -879,8 +885,6 @@ class SafeStackFrameIterator BASE_EMBEDDED {
bool done() const { return iteration_done_ || iterator_.done(); }
void Advance();
- static bool is_active(Isolate* isolate);
-
private:
void AdvanceOneFrame();
@@ -921,20 +925,6 @@ class SafeStackFrameIterator BASE_EMBEDDED {
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_;
Index: src/isolate.h
diff --git a/src/isolate.h b/src/isolate.h
index
0552ef7f8085b60f8b3d349580424b05255092ec..b1e2b07835e2a781988ccd3ac5d0c6a1c78712fe
100644
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -368,8 +368,6 @@ typedef List<HeapObject*,
PreallocatedStorageAllocationPolicy> DebugObjectCache;
/* 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.
