From d296a22917edad2820dbcab9ef33c763f3a7fd5a Mon Sep 17 00:00:00 2001 From: "bak@chromium.org" Date: Thu, 23 Oct 2008 14:55:45 +0000 Subject: [PATCH] - Fixed performance regression caused by ComputeContextSlotReceiver. - Eliminated a few write barriers. Review URL: http://codereview.chromium.org/8103 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@573 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/heap.cc | 17 +++---------- src/heap.h | 6 ++--- src/objects-inl.h | 17 ++++++++----- src/objects.cc | 16 +++++------- src/objects.h | 9 ++++--- src/runtime.cc | 65 ++++++++++++++++++++++++++++++----------------- 6 files changed, 70 insertions(+), 60 deletions(-) diff --git a/src/heap.cc b/src/heap.cc index 00cf78be5..4262cb06e 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -1378,12 +1378,11 @@ Object* Heap::AllocateConsString(String* first, String* second) { Object* result = Allocate(map, NEW_SPACE); if (result->IsFailure()) return result; - + ASSERT(InNewSpace(result)); ConsString* cons_string = ConsString::cast(result); - cons_string->set_first(first); - cons_string->set_second(second); + cons_string->set_first(first, SKIP_WRITE_BARRIER); + cons_string->set_second(second, SKIP_WRITE_BARRIER); cons_string->set_length(length); - return result; } @@ -1615,7 +1614,7 @@ Object* Heap::InitializeFunction(JSFunction* function, function->set_shared(shared); function->set_prototype_or_initial_map(prototype); function->set_context(undefined_value()); - function->set_literals(empty_fixed_array()); + function->set_literals(empty_fixed_array(), SKIP_WRITE_BARRIER); return function; } @@ -1676,14 +1675,6 @@ Object* Heap::AllocateArgumentsObject(Object* callee, int length) { Smi::FromInt(length), SKIP_WRITE_BARRIER); - // Allocate the elements if needed. - if (length > 0) { - // Allocate the fixed array. - Object* obj = Heap::AllocateFixedArray(length); - if (obj->IsFailure()) return obj; - JSObject::cast(result)->set_elements(FixedArray::cast(obj)); - } - // Check the state of the object ASSERT(JSObject::cast(result)->HasFastProperties()); ASSERT(JSObject::cast(result)->HasFastElements()); diff --git a/src/heap.h b/src/heap.h index c619e9f7f..87b972d9b 100644 --- a/src/heap.h +++ b/src/heap.h @@ -741,6 +741,9 @@ class Heap : public AllStatic { return amount_of_external_allocated_memory_; } + // Allocate unitialized fixed array (pretenure == NON_TENURE). + static Object* AllocateRawFixedArray(int length); + private: static int semispace_size_; static int initial_semispace_size_; @@ -835,9 +838,6 @@ class Heap : public AllStatic { // (since both AllocateRaw and AllocateRawMap are inlined). static inline Object* AllocateRawMap(int size_in_bytes); - // Allocate unitialized fixed array (pretenure == NON_TENURE). - static Object* AllocateRawFixedArray(int length); - // Initializes a JSObject based on its map. static void InitializeJSObjectFromMap(JSObject* obj, FixedArray* properties, diff --git a/src/objects-inl.h b/src/objects-inl.h index 0cfa12b01..fe470c898 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -510,7 +510,7 @@ Object* Object::GetProperty(String* key, PropertyAttributes* attributes) { #define WRITE_BARRIER(object, offset) \ Heap::RecordWrite(object->address(), offset); -// CONITIONAL_WRITE_BARRIER must be issued after the actual +// CONDITIONAL_WRITE_BARRIER must be issued after the actual // write due to the assert validating the written value. #define CONDITIONAL_WRITE_BARRIER(object, offset, mode) \ if (mode == UPDATE_WRITE_BARRIER) { \ @@ -1534,9 +1534,9 @@ Object* ConsString::first() { } -void ConsString::set_first(Object* value) { +void ConsString::set_first(Object* value, WriteBarrierMode mode) { WRITE_FIELD(this, kFirstOffset, value); - WRITE_BARRIER(this, kFirstOffset); + CONDITIONAL_WRITE_BARRIER(this, kFirstOffset, mode); } @@ -1545,9 +1545,9 @@ Object* ConsString::second() { } -void ConsString::set_second(Object* value) { +void ConsString::set_second(Object* value, WriteBarrierMode mode) { WRITE_FIELD(this, kSecondOffset, value); - WRITE_BARRIER(this, kSecondOffset); + CONDITIONAL_WRITE_BARRIER(this, kSecondOffset, mode); } @@ -2071,6 +2071,11 @@ bool JSFunction::is_compiled() { } +int JSFunction::NumberOfLiterals() { + return literals()->length(); +} + + Object* JSBuiltinsObject::javascript_builtin(Builtins::JavaScript id) { ASSERT(0 <= id && id < kJSBuiltinsCount); return READ_FIELD(this, kJSBuiltinsOffset + (id * kPointerSize)); @@ -2351,7 +2356,7 @@ void Map::ClearCodeCache() { void JSArray::SetContent(FixedArray* storage) { - set_length(Smi::FromInt(storage->length())); + set_length(Smi::FromInt(storage->length()), SKIP_WRITE_BARRIER); set_elements(storage); } diff --git a/src/objects.cc b/src/objects.cc index d12c3ebcc..65fa9eedc 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -2580,14 +2580,15 @@ Object* FixedArray::UnionOfKeys(FixedArray* other) { if (obj->IsFailure()) return obj; // Fill in the content FixedArray* result = FixedArray::cast(obj); + WriteBarrierMode mode = result->GetWriteBarrierMode(); for (int i = 0; i < len0; i++) { - result->set(i, get(i)); + result->set(i, get(i), mode); } // Fill in the extra keys. int index = 0; for (int y = 0; y < len1; y++) { if (!HasKey(this, other->get(y))) { - result->set(len0 + index, other->get(y)); + result->set(len0 + index, other->get(y), mode); index++; } } @@ -2601,14 +2602,14 @@ Object* FixedArray::CopySize(int new_length) { Object* obj = Heap::AllocateFixedArray(new_length); if (obj->IsFailure()) return obj; FixedArray* result = FixedArray::cast(obj); - WriteBarrierMode mode = result->GetWriteBarrierMode(); // Copy the content int len = length(); if (new_length < len) len = new_length; + result->set_map(map()); + WriteBarrierMode mode = result->GetWriteBarrierMode(); for (int i = 0; i < len; i++) { result->set(i, get(i), mode); } - result->set_map(map()); return result; } @@ -4047,11 +4048,6 @@ void Map::MapIterateBody(ObjectVisitor* v) { } -int JSFunction::NumberOfLiterals() { - return literals()->length(); -} - - Object* JSFunction::SetInstancePrototype(Object* value) { ASSERT(value->IsJSObject()); @@ -5974,7 +5970,7 @@ Object* LookupCache::Put(Map* map, String* name, int value) { int entry = cache->FindInsertionEntry(k, key.Hash()); int index = EntryToIndex(entry); cache->set(index, k); - cache->set(index + 1, Smi::FromInt(value)); + cache->set(index + 1, Smi::FromInt(value), SKIP_WRITE_BARRIER); cache->ElementAdded(); return cache; } diff --git a/src/objects.h b/src/objects.h index e3bd522a8..c490831df 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1601,7 +1601,6 @@ class DescriptorArray: public FixedArray { inline void Get(int descriptor_number, Descriptor* desc); inline void Set(int descriptor_number, Descriptor* desc); - // Copy the descriptor array, insert a new descriptor and optionally // remove map transitions. If the descriptor is already present, it is // replaced. If a replaced descriptor is a real property (not a transition @@ -2750,7 +2749,7 @@ class JSFunction: public JSObject { #endif // Returns the number of allocated literals. - int NumberOfLiterals(); + inline int NumberOfLiterals(); // Retrieve the global context from a function's literal array. static Context* GlobalContextFromLiterals(FixedArray* literals); @@ -3347,11 +3346,13 @@ class ConsString: public String { public: // First object of the cons cell. inline Object* first(); - inline void set_first(Object* first); + inline void set_first(Object* first, + WriteBarrierMode mode = UPDATE_WRITE_BARRIER); // Second object of the cons cell. inline Object* second(); - inline void set_second(Object* second); + inline void set_second(Object* second, + WriteBarrierMode mode = UPDATE_WRITE_BARRIER); // Dispatched behavior. uint16_t ConsStringGet(int index); diff --git a/src/runtime.cc b/src/runtime.cc index 32246f348..9c91d5293 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -928,7 +928,7 @@ static Object* Runtime_SetCode(Arguments args) { literals->set(JSFunction::kLiteralGlobalContextIndex, context->global_context()); } - target->set_literals(*literals); + target->set_literals(*literals, SKIP_WRITE_BARRIER); } target->set_context(*context); @@ -3134,11 +3134,16 @@ static Object* Runtime_NewArguments(Arguments args) { const int length = frame->GetProvidedParametersCount(); Object* result = Heap::AllocateArgumentsObject(callee, length); if (result->IsFailure()) return result; - FixedArray* array = FixedArray::cast(JSObject::cast(result)->elements()); - ASSERT(array->length() == length); - WriteBarrierMode mode = array->GetWriteBarrierMode(); - for (int i = 0; i < length; i++) { - array->set(i, frame->GetParameter(i), mode); + if (length > 0) { + Object* obj = Heap::AllocateFixedArray(length); + if (obj->IsFailure()) return obj; + FixedArray* array = FixedArray::cast(obj); + ASSERT(array->length() == length); + WriteBarrierMode mode = array->GetWriteBarrierMode(); + for (int i = 0; i < length; i++) { + array->set(i, frame->GetParameter(i), mode); + } + JSObject::cast(result)->set_elements(array); } return result; } @@ -3154,11 +3159,22 @@ static Object* Runtime_NewArgumentsFast(Arguments args) { Object* result = Heap::AllocateArgumentsObject(callee, length); if (result->IsFailure()) return result; - FixedArray* array = FixedArray::cast(JSObject::cast(result)->elements()); - ASSERT(array->length() == length); - WriteBarrierMode mode = array->GetWriteBarrierMode(); - for (int i = 0; i < length; i++) { - array->set(i, *--parameters, mode); + ASSERT(Heap::InNewSpace(result)); + + // Allocate the elements if needed. + if (length > 0) { + // Allocate the fixed array. + Object* obj = Heap::AllocateRawFixedArray(length); + if (obj->IsFailure()) return obj; + reinterpret_cast(obj)->set_map(Heap::fixed_array_map()); + FixedArray* array = FixedArray::cast(obj); + array->set_length(length); + WriteBarrierMode mode = array->GetWriteBarrierMode(); + for (int i = 0; i < length; i++) { + array->set(i, *--parameters, mode); + } + JSObject::cast(result)->set_elements(FixedArray::cast(obj), + SKIP_WRITE_BARRIER); } return result; } @@ -3366,10 +3382,17 @@ static Object* ComputeContextSlotReceiver(Object* holder) { // If the "property" we were looking for is a local variable or an // argument in a context, the receiver is the global object; see // ECMA-262, 3rd., 10.1.6 and 10.2.3. - HeapObject* object = HeapObject::cast(holder); - Context* top = Top::context(); - if (holder->IsContext()) return top->global()->global_receiver(); + // Contexts and global objects are most common. + if (holder->IsContext()) { + return Context::cast(holder)->global()->global_receiver(); + } + if (holder->IsGlobalObject()) { + // If the holder is a global object, we have to be careful to wrap + // it in its proxy if necessary. + return GlobalObject::cast(holder)->global_receiver(); + } + Context* top = Top::context(); // TODO(125): Find a better - and faster way - of checking for // arguments and context extension objects. This kinda sucks. JSFunction* context_extension_function = @@ -3386,13 +3409,7 @@ static Object* ComputeContextSlotReceiver(Object* holder) { return Top::context()->global()->global_receiver(); } - // If the holder is a global object, we have to be careful to wrap - // it in its proxy if necessary. - if (object->IsGlobalObject()) { - return GlobalObject::cast(object)->global_receiver(); - } else { - return object; - } + return holder; } @@ -4900,13 +4917,13 @@ static Handle GetArgumentsObject(JavaScriptFrame* frame, } const int length = frame->GetProvidedParametersCount(); - Handle arguments = Factory::NewArgumentsObject(function, length); - FixedArray* array = FixedArray::cast(JSObject::cast(*arguments)->elements()); - ASSERT(array->length() == length); + Handle arguments = Factory::NewArgumentsObject(function, length); + Handle array = Factory::NewFixedArray(length); WriteBarrierMode mode = array->GetWriteBarrierMode(); for (int i = 0; i < length; i++) { array->set(i, frame->GetParameter(i), mode); } + arguments->set_elements(*array); return arguments; } -- 2.34.1