Initialise BoundFunction arguments before construction

Summary:
We currently construct the `ArrayStorage` containing arguments to a
`BoundFunction` before constructing the `BoundFunction` itself, but
only actually populate it after the `BoundFunction` has been
constructed. In the presence of trimming, this ends up meaning that we
need to call `ensureCapacity` after allocating the `BoundFunction`, and
always update the `argStorage_` property of the newly constructed
`BoundFunction` anyway.

Instead, we can populate the `ArrayStorage` before constructing the
`BoundFunction`. This has the following advantages:

1. The `argStorage_` property is only set once, so there is only one
write barrier (and it is a cheaper type of barrier).
2. We don't have to worry about trimming or careful size management.
3. We don't have to worry about managing the exception returned by
`ensureCapacity`. There is only one place that may throw.

Reviewed By: mattbfb

Differential Revision: D39454308

fbshipit-source-id: 19f8a31b529564342d69c69403cef7bf0f7c93d1

lib/VM/Callable.cpp

@@ -503,40 +503,32 @@ CallResult<HermesValue> BoundFunction::create(
     ConstArgIterator argsWithThis) {
   unsigned argCount = argCountWithThis > 0 ? argCountWithThis - 1 : 0;
 
+  // Copy the arguments. If we don't have any, we must at least initialize
+  // 'this' to 'undefined'.
   auto arrRes = ArrayStorage::create(runtime, argCount + 1);
   if (LLVM_UNLIKELY(arrRes == ExecutionStatus::EXCEPTION)) {
     return ExecutionStatus::EXCEPTION;
   }
-  auto argStorageHandle = runtime.makeHandle<ArrayStorage>(*arrRes);
-
-  auto *cell = runtime.makeAFixed<BoundFunction>(
-      runtime,
-      Handle<JSObject>::vmcast(&runtime.functionPrototype),
-      runtime.getHiddenClassForPrototype(
-          runtime.functionPrototypeRawPtr, numOverlapSlots<BoundFunction>()),
-      target,
-      argStorageHandle);
-  auto selfHandle = JSObjectInit::initToHandle(runtime, cell);
-
-  // Copy the arguments. If we don't have any, we must at least initialize
-  // 'this' to 'undefined'.
-  MutableHandle<ArrayStorage> handle(
-      runtime, selfHandle->argStorage_.get(runtime));
-
-  // In case the storage was trimmed, make sure it has enough capacity.
-  ArrayStorage::ensureCapacity(handle, runtime, argCount + 1);
+  auto arrHandle = runtime.makeMutableHandle(vmcast<ArrayStorage>(*arrRes));
 
   if (argCountWithThis) {
     for (unsigned i = 0; i != argCountWithThis; ++i) {
-      ArrayStorage::push_back(handle, runtime, Handle<>(&argsWithThis[i]));
+      ArrayStorage::push_back(arrHandle, runtime, Handle<>(&argsWithThis[i]));
     }
   } else {
     // Don't need to worry about resizing since it was created with a capacity
     // of at least 1.
-    ArrayStorage::push_back(handle, runtime, Runtime::getUndefinedValue());
+    ArrayStorage::push_back(arrHandle, runtime, Runtime::getUndefinedValue());
   }
-  // Update the storage pointer in case push_back() needed to reallocate.
-  selfHandle->argStorage_.set(runtime, *handle, runtime.getHeap());
+
+  auto *cell = runtime.makeAFixed<BoundFunction>(
+      runtime,
+      Handle<JSObject>::vmcast(&runtime.functionPrototype),
+      runtime.getHiddenClassForPrototype(
+          runtime.functionPrototypeRawPtr, numOverlapSlots<BoundFunction>()),
+      target,
+      arrHandle);
+  auto selfHandle = JSObjectInit::initToHandle(runtime, cell);
 
   if (target->isLazy()) {
     // If the target is lazy we can make the bound function lazy.