Revision: 15348
Author: [email protected]
Date: Thu Jun 27 02:28:11 2013
Log: Do not iterate stack handlers in SafeStackFrameIterator
CPU profiler doesn't use stack handlers so there is no need to iterate
through them while traversing stack. This change SafeStackFrameIterator
always iterate only frames and removes checks corresponding to the handlers
iteration.
The problem described in the bug occurred because of a false assumption in
SafeStackFrameIterator that if Isolate::c_entry_fp is not NULL then the top
frame on the stack is always a C++ frame. It is false because we may have
entered JS code again, in which case JS_ENTRY code stub generated by
JSEntryStub::GenerateBody() will save current c_entry_fp value but not
reset it to NULL and after that it will create ENTRY stack frame and
JS_ENTRY handler on the stack and put the latter into
Isolate::handler(top). This means that if we start iterating from
c_entry_fp frame and try to compare the frame's sp with
Isolate::handler()->address() it will turn out that frame->sp() >
handler->address() and the condition in
SafeStackFrameIterator::CanIterateHandles is not held.
BUG=252097
[email protected], [email protected]
Review URL: https://codereview.chromium.org/17589022
http://code.google.com/p/v8/source/detail?r=15348
Modified:
/branches/bleeding_edge/src/frames.cc
/branches/bleeding_edge/src/frames.h
/branches/bleeding_edge/test/cctest/test-cpu-profiler.cc
=======================================
--- /branches/bleeding_edge/src/frames.cc Wed Jun 26 07:04:25 2013
+++ /branches/bleeding_edge/src/frames.cc Thu Jun 27 02:28:11 2013
@@ -112,8 +112,7 @@
frame_(NULL), handler_(NULL),
thread_(use_top ? isolate_->thread_local_top() : NULL),
fp_(use_top ? NULL : fp), sp_(sp),
- advance_(use_top ? &StackFrameIterator::AdvanceWithHandler :
- &StackFrameIterator::AdvanceWithoutHandler),
+ advance_(&StackFrameIterator::AdvanceWithoutHandler),
can_access_heap_objects_(false) {
if (use_top || fp != NULL) {
Reset();
@@ -299,7 +298,6 @@
Address last_sp = last_frame->sp(), last_fp = last_frame->fp();
// Before advancing to the next stack frame, perform pointer validity
tests
iteration_done_ = !IsValidFrame(last_frame) ||
- !CanIterateHandles(last_frame, iterator_.handler()) ||
!IsValidCaller(last_frame);
if (iteration_done_) return;
@@ -309,15 +307,6 @@
StackFrame* prev_frame = iterator_.frame();
iteration_done_ = prev_frame->sp() < last_sp || prev_frame->fp() <
last_fp;
}
-
-
-bool SafeStackFrameIterator::CanIterateHandles(StackFrame* frame,
- StackHandler* handler) {
- // If StackIterator iterates over StackHandles, verify that
- // StackHandlerIterator can be instantiated (see StackHandlerIterator
- // constructor.)
- return !is_valid_top_ || (frame->sp() <= handler->address());
-}
bool SafeStackFrameIterator::IsValidFrame(StackFrame* frame) const {
=======================================
--- /branches/bleeding_edge/src/frames.h Tue Jun 25 03:09:19 2013
+++ /branches/bleeding_edge/src/frames.h Thu Jun 27 02:28:11 2013
@@ -919,7 +919,6 @@
bool IsValidStackAddress(Address addr) const {
return stack_validator_.IsValid(addr);
}
- bool CanIterateHandles(StackFrame* frame, StackHandler* handler);
bool IsValidFrame(StackFrame* frame) const;
bool IsValidCaller(StackFrame* frame);
static bool IsValidTop(Isolate* isolate,
=======================================
--- /branches/bleeding_edge/test/cctest/test-cpu-profiler.cc Thu Jun 13
12:16:35 2013
+++ /branches/bleeding_edge/test/cctest/test-cpu-profiler.cc Thu Jun 27
02:28:11 2013
@@ -914,3 +914,54 @@
cpu_profiler->DeleteAllCpuProfiles();
}
+
+
+static const char* bound_function_test_source = "function foo(iterations)
{\n"
+" var r = 0;\n"
+" for (var i = 0; i < iterations; i++) { r += i; }\n"
+" return r;\n"
+"}\n"
+"function start(duration) {\n"
+" var callback = foo.bind(this);\n"
+" var start = Date.now();\n"
+" while (Date.now() - start < duration) {\n"
+" callback(10 * 1000);\n"
+" }\n"
+"}";
+
+
+TEST(BoundFunctionCall) {
+ LocalContext env;
+ v8::HandleScope scope(env->GetIsolate());
+
+ v8::Script::Compile(v8::String::New(bound_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(3);
+ names[0] = v8::String::New(ProfileGenerator::kGarbageCollectorEntryName);
+ names[1] = v8::String::New(ProfileGenerator::kProgramEntryName);
+ names[2] = v8::String::New("start");
+ // Don't allow |foo| node to be at the top level.
+ CheckChildrenNames(root, names);
+
+ const v8::CpuProfileNode* startNode = GetChild(root, "start");
+ GetChild(startNode, "foo");
+
+ cpu_profiler->DeleteAllCpuProfiles();
+}
--
--
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.
