[builtins] Squeeze JSPromise::result and JSPromise::reactions into a single field.

A given JSPromise can either be in pending state, and accumulates
reactions, or in settled state, where all reactions are scheduled
as microtasks, and it carries a result. So we can use a single field
on the JSPromise instance to hold both the result and the reactions
and that field is interpreted differently depending on the status of
the JSPromise.

Bug: v8:7253
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I19a7d499c88f452f0d35979ab95deb110021cde9
Reviewed-on: https://chromium-review.googlesource.com/895528Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51004}

src/builtins/builtins-promise-gen.cc

@@ -22,21 +22,26 @@ Node* PromiseBuiltinsAssembler::AllocateJSPromise(Node* context) {
   Node* const promise_fun =
       LoadContextElement(native_context, Context::PROMISE_FUNCTION_INDEX);
   CSA_ASSERT(this, IsFunctionWithPrototypeSlotMap(LoadMap(promise_fun)));
-  Node* const initial_map =
+  Node* const promise_map =
       LoadObjectField(promise_fun, JSFunction::kPrototypeOrInitialMapOffset);
-  Node* const instance = AllocateJSObjectFromMap(initial_map);
-  return instance;
+  Node* const promise = Allocate(JSPromise::kSizeWithEmbedderFields);
+  StoreMapNoWriteBarrier(promise, promise_map);
+  StoreObjectFieldRoot(promise, JSPromise::kPropertiesOrHashOffset,
+                       Heap::kEmptyFixedArrayRootIndex);
+  StoreObjectFieldRoot(promise, JSPromise::kElementsOffset,
+                       Heap::kEmptyFixedArrayRootIndex);
+  return promise;
 }
 
 void PromiseBuiltinsAssembler::PromiseInit(Node* promise) {
   STATIC_ASSERT(v8::Promise::kPending == 0);
-  StoreObjectFieldNoWriteBarrier(promise, JSPromise::kReactionsOffset,
+  StoreObjectFieldNoWriteBarrier(promise, JSPromise::kReactionsOrResultOffset,
                                  SmiConstant(Smi::kZero));
   StoreObjectFieldNoWriteBarrier(promise, JSPromise::kFlagsOffset,
                                  SmiConstant(Smi::kZero));
   for (int i = 0; i < v8::Promise::kEmbedderFieldCount; i++) {
     int offset = JSPromise::kSize + i * kPointerSize;
-    StoreObjectFieldNoWriteBarrier(promise, offset, SmiConstant(0));
+    StoreObjectFieldNoWriteBarrier(promise, offset, SmiConstant(Smi::kZero));
   }
 }
 
@@ -60,12 +65,12 @@ Node* PromiseBuiltinsAssembler::AllocateAndInitJSPromise(Node* context,
 
 Node* PromiseBuiltinsAssembler::AllocateAndSetJSPromise(
     Node* context, v8::Promise::PromiseState status, Node* result) {
-  Node* const instance = AllocateJSPromise(context);
+  DCHECK_NE(Promise::kPending, status);
 
-  StoreObjectFieldNoWriteBarrier(instance, JSPromise::kResultOffset, result);
+  Node* const instance = AllocateJSPromise(context);
+  StoreObjectFieldNoWriteBarrier(instance, JSPromise::kReactionsOrResultOffset,
+                                 result);
   STATIC_ASSERT(JSPromise::kStatusShift == 0);
-  StoreObjectFieldNoWriteBarrier(instance, JSPromise::kReactionsOffset,
-                                 SmiConstant(Smi::kZero));
   StoreObjectFieldNoWriteBarrier(instance, JSPromise::kFlagsOffset,
                                  SmiConstant(status));
   for (int i = 0; i < v8::Promise::kEmbedderFieldCount; i++) {
@@ -393,11 +398,11 @@ void PromiseBuiltinsAssembler::InternalPerformPromiseThen(Node* context,
     // Once the {promise} is resolved we decide on the concrete handler to
     // push onto the microtask queue.
     Node* const promise_reactions =
-        LoadObjectField(promise, JSPromise::kReactionsOffset);
+        LoadObjectField(promise, JSPromise::kReactionsOrResultOffset);
     Node* const reaction = AllocatePromiseReaction(promise_reactions, result,
                                                    var_on_fulfilled.value(),
                                                    var_on_rejected.value());
-    StoreObjectField(promise, JSPromise::kReactionsOffset, reaction);
+    StoreObjectField(promise, JSPromise::kReactionsOrResultOffset, reaction);
     Goto(&done);
   }
 
@@ -428,7 +433,8 @@ void PromiseBuiltinsAssembler::InternalPerformPromiseThen(Node* context,
     }
 
     BIND(&enqueue);
-    Node* argument = LoadObjectField(promise, JSPromise::kResultOffset);
+    Node* argument =
+        LoadObjectField(promise, JSPromise::kReactionsOrResultOffset);
     Node* microtask = AllocatePromiseReactionJobTask(
         var_map.value(), context, argument, var_handler.value(), result);
     CallBuiltin(Builtins::kEnqueueMicrotask, NoContextConstant(), microtask);
@@ -583,7 +589,7 @@ void PromiseBuiltinsAssembler::InternalResolvePromise(Node* context,
   {
     Node* const thenable_status = PromiseStatus(result);
     Node* const thenable_value =
-        LoadObjectField(result, JSPromise::kResultOffset);
+        LoadObjectField(result, JSPromise::kReactionsOrResultOffset);
 
     Label if_isnotpending(this);
     GotoIfNot(IsPromiseStatus(thenable_status, v8::Promise::kPending),
@@ -714,7 +720,7 @@ void PromiseBuiltinsAssembler::PromiseFulfill(
   Node* reactions;
   {
     VARIABLE(var_current, MachineRepresentation::kTagged,
-             LoadObjectField(promise, JSPromise::kReactionsOffset));
+             LoadObjectField(promise, JSPromise::kReactionsOrResultOffset));
     VARIABLE(var_reversed, MachineRepresentation::kTagged,
              SmiConstant(Smi::kZero));
 
@@ -735,9 +741,7 @@ void PromiseBuiltinsAssembler::PromiseFulfill(
   }
 
   // Reset the {promise}.
-  StoreObjectField(promise, JSPromise::kResultOffset, result);
-  StoreObjectFieldNoWriteBarrier(promise, JSPromise::kReactionsOffset,
-                                 SmiConstant(Smi::kZero));
+  StoreObjectField(promise, JSPromise::kReactionsOrResultOffset, result);
   PromiseSetStatus(promise, status);
 
   // Morph the {reactions} into PromiseReactionJobTasks and push them

src/compiler/js-create-lowering.cc

@@ -1013,14 +1013,12 @@ Reduction JSCreateLowering::ReduceJSCreatePromise(Node* node) {
           jsgraph()->EmptyFixedArrayConstant());
   a.Store(AccessBuilder::ForJSObjectElements(),
           jsgraph()->EmptyFixedArrayConstant());
-  a.Store(AccessBuilder::ForJSObjectOffset(JSPromise::kResultOffset),
-          jsgraph()->UndefinedConstant());
-  a.Store(AccessBuilder::ForJSObjectOffset(JSPromise::kReactionsOffset),
+  a.Store(AccessBuilder::ForJSObjectOffset(JSPromise::kReactionsOrResultOffset),
           jsgraph()->ZeroConstant());
   STATIC_ASSERT(v8::Promise::kPending == 0);
   a.Store(AccessBuilder::ForJSObjectOffset(JSPromise::kFlagsOffset),
           jsgraph()->ZeroConstant());
-  STATIC_ASSERT(JSPromise::kSize == 6 * kPointerSize);
+  STATIC_ASSERT(JSPromise::kSize == 5 * kPointerSize);
   for (int i = 0; i < v8::Promise::kEmbedderFieldCount; ++i) {
     a.Store(
         AccessBuilder::ForJSObjectOffset(JSPromise::kSize + i * kPointerSize),

src/objects-debug.cc

@@ -1125,10 +1125,11 @@ void PromiseReaction::PromiseReactionVerify() {
 void JSPromise::JSPromiseVerify() {
   CHECK(IsJSPromise());
   JSObjectVerify();
-  VerifyPointer(result());
-  VerifyPointer(reactions());
-  CHECK(reactions()->IsSmi() || reactions()->IsPromiseReaction());
+  VerifyPointer(reactions_or_result());
   VerifySmiField(kFlagsOffset);
+  if (status() == Promise::kPending) {
+    CHECK(reactions()->IsSmi() || reactions()->IsPromiseReaction());
+  }
 }
 
 template <typename Derived>

src/objects-inl.h

@@ -2835,12 +2835,20 @@ ACCESSORS(PromiseCapability, promise, HeapObject, kPromiseOffset)
 ACCESSORS(PromiseCapability, resolve, Object, kResolveOffset)
 ACCESSORS(PromiseCapability, reject, Object, kRejectOffset)
 
-ACCESSORS(JSPromise, result, Object, kResultOffset)
-ACCESSORS(JSPromise, reactions, Object, kReactionsOffset)
+ACCESSORS(JSPromise, reactions_or_result, Object, kReactionsOrResultOffset)
 SMI_ACCESSORS(JSPromise, flags, kFlagsOffset)
 BOOL_ACCESSORS(JSPromise, flags, has_handler, kHasHandlerBit)
 BOOL_ACCESSORS(JSPromise, flags, handled_hint, kHandledHintBit)
 
+Object* JSPromise::result() const {
+  DCHECK_NE(Promise::kPending, status());
+  return reactions_or_result();
+}
+
+Object* JSPromise::reactions() const {
+  DCHECK_EQ(Promise::kPending, status());
+  return reactions_or_result();
+}
 
 ElementsKind JSObject::GetElementsKind() {
   ElementsKind kind = map()->elements_kind();

src/objects-printer.cc

@@ -552,8 +552,11 @@ void JSArray::JSArrayPrint(std::ostream& os) {  // NOLINT
 void JSPromise::JSPromisePrint(std::ostream& os) {  // NOLINT
   JSObjectPrintHeader(os, this, "JSPromise");
   os << "\n - status = " << JSPromise::Status(status());
+  if (status() == Promise::kPending) {
+    os << "\n - reactions = " << Brief(reactions());
+  } else {
     os << "\n - result = " << Brief(result());
-  os << "\n - reactions: " << Brief(reactions());
+  }
   os << "\n - has_handler = " << has_handler();
   os << "\n ";
 }

src/objects.h

@@ -3998,13 +3998,27 @@ class JSMessageObject: public JSObject {
   typedef BodyDescriptor BodyDescriptorWeak;
 };
 
+// Representation of promise objects in the specification. Our layout of
+// JSPromise differs a bit from the layout in the specification, for example
+// there's only a single list of PromiseReaction objects, instead of separate
+// lists for fulfill and reject reactions. The PromiseReaction carries both
+// callbacks from the start, and is eventually morphed into the proper kind of
+// PromiseReactionJobTask when the JSPromise is settled.
+//
+// We also overlay the result and reactions fields on the JSPromise, since
+// the reactions are only necessary for pending promises, whereas the result
+// is only meaningful for settled promises.
 class JSPromise : public JSObject {
  public:
-  // TODO(bmeurer): Overlay {result} and {reactions}!
-  DECL_ACCESSORS(result, Object)
+  // [reactions_or_result]: Smi 0 terminated list of PromiseReaction objects
+  // in case the JSPromise was not settled yet, otherwise the result.
+  DECL_ACCESSORS(reactions_or_result, Object)
+
+  // [result]: Checks that the promise is settled and returns the result.
+  inline Object* result() const;
 
-  // [reactions]: Smi 0 terminated list of PromiseReaction objects.
-  DECL_ACCESSORS(reactions, Object)
+  // [reactions]: Checks that the promise is pending and returns the reactions.
+  inline Object* reactions() const;
 
   DECL_INT_ACCESSORS(flags)
 
@@ -4031,9 +4045,8 @@ class JSPromise : public JSObject {
   DECL_VERIFIER(JSPromise)
 
   // Layout description.
-  static const int kResultOffset = JSObject::kHeaderSize;
-  static const int kReactionsOffset = kResultOffset + kPointerSize;
-  static const int kFlagsOffset = kReactionsOffset + kPointerSize;
+  static const int kReactionsOrResultOffset = JSObject::kHeaderSize;
+  static const int kFlagsOffset = kReactionsOrResultOffset + kPointerSize;
   static const int kSize = kFlagsOffset + kPointerSize;
   static const int kSizeWithEmbedderFields =
       kSize + v8::Promise::kEmbedderFieldCount * kPointerSize;

src/profiler/heap-snapshot-generator.cc

@@ -1312,10 +1312,9 @@ void V8HeapExplorer::ExtractJSArrayBufferReferences(
 }
 
 void V8HeapExplorer::ExtractJSPromiseReferences(int entry, JSPromise* promise) {
-  SetInternalReference(promise, entry, "result", promise->result(),
-                       JSPromise::kResultOffset);
-  SetInternalReference(promise, entry, "reactions", promise->reactions(),
-                       JSPromise::kReactionsOffset);
+  SetInternalReference(promise, entry, "reactions_or_result",
+                       promise->reactions_or_result(),
+                       JSPromise::kReactionsOrResultOffset);
 }
 
 void V8HeapExplorer::ExtractFixedArrayReferences(int entry, FixedArray* array) {

src/runtime/runtime-debug.cc

@@ -265,7 +265,10 @@ MaybeHandle<JSArray> Runtime::GetInternalProperties(Isolate* isolate,
     Handle<String> status_str = factory->NewStringFromAsciiChecked(status);
     result->set(1, *status_str);
 
-    Handle<Object> value_obj(promise->result(), isolate);
+    Handle<Object> value_obj(promise->status() == Promise::kPending
+                                 ? isolate->heap()->undefined_value()
+                                 : promise->result(),
+                             isolate);
     Handle<String> promise_value =
         factory->NewStringFromAsciiChecked("[[PromiseValue]]");
     result->set(2, *promise_value);

test/cctest/test-heap-profiler.cc

@@ -2769,17 +2769,17 @@ TEST(JSPromise) {
   const v8::HeapGraphNode* resolved = GetProperty(
       env->GetIsolate(), global, v8::HeapGraphEdge::kProperty, "resolved");
   CHECK(GetProperty(env->GetIsolate(), resolved, v8::HeapGraphEdge::kInternal,
-                    "result"));
+                    "reactions_or_result"));
 
   const v8::HeapGraphNode* rejected = GetProperty(
       env->GetIsolate(), global, v8::HeapGraphEdge::kProperty, "rejected");
   CHECK(GetProperty(env->GetIsolate(), rejected, v8::HeapGraphEdge::kInternal,
-                    "result"));
+                    "reactions_or_result"));
 
   const v8::HeapGraphNode* pending = GetProperty(
       env->GetIsolate(), global, v8::HeapGraphEdge::kProperty, "pending");
   CHECK(GetProperty(env->GetIsolate(), pending, v8::HeapGraphEdge::kInternal,
-                    "reactions"));
+                    "reactions_or_result"));
 
   const char* objectNames[] = {"resolved", "rejected", "pending", "chained"};
   for (auto objectName : objectNames) {