Here's the top of an oprofile run before this change, showing RawSyncElement and PrepareForCall: 44461 3.8503 shell v8::internal::MarkingVisitor::VisitPointers(v8::internal::Object**, v8::internal::Object**) 36612 3.1706 shell v8::internal::Scanner::ScanIdentifier() 30610 2.6508 shell v8::internal::UTF8Buffer::AddChar(int) 29237 2.5319 shell v8::internal::JumpTarget::ComputeEntryFrame(int) 22616 1.9585 shell v8::internal::VirtualFrame::RawSyncElementAt(int) 21849 1.8921 shell v8::internal::Heap::IterateRSet(v8::internal::PagedSpace*, void (*)(v8::internal::HeapObject**)) 21259 1.8410 shell v8::internal::RelocIterator::next() 21080 1.8255 shell v8::internal::Scanner::ScanToken() 20865 1.8069 shell v8::internal::Scanner::SkipWhiteSpace(bool) 20150 1.7450 shell v8::internal::Heap::CopyFixedArray(v8::internal::FixedArray*) 19240 1.6662 shell v8::internal::String::IsEqualTo(v8::internal::Vector<char const>) 18414 1.5947 shell unibrow::Utf8::ReadBlock(unibrow::Buffer<char const*>, unsigned char*, unsigned int, unsigned int*, unsigned int*) 17095 1.4804 shell v8::internal::Scanner::Next() 14925 1.2925 shell v8::internal::VirtualFrame::PrepareMergeTo(v8::internal::VirtualFrame*) 14001 1.2125 shell v8::internal::ScavengeVisitor::VisitPointers(v8::internal::Object**, v8::internal::Object**) 12028 1.0416 shell v8::internal::JSObject::GetElementWithReceiver(v8::internal::JSObject*, unsigned int) 11172 0.9675 shell v8::internal::Heap::AllocateStringFromUtf8(v8::internal::Vector<char const>, v8::internal::PretenureFlag) 10485 0.9080 shell v8::internal::Heap::IterateRSetRange(unsigned char*, unsigned char*, unsigned char*, void (*)(v8::internal::HeapObject**)) 10411 0.9016 shell v8::internal::Token::Lookup(char const*) 10358 0.8970 shell v8::internal::VirtualFrame::PrepareForCall(int, int)
On Wed, Mar 25, 2009 at 4:58 PM, <[email protected]> wrote: > Oh, and LGTM. > > > http://codereview.chromium.org/49029/diff/2007/2010 > File src/virtual-frame.cc (right): > > http://codereview.chromium.org/49029/diff/2007/2010#newcode216 > Line 216: // [min(stack_pointer_,begin), end). > You mean min(stack_pointer + 1, begin)? > > It might be better in the sense of avoiding surprise to not make this > general and to require the caller to give a proper range (ie, beginning > at or below stack_pointer + 1). It would be more explicit at the > calling sites (both of them!) what's going on (and I think it's true now > anyway). > > http://codereview.chromium.org/49029/diff/2007/2010#newcode220 > Line 220: if (begin > stack_pointer_) { > This whole think is just complicated enough to need some comments so it > doesn't get off by one when maintaining it. > > > http://codereview.chromium.org/49029 > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
