Refactoring to clean up duplicate code in Heap::Allocate methods.

src/heap.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 665 matching lines @@
     return old_data_space_->allocation_top_address();
   }
   Address* OldDataSpaceAllocationLimitAddress() {
     return old_data_space_->allocation_limit_address();
   }
 
   // Allocates and initializes a new JavaScript object based on a
   // 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. allocation_site does not influence the
+  // pretenure state.

[Hannes Payer (out of office), 2014/02/19 21:29:07]

What do you mean by "does not influence the pretenure state"?

[mvstanton, 2014/02/26 09:47:41]

I mean that the AllocationSite is not mined for information used to determine
the Space for the allocation.

Perhaps there should be some renaming of these methods. The way they function is
that a site is passed only so that a memento can be allocated with the object.
Maybe the argument should be called "site_for_memento."

[Hannes Payer (out of office), 2014/03/03 17:03:46]

Just drop the "allocation_site does not influence the pretenure state."

[mvstanton, 2014/03/04 09:52:02]

Done.

   // 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<AllocationSite> allocation_site);
+      PretenureFlag pretenure = NOT_TENURED,
+      AllocationSite* allocation_site = NULL);

[Hannes Payer (out of office), 2014/02/19 21:29:07]

What about implementing an AllocationMode, similar to HAllocationMode?

[Hannes Payer (out of office), 2014/02/19 21:29:07]

Why did you get rid of the Handle?

[mvstanton, 2014/02/26 09:47:41]

Because it's not a handlefied method and the other arguments to the method are
pointers. The use of a handle was asymmetrical, and sends a contradictory
message to the "raw" MaybeObject* style of the method.

[mvstanton, 2014/02/26 09:47:41]

I am thinking small with this CL, focused on eliminating duplicate code rather
than introducing new constructs.

[Hannes Payer (out of office), 2014/03/03 17:03:46]

OK, I guess we should implement an AllocationMode in a follow-up CL.

[mvstanton, 2014/03/04 09:52:02]

yep, thanks, will be done.

 
   MUST_USE_RESULT MaybeObject* AllocateJSModule(Context* context,
                                                 ScopeInfo* scope_info);
 
   // Allocate a JSArray with no elements
   MUST_USE_RESULT MaybeObject* AllocateEmptyJSArray(
       ElementsKind elements_kind,
       PretenureFlag pretenure = NOT_TENURED) {
     return AllocateJSArrayAndStorage(elements_kind, 0, 0,
                                      DONT_INITIALIZE_ARRAY_ELEMENTS,
@@ skipping 59 matching lines @@
   // Reinitialize an JSGlobalProxy based on a constructor.  The object
   // must have the same size as objects allocated using the
   // constructor.  The object is reinitialized and behaves as an
   // object that has been freshly allocated using the constructor.
   MUST_USE_RESULT MaybeObject* ReinitializeJSGlobalProxy(
       JSFunction* constructor, JSGlobalProxy* global);
 
   // 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. It doesn't influence pretenure state.

[Hannes Payer (out of office), 2014/02/19 21:29:07]

same comments as above

[mvstanton, 2014/02/26 09:47:41]

Indeed. And probably the fact that I have to explain this (unsuccessfully at
that) implies that the API is broken anyway!

[Hannes Payer (out of office), 2014/03/03 17:03:46]

Just drop the "It doesn't influence pretenure state."

[mvstanton, 2014/03/04 09:52:02]

Done.

   // 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<AllocationSite> 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<AllocationSite> 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
   // failed.
   // Please note this function does not perform a garbage collection.
   MUST_USE_RESULT MaybeObject* AllocateMap(
       InstanceType instance_type,
       int instance_size,
       ElementsKind elements_kind = TERMINAL_FAST_ELEMENTS_KIND);
 
@@ skipping 2315 matching lines @@
   DisallowHeapAllocation no_allocation;  // i.e. no gc allowed.
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(PathTracer);
 };
 #endif  // DEBUG
 
 } }  // namespace v8::internal
 
 #endif  // V8_HEAP_H_