Cool. Couple comments:
http://codereview.chromium.org/3139005/diff/1/2 File src/contexts.cc (right): http://codereview.chromium.org/3139005/diff/1/2#newcode70 src/contexts.cc:70: Context* Context::global_context(Heap* heap) { I wonder how much speedup this particular function brings. It seems the most often case is even before GetHeap() would be called. Maybe nice to remove a duplicate if speed is not gained. http://codereview.chromium.org/3139005/diff/1/4 File src/objects-inl.h (right): http://codereview.chromium.org/3139005/diff/1/4#newcode475 src/objects-inl.h:475: bool Object::IsGlobalContext(Heap* heap) { Same Q about this duplicate: if replaced with IsGlobalContext(), does it affect the perf? In some callsites perhaps this will allow to postpone getting the heap (and leave compiler a bit more space for optimization) http://codereview.chromium.org/3139005/diff/1/4#newcode1067 src/objects-inl.h:1067: #ifdef DEBUG do we need this debug code? I believe ASSERT is already in Map::heap(). http://codereview.chromium.org/3139005/diff/1/4#newcode1077 src/objects-inl.h:1077: Heap* Object::SlowGetHeap() { I measured similar function recently... It seems having that conditional statement makes it pretty much same speed as simple Isolate::Current()->heap(). Perhaps can be removed? http://codereview.chromium.org/3139005/diff/1/4#newcode1088 src/objects-inl.h:1088: Heap* Map::GetHeap() { If we move ASSERT for GC into Map::heap(), that one can be used instead (I assume you want to avoid extra dereference). Having non-virtual methods with the same signature on derived objects seems a bit dangerous... http://codereview.chromium.org/3139005/diff/1/4#newcode1095 src/objects-inl.h:1095: Heap* Map::MapGetHeap() { this seems not used. http://codereview.chromium.org/3139005/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
