From 45ebabfbec47e14bdd2c01192a15c6b1bf6d3be0 Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Tue, 4 Mar 2014 12:51:40 +0000 Subject: [PATCH] Refactoring to clean up duplicate code in Heap::Allocate methods. R=hpayer@chromium.org Review URL: https://codereview.chromium.org/170703002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19651 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/heap.cc | 116 +++++++++++---------------------------------------------- src/heap.h | 25 ++++++------- src/runtime.cc | 22 ++++++++++- 3 files changed, 54 insertions(+), 109 deletions(-) diff --git a/src/heap.cc b/src/heap.cc index 8b94dbe..1f97fc3 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -4193,28 +4193,8 @@ void Heap::InitializeAllocationMemento(AllocationMemento* memento, } -MaybeObject* Heap::AllocateWithAllocationSite(Map* map, AllocationSpace space, - Handle allocation_site) { - ASSERT(gc_state_ == NOT_IN_GC); - ASSERT(map->instance_type() != MAP_TYPE); - // If allocation failures are disallowed, we may allocate in a different - // space when new space is full and the object is not a large object. - AllocationSpace retry_space = - (space != NEW_SPACE) ? space : TargetSpaceId(map->instance_type()); - int size = map->instance_size() + AllocationMemento::kSize; - Object* result; - MaybeObject* maybe_result = AllocateRaw(size, space, retry_space); - if (!maybe_result->ToObject(&result)) return maybe_result; - // No need for write barrier since object is white and map is in old space. - HeapObject::cast(result)->set_map_no_write_barrier(map); - AllocationMemento* alloc_memento = reinterpret_cast( - reinterpret_cast
(result) + map->instance_size()); - InitializeAllocationMemento(alloc_memento, *allocation_site); - return result; -} - - -MaybeObject* Heap::Allocate(Map* map, AllocationSpace space) { +MaybeObject* Heap::Allocate(Map* map, AllocationSpace space, + AllocationSite* allocation_site) { ASSERT(gc_state_ == NOT_IN_GC); ASSERT(map->instance_type() != MAP_TYPE); // If allocation failures are disallowed, we may allocate in a different @@ -4222,11 +4202,19 @@ MaybeObject* Heap::Allocate(Map* map, AllocationSpace space) { AllocationSpace retry_space = (space != NEW_SPACE) ? space : TargetSpaceId(map->instance_type()); int size = map->instance_size(); + if (allocation_site != NULL) { + size += AllocationMemento::kSize; + } Object* result; MaybeObject* maybe_result = AllocateRaw(size, space, retry_space); if (!maybe_result->ToObject(&result)) return maybe_result; // No need for write barrier since object is white and map is in old space. HeapObject::cast(result)->set_map_no_write_barrier(map); + if (allocation_site != NULL) { + AllocationMemento* alloc_memento = reinterpret_cast( + reinterpret_cast
(result) + map->instance_size()); + InitializeAllocationMemento(alloc_memento, allocation_site); + } return result; } @@ -4350,7 +4338,10 @@ void Heap::InitializeJSObjectFromMap(JSObject* obj, MaybeObject* Heap::AllocateJSObjectFromMap( - Map* map, PretenureFlag pretenure, bool allocate_properties) { + Map* map, + PretenureFlag pretenure, + bool allocate_properties, + AllocationSite* allocation_site) { // JSFunctions should be allocated using AllocateFunction to be // properly initialized. ASSERT(map->instance_type() != JS_FUNCTION_TYPE); @@ -4376,7 +4367,7 @@ MaybeObject* Heap::AllocateJSObjectFromMap( int size = map->instance_size(); AllocationSpace space = SelectSpace(size, OLD_POINTER_SPACE, pretenure); Object* obj; - MaybeObject* maybe_obj = Allocate(map, space); + MaybeObject* maybe_obj = Allocate(map, space, allocation_site); if (!maybe_obj->To(&obj)) return maybe_obj; // Initialize the JSObject. @@ -4387,79 +4378,16 @@ MaybeObject* Heap::AllocateJSObjectFromMap( } -MaybeObject* Heap::AllocateJSObjectFromMapWithAllocationSite( - Map* map, Handle allocation_site) { - // JSFunctions should be allocated using AllocateFunction to be - // properly initialized. - ASSERT(map->instance_type() != JS_FUNCTION_TYPE); - - // Both types of global objects should be allocated using - // AllocateGlobalObject to be properly initialized. - ASSERT(map->instance_type() != JS_GLOBAL_OBJECT_TYPE); - ASSERT(map->instance_type() != JS_BUILTINS_OBJECT_TYPE); - - // Allocate the backing storage for the properties. - int prop_size = map->InitialPropertiesLength(); - ASSERT(prop_size >= 0); - FixedArray* properties; - { MaybeObject* maybe_properties = AllocateFixedArray(prop_size); - if (!maybe_properties->To(&properties)) return maybe_properties; - } - - // Allocate the JSObject. - int size = map->instance_size(); - AllocationSpace space = SelectSpace(size, OLD_POINTER_SPACE, NOT_TENURED); - Object* obj; - MaybeObject* maybe_obj = - AllocateWithAllocationSite(map, space, allocation_site); - if (!maybe_obj->To(&obj)) return maybe_obj; - - // Initialize the JSObject. - InitializeJSObjectFromMap(JSObject::cast(obj), properties, map); - ASSERT(JSObject::cast(obj)->HasFastElements()); - return obj; -} - - MaybeObject* Heap::AllocateJSObject(JSFunction* constructor, - PretenureFlag pretenure) { + PretenureFlag pretenure, + AllocationSite* allocation_site) { ASSERT(constructor->has_initial_map()); - // Allocate the object based on the constructors initial map. - MaybeObject* result = AllocateJSObjectFromMap( - constructor->initial_map(), pretenure); -#ifdef DEBUG - // Make sure result is NOT a global object if valid. - Object* non_failure; - ASSERT(!result->ToObject(&non_failure) || !non_failure->IsGlobalObject()); -#endif - return result; -} - -MaybeObject* Heap::AllocateJSObjectWithAllocationSite(JSFunction* constructor, - Handle allocation_site) { - ASSERT(constructor->has_initial_map()); - // Allocate the object based on the constructors initial map, or the payload - // advice - Map* initial_map = constructor->initial_map(); - - ElementsKind to_kind = allocation_site->GetElementsKind(); - AllocationSiteMode mode = TRACK_ALLOCATION_SITE; - if (to_kind != initial_map->elements_kind()) { - MaybeObject* maybe_new_map = initial_map->AsElementsKind(to_kind); - if (!maybe_new_map->To(&initial_map)) return maybe_new_map; - // Possibly alter the mode, since we found an updated elements kind - // in the type info cell. - mode = AllocationSite::GetMode(to_kind); - } - - MaybeObject* result; - if (mode == TRACK_ALLOCATION_SITE) { - result = AllocateJSObjectFromMapWithAllocationSite(initial_map, - allocation_site); - } else { - result = AllocateJSObjectFromMap(initial_map, NOT_TENURED); - } + // Allocate the object based on the constructors initial map. + MaybeObject* result = AllocateJSObjectFromMap(constructor->initial_map(), + pretenure, + true, + allocation_site); #ifdef DEBUG // Make sure result is NOT a global object if valid. Object* non_failure; diff --git a/src/heap.h b/src/heap.h index e2c42dd..c82c34e 100644 --- a/src/heap.h +++ b/src/heap.h @@ -684,14 +684,13 @@ class Heap { // constructor. // Returns Failure::RetryAfterGC(requested_bytes, space) if the allocation // failed. + // If allocation_site is non-null, then a memento is emitted after the object + // that points to the site. // Please note this does not perform a garbage collection. MUST_USE_RESULT MaybeObject* AllocateJSObject( JSFunction* constructor, - PretenureFlag pretenure = NOT_TENURED); - - MUST_USE_RESULT MaybeObject* AllocateJSObjectWithAllocationSite( - JSFunction* constructor, - Handle allocation_site); + PretenureFlag pretenure = NOT_TENURED, + AllocationSite* allocation_site = NULL); MUST_USE_RESULT MaybeObject* AllocateJSModule(Context* context, ScopeInfo* scope_info); @@ -771,21 +770,21 @@ class Heap { // Allocates and initializes a new JavaScript object based on a map. // Returns Failure::RetryAfterGC(requested_bytes, space) if the allocation // failed. + // Passing an allocation site means that a memento will be created that + // points to the site. // Please note this does not perform a garbage collection. MUST_USE_RESULT MaybeObject* AllocateJSObjectFromMap( - Map* map, PretenureFlag pretenure = NOT_TENURED, bool alloc_props = true); - - MUST_USE_RESULT MaybeObject* AllocateJSObjectFromMapWithAllocationSite( - Map* map, Handle allocation_site); + Map* map, + PretenureFlag pretenure = NOT_TENURED, + bool alloc_props = true, + AllocationSite* allocation_site = NULL); // Allocates a heap object based on the map. // Returns Failure::RetryAfterGC(requested_bytes, space) if the allocation // failed. // Please note this function does not perform a garbage collection. - MUST_USE_RESULT MaybeObject* Allocate(Map* map, AllocationSpace space); - - MUST_USE_RESULT MaybeObject* AllocateWithAllocationSite(Map* map, - AllocationSpace space, Handle allocation_site); + MUST_USE_RESULT MaybeObject* Allocate(Map* map, AllocationSpace space, + AllocationSite* allocation_site = NULL); // Allocates a JS Map in the heap. // Returns Failure::RetryAfterGC(requested_bytes, space) if the allocation diff --git a/src/runtime.cc b/src/runtime.cc index a1932b5..27e5577 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -14800,8 +14800,26 @@ static MaybeObject* ArrayConstructorCommon(Isolate* isolate, site->SetElementsKind(to_kind); } - maybe_array = isolate->heap()->AllocateJSObjectWithAllocationSite( - *constructor, site); + // We should allocate with an initial map that reflects the allocation site + // advice. Therefore we use AllocateJSObjectFromMap instead of passing + // the constructor. + Map* initial_map = constructor->initial_map(); + if (to_kind != initial_map->elements_kind()) { + MaybeObject* maybe_new_map = initial_map->AsElementsKind(to_kind); + if (!maybe_new_map->To(&initial_map)) return maybe_new_map; + } + + // If we don't care to track arrays of to_kind ElementsKind, then + // don't emit a memento for them. + AllocationSite* allocation_site = + (AllocationSite::GetMode(to_kind) == TRACK_ALLOCATION_SITE) + ? *site + : NULL; + + maybe_array = isolate->heap()->AllocateJSObjectFromMap(initial_map, + NOT_TENURED, + true, + allocation_site); if (!maybe_array->To(&array)) return maybe_array; } else { maybe_array = isolate->heap()->AllocateJSObject(*constructor); -- 2.7.4