Bug 1598347, part 2: pass "inner singleton" to NEWOBJECT group logic. r=djvj,iain
authorChris Fallin <cfallin@mozilla.com>
Wed, 27 Nov 2019 22:49:33 +0000
changeset 504902 7bd0784e7a6f3702152bdca395b2305f16ca0479
parent 504901 6da3c648995f15cacd586ebb3c8022a690c9649f
child 504903 953b1e11b4017f21648c839a10e945df419d0105
push id36873
push usershindli@mozilla.com
push dateTue, 03 Dec 2019 09:48:30 +0000
treeherdermozilla-central@8e08311f2d87 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdjvj, iain
bugs1598347
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1598347, part 2: pass "inner singleton" to NEWOBJECT group logic. r=djvj,iain This change passes through the "inner singleton" status of a particular object literal to the group-assignment logic for JSOP_NEWOBJECT by instead using a variant opcode, JSOP_NEWOBJECT_WITHGROUP. "Inner singleton" status is meant to emulate old behavior (prior to ObjLiteral) in which the frontend built an entire tree of objects/arrays with a top-level singleton. In the old world, these objects were allocated by ParseNode::getConstantValue() in the frontend and built by ObjectGroup::newPlainObject, which looked up an object group by property names. In the new world, the default NEWOBJECT logic uses a separate test for whether an allocation site should have a singleton group, and even after we carefully emulate the old group behavior in the frontend, the new-object allocation itself will decide to allocate singleton groups. In the particular regression case motivating this change, these singleton groups break the TI information for a singleton array (typeset for all elems no longer has a single group) and as a result, a simple property access can no longer be inlined. This patch matches the old behavior and allows the inlining to occur. Differential Revision: https://phabricator.services.mozilla.com/D54609
js/src/debugger/Script.cpp
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/ObjLiteral.h
js/src/jit/BaselineCodeGen.cpp
js/src/jit/BaselineCodeGen.h
js/src/jit/BaselineIC.cpp
js/src/jit/IonBuilder.cpp
js/src/vm/BytecodeUtil.cpp
js/src/vm/Interpreter.cpp
js/src/vm/Opcodes.h
--- a/js/src/debugger/Script.cpp
+++ b/js/src/debugger/Script.cpp
@@ -1575,16 +1575,17 @@ static bool BytecodeIsEffectful(JSOp op)
     case JSOP_UNINITIALIZED:
     case JSOP_POP:
     case JSOP_POPN:
     case JSOP_DUPAT:
     case JSOP_NEWARRAY:
     case JSOP_NEWARRAY_COPYONWRITE:
     case JSOP_NEWINIT:
     case JSOP_NEWOBJECT:
+    case JSOP_NEWOBJECT_WITHGROUP:
     case JSOP_INITELEM:
     case JSOP_INITHIDDENELEM:
     case JSOP_INITELEM_INC:
     case JSOP_INITELEM_ARRAY:
     case JSOP_INITPROP:
     case JSOP_INITLOCKEDPROP:
     case JSOP_INITHIDDENPROP:
     case JSOP_INITPROP_GETTER:
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -8157,18 +8157,22 @@ bool BytecodeEmitter::emitPropertyListOb
     }
   }
 
   uint32_t gcThingIndex = 0;
   if (!perScriptData().gcThingList().append(std::move(data), &gcThingIndex)) {
     return false;
   }
 
-  bool success = singleton ? emitIndex32(JSOP_OBJECT, gcThingIndex)
-                           : emitIndex32(JSOP_NEWOBJECT, gcThingIndex);
+  bool isInnerSingleton = flags.contains(ObjLiteralFlag::IsInnerSingleton);
+
+  JSOp op = singleton
+                ? JSOP_OBJECT
+                : isInnerSingleton ? JSOP_NEWOBJECT_WITHGROUP : JSOP_NEWOBJECT;
+  bool success = emitIndex32(op, gcThingIndex);
   if (!success) {
     return false;
   }
 
   bytecodeSection().setStackDepth(stackDepth + 1);
   return true;
 }
 
@@ -8540,16 +8544,19 @@ MOZ_NEVER_INLINE bool BytecodeEmitter::e
       flags += ObjLiteralFlag::SpecificGroup;
       if (!isInner) {
         flags += ObjLiteralFlag::Singleton;
       }
     }
     if (!useObjLiteralValues) {
       flags += ObjLiteralFlag::NoValues;
     }
+    if (isInner) {
+      flags += ObjLiteralFlag::IsInnerSingleton;
+    }
 
     // Use an ObjLiteral op. This will record ObjLiteral insns in the
     // objLiteralWriter's buffer and add a fixup to the list of ObjLiteral
     // fixups so that at GC-publish time at the end of parse, the full (case 1)
     // or template-without-values (case 2) object can be allocated and the
     // bytecode can be patched to refer to it.
     if (!emitPropertyListObjLiteral(objNode, ObjectLiteral, flags)) {
       //              [stack] OBJ
--- a/js/src/frontend/ObjLiteral.h
+++ b/js/src/frontend/ObjLiteral.h
@@ -12,16 +12,136 @@
 #include "mozilla/EnumSet.h"
 #include "mozilla/Span.h"
 
 #include "js/AllocPolicy.h"
 #include "js/GCPolicyAPI.h"
 #include "js/Value.h"
 #include "js/Vector.h"
 
+/*
+ * [SMDOC] ObjLiteral (Object Literal) Handling
+ * ============================================
+ *
+ * The `ObjLiteral*` family of classes defines an infastructure to handle
+ * object literals as they are encountered at parse time and translate them
+ * into objects that are attached to the bytecode.
+ *
+ * The object-literal "instructions", whose opcodes are defined in
+ * `ObjLiteralOpcode` below, each specify one key (atom property name, or
+ * numeric index) and one value. An `ObjLiteralWriter` buffers a linear
+ * sequence of such instructions, along with a side-table of atom references.
+ * The writer stores a compact binary format that is then interpreted by the
+ * `ObjLiteralReader` to construct an object according to the instructions.
+ *
+ * This may seem like an odd dance: create an intermediate data structure that
+ * specifies key/value pairs, then later build the object. Why not just do so
+ * directly, as we parse? In fact, we used to do this. However, for several
+ * good reasons, we want to avoid allocating or touching GC objects at all
+ * *during* the parse. We thus use a sequence of ObjLiteral instructions as an
+ * intermediate data structure to carry object literal contents from parse to
+ * the time at which we *can* allocate objects.
+ *
+ * (The original intent was to allow for ObjLiteral instructions to actually be
+ * invoked by a new JS opcode, JSOP_OBJLITERAL, thus replacing the more general
+ * opcode sequences sometimes generated to fill in objects and removing the
+ * need to attach actual objects to JSOP_OBJECT or JSOP_NEWOBJECT. However,
+ * this was far too invasive and led to performance regressions, so currently
+ * ObjLiteral only carries literals as far as the end of the parse pipeline,
+ * when all GC things are allocated.)
+ *
+ * ObjLiteral data structures are used to represent object literals whenever
+ * they are "compatible". See
+ * BytecodeEmitter::isPropertyListObjLiteralCompatible for the precise
+ * conditions; in brief, we can represent object literals with "primitive"
+ * (numeric, boolean, string, null/undefined) values, and "normal"
+ * (non-computed) object names. We can also represent arrays with the same
+ * value restrictions. We cannot represent nested objects. We use ObjLiteral in
+ * two different ways:
+ *
+ * - To build a template object, when we can support the properties but not the
+ *   keys.
+ * - To build the actual result object, when we support the properties and the
+ *   keys and this is a JSOP_OBJECT case (see below).
+ *
+ * Design and Performance Considerations
+ * -------------------------------------
+ *
+ * As a brief overview, there are a number of opcodes that allocate objects:
+ *
+ * - JSOP_NEWINIT allocates a new empty `{}` object.
+ *
+ * - JSOP_NEWOBJECT, with an object as an argument (held by the script data
+ *   side-tables), allocates a new object with `undefined` property values but
+ *   with a defined set of properties. The given object is used as a
+ *   *template*.
+ *
+ * - JSOP_NEWOBJECT_WITHGROUP (added as part of this ObjLiteral work), same as
+ *   above but uses the ObjectGroup of the template object for the new object,
+ *   rather than trying to apply a set of heuristics to choose a group.
+ *
+ * - JSOP_OBJECT, with an object as argument, instructs the runtime to
+ *   literally return the object argument as the result. This is thus only an
+ *   "allocation" in the sense that the object was originally allocated when
+ *   the script data / bytecode was created. It is only used when we know for
+ *   sure that the script, and this program point within the script, will run
+ *   *once*. (See the `treatAsRunOnce` flag on JSScript.)
+ *
+ * Before we go further, we also define "singleton context" and "singleton
+ * group". An operation occurs in a "singleton context", according to the
+ * parser, if it will only ever execute once. In particular, this happens when
+ * (i) the script is a "run-once" script, which is usually the case for e.g.
+ * top-level scripts of web-pages (they run on page load, but no function or
+ * handle wraps or refers to the script so it can't be invoked again),
+ * and (ii) the operation itself is not within a loop or function in that
+ * run-once script. "Singleton group", on the other hand, refers to an
+ * ObjectGroup (used by Type Inference) that represents only one object, and
+ * has a special flag set to mark it as such. Usually we want to give singleton
+ * groups to object allocations that happen in a singleton context (because
+ * there will only ever be one of the object), hence the connection between
+ * these terms.
+ *
+ * When we encounter an object literal, we decide which opcode to use, and we
+ * construct the ObjLiteral and the bytecode using its result appropriately:
+ *
+ * - If in a singleton context, and if we support the values, we use
+ *   JSOP_OBJECT and we build the ObjLiteral instructions with values.
+ * - Otherwise, if we support the keys but not the values, or if we are not
+ *   in a singleton context, we use JSOP_NEWOBJECT or JSOP_NEWOBJECT_WITHGROUP,
+ *   depending on the "inner singleton" status (see below). In this case, the
+ *   initial opcode only creates an object with empty values, so
+ *   BytecodeEmitter then generates bytecode to set the values
+ *   appropriately.
+ * - Otherwise, we generate JSOP_NEWINIT and bytecode to add properties one at
+ *   a time. This will always work, but is the slowest and least
+ *   memory-efficient option.
+ *
+ * We need to take special care to ensure that the ObjectGroup of the resulting
+ * object is chosen "correctly". Failure to do so can result in all sorts
+ * of performance and/or memory regressions. In brief, we want to use a
+ * singleton group whenever an object is allocated in a singleton context.
+ * However, there is a special "inner singleton" context that deserves special
+ * mention. When a program has a nested tree of objects, the old
+ * (pre-ObjLiteral) world would perform a group lookup by shape (list of
+ * property IDs) for all non-root objects, so in the following snippet, the
+ * inner objects would share a group:
+ *
+ *     var list = [{a: 0}, {a: 1}];
+ *     var obj = { first: {a: 0}, second: {a: 1} };
+ *
+ * In the generated bytecode, the inner objects are created first, then placed
+ * in the relevant properties of the outer objects/arrays using
+ * INITPROP/INITELEM. Thus to a naïve analysis, it appears that the inner
+ * objects are singletons. But heuristically it is better if they are not. So
+ * we pass down an `isInner` boolean while recursively traversing the
+ * parse-node tree and generating bytecode. If we encounter an object literal
+ * that is in singleton (run-once) context but also `isInner`, then we set
+ * special flags to ensure its shape is looked up based on properties instead.
+ */
+
 namespace js {
 
 // Object-literal instruction opcodes. An object literal is constructed by a
 // straight-line sequence of these ops, each adding one property to the
 // object.
 enum class ObjLiteralOpcode : uint8_t {
   INVALID = 0,
 
@@ -49,16 +169,23 @@ enum class ObjLiteralFlag : uint8_t {
   // rather than TenuredObject.
   Singleton = 3,
   // If set, the created array will be created as a COW array rather than a
   // normal array.
   ArrayCOW = 4,
 
   // No values are provided; the object is meant as a template object.
   NoValues = 5,
+
+  // This object is inside a top-level singleton, and so prior to ObjLiteral,
+  // would have been allocated at parse time, but is now allocated in bytecode.
+  // We do special things to get the right group on the template object; this
+  // flag indicates that if JSOP_NEWOBJECT copies the object, it should retain
+  // its group.
+  IsInnerSingleton = 6,
 };
 
 using ObjLiteralFlags = mozilla::EnumSet<ObjLiteralFlag>;
 
 inline bool ObjLiteralOpcodeHasValueArg(ObjLiteralOpcode op) {
   return op == ObjLiteralOpcode::ConstValue;
 }
 
--- a/js/src/jit/BaselineCodeGen.cpp
+++ b/js/src/jit/BaselineCodeGen.cpp
@@ -3053,28 +3053,31 @@ bool BaselineCodeGen<Handler>::emit_JSOP
 
   // Pop the rhs, so that the object is on the top of the stack.
   frame.pop();
   return true;
 }
 
 template <typename Handler>
 bool BaselineCodeGen<Handler>::emit_JSOP_NEWOBJECT() {
-  frame.syncStack(0);
-
-  if (!emitNextIC()) {
-    return false;
-  }
-
-  frame.push(R0);
-  return true;
+  return emitNewObject();
+}
+
+template <typename Handler>
+bool BaselineCodeGen<Handler>::emit_JSOP_NEWOBJECT_WITHGROUP() {
+  return emitNewObject();
 }
 
 template <typename Handler>
 bool BaselineCodeGen<Handler>::emit_JSOP_NEWINIT() {
+  return emitNewObject();
+}
+
+template <typename Handler>
+bool BaselineCodeGen<Handler>::emitNewObject() {
   frame.syncStack(0);
 
   if (!emitNextIC()) {
     return false;
   }
 
   frame.push(R0);
   return true;
--- a/js/src/jit/BaselineCodeGen.h
+++ b/js/src/jit/BaselineCodeGen.h
@@ -90,16 +90,17 @@ namespace jit {
   _(JSOP_DEFAULT)                   \
   _(JSOP_LINENO)                    \
   _(JSOP_BITNOT)                    \
   _(JSOP_NEG)                       \
   _(JSOP_NEWARRAY)                  \
   _(JSOP_NEWARRAY_COPYONWRITE)      \
   _(JSOP_INITELEM_ARRAY)            \
   _(JSOP_NEWOBJECT)                 \
+  _(JSOP_NEWOBJECT_WITHGROUP)       \
   _(JSOP_NEWINIT)                   \
   _(JSOP_INITELEM)                  \
   _(JSOP_INITELEM_GETTER)           \
   _(JSOP_INITELEM_SETTER)           \
   _(JSOP_INITELEM_INC)              \
   _(JSOP_MUTATEPROTO)               \
   _(JSOP_INITPROP)                  \
   _(JSOP_INITLOCKEDPROP)            \
@@ -439,16 +440,19 @@ class BaselineCodeGen {
   MOZ_MUST_USE bool emitUnaryArith();
 
   // JSOP_BITXOR, JSOP_LSH, JSOP_ADD etc.
   MOZ_MUST_USE bool emitBinaryArith();
 
   // Handles JSOP_LT, JSOP_GT, and friends
   MOZ_MUST_USE bool emitCompare();
 
+  // Handles JSOP_NEWOBJECT, JSOP_NEWOBJECT_WITHGROUP, and JSOP_NEWINIT.
+  MOZ_MUST_USE bool emitNewObject();
+
   // For a JOF_JUMP op, jumps to the op's jump target.
   void emitJump();
 
   // For a JOF_JUMP op, jumps to the op's jump target depending on the Value
   // in |val|.
   void emitTestBooleanTruthy(bool branchIfTrue, ValueOperand val);
 
   // Converts |val| to an index in the jump table and stores this in |dest|
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -319,16 +319,17 @@ bool JitScript::initICEntriesAndBytecode
         ICStub* stub =
             alloc.newStub<ICNewArray_Fallback>(Kind::NewArray, group);
         if (!addIC(loc, stub)) {
           return false;
         }
         break;
       }
       case JSOP_NEWOBJECT:
+      case JSOP_NEWOBJECT_WITHGROUP:
       case JSOP_NEWINIT: {
         ICStub* stub = alloc.newStub<ICNewObject_Fallback>(Kind::NewObject);
         if (!addIC(loc, stub)) {
           return false;
         }
         break;
       }
       case JSOP_INITELEM:
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -2305,16 +2305,17 @@ AbortReasonOr<Ok> IonBuilder::inspectOpc
     case JSOP_NEWARRAY:
       return jsop_newarray(GET_UINT32(pc));
 
     case JSOP_NEWARRAY_COPYONWRITE:
       return jsop_newarray_copyonwrite();
 
     case JSOP_NEWINIT:
     case JSOP_NEWOBJECT:
+    case JSOP_NEWOBJECT_WITHGROUP:
       return jsop_newobject();
 
     case JSOP_INITELEM:
     case JSOP_INITHIDDENELEM:
       return jsop_initelem();
 
     case JSOP_INITELEM_INC:
       return jsop_initelem_inc();
@@ -7411,17 +7412,18 @@ AbortReasonOr<Ok> IonBuilder::newObjectT
       trackOptimizationOutcome(TrackedOutcome::NoTemplateObject);
     }
     return Ok();
   }
 
   // Emit fastpath.
 
   MNewObject::Mode mode;
-  if (JSOp(*pc) == JSOP_NEWOBJECT || JSOp(*pc) == JSOP_NEWINIT) {
+  if (JSOp(*pc) == JSOP_NEWOBJECT || JSOp(*pc) == JSOP_NEWOBJECT_WITHGROUP ||
+      JSOp(*pc) == JSOP_NEWINIT) {
     mode = MNewObject::ObjectLiteral;
   } else {
     mode = MNewObject::ObjectCreate;
   }
 
   gc::InitialHeap heap = templateObject->group()->initialHeap(constraints());
   MConstant* templateConst =
       MConstant::NewConstraintlessObject(alloc(), templateObject);
@@ -7439,17 +7441,19 @@ AbortReasonOr<Ok> IonBuilder::newObjectT
   }
   *emitted = true;
   return Ok();
 }
 
 AbortReasonOr<Ok> IonBuilder::newObjectTryVM(bool* emitted,
                                              JSObject* templateObject) {
   // Emit a VM call.
-  MOZ_ASSERT(JSOp(*pc) == JSOP_NEWOBJECT || JSOp(*pc) == JSOP_NEWINIT);
+  MOZ_ASSERT(JSOp(*pc) == JSOP_NEWOBJECT ||
+             JSOp(*pc) == JSOP_NEWOBJECT_WITHGROUP ||
+             JSOp(*pc) == JSOP_NEWINIT);
 
   trackOptimizationAttempt(TrackedStrategy::NewObject_Call);
 
   gc::InitialHeap heap = gc::DefaultHeap;
   MConstant* templateConst = MConstant::New(alloc(), NullValue());
 
   if (templateObject) {
     heap = templateObject->group()->initialHeap(constraints());
--- a/js/src/vm/BytecodeUtil.cpp
+++ b/js/src/vm/BytecodeUtil.cpp
@@ -2076,16 +2076,17 @@ bool ExpressionDecompiler::decompilePC(j
         MOZ_ASSERT(defIndex == 1);
         return write("MOREITER");
 
       case JSOP_MUTATEPROTO:
         return write("SUCCEEDED");
 
       case JSOP_NEWINIT:
       case JSOP_NEWOBJECT:
+      case JSOP_NEWOBJECT_WITHGROUP:
       case JSOP_OBJWITHPROTO:
         return write("OBJ");
 
       case JSOP_OPTIMIZE_SPREADCALL:
         // For stack dump, defIndex == 0 is not used.
         MOZ_ASSERT(defIndex == 1);
         return write("OPTIMIZED");
 
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -3790,17 +3790,18 @@ static MOZ_NEVER_INLINE JS_HAZ_JSNATIVE_
       if (!obj) {
         goto error;
       }
 
       PUSH_OBJECT(*obj);
     }
     END_CASE(JSOP_NEWARRAY_COPYONWRITE)
 
-    CASE(JSOP_NEWOBJECT) {
+    CASE(JSOP_NEWOBJECT)
+    CASE(JSOP_NEWOBJECT_WITHGROUP) {
       JSObject* obj = NewObjectOperation(cx, script, REGS.pc);
       if (!obj) {
         goto error;
       }
       PUSH_OBJECT(*obj);
     }
     END_CASE(JSOP_NEWOBJECT)
 
@@ -5190,19 +5191,39 @@ bool js::OptimizeSpreadCall(JSContext* c
 
   return stubChain->tryOptimizeArray(cx, obj.as<ArrayObject>(), optimized);
 }
 
 JSObject* js::NewObjectOperation(JSContext* cx, HandleScript script,
                                  jsbytecode* pc,
                                  NewObjectKind newKind /* = GenericObject */) {
   MOZ_ASSERT(newKind != SingletonObject);
+  bool withTemplate =
+      (*pc == JSOP_NEWOBJECT || *pc == JSOP_NEWOBJECT_WITHGROUP);
+  bool withTemplateGroup = (*pc == JSOP_NEWOBJECT_WITHGROUP);
 
   RootedObjectGroup group(cx);
-  if (ObjectGroup::useSingletonForAllocationSite(script, pc, JSProto_Object)) {
+  RootedPlainObject baseObject(cx);
+
+  // Extract the template object, if one exists.
+  if (withTemplate) {
+    baseObject = &script->getObject(pc)->as<PlainObject>();
+  }
+
+  // Choose the group. Three cases:
+  // - JSOP_NEWOBJECT_WITHGROUP explicitly indicates that we should use the
+  //   same group as the template object's group.
+  // - otherwise, if some heuristics indicate that we should use a singleton,
+  //   we set the allocation-kind to ensure this.
+  // - otherwise, we look up a group based on the allocation site, i.e., the
+  //   (script, pc) tuple.
+  if (withTemplateGroup) {
+    group = baseObject->getGroup(cx, baseObject);
+  } else if (ObjectGroup::useSingletonForAllocationSite(script, pc,
+                                                        JSProto_Object)) {
     newKind = SingletonObject;
   } else {
     group = ObjectGroup::allocationSiteGroup(cx, script, pc, JSProto_Object);
     if (!group) {
       return nullptr;
     }
 
     {
@@ -5215,37 +5236,39 @@ JSObject* js::NewObjectOperation(JSConte
           group->maybePreliminaryObjects(sweep)) {
         newKind = TenuredObject;
       }
     }
   }
 
   RootedPlainObject obj(cx);
 
-  if (*pc == JSOP_NEWOBJECT) {
-    RootedPlainObject baseObject(cx, &script->getObject(pc)->as<PlainObject>());
+  // Actually allocate the object.
+  if (withTemplate) {
     obj = CopyInitializerObject(cx, baseObject, newKind);
   } else {
     MOZ_ASSERT(*pc == JSOP_NEWINIT);
     obj = NewBuiltinClassInstance<PlainObject>(cx, newKind);
   }
 
   if (!obj) {
     return nullptr;
   }
 
   if (newKind == SingletonObject) {
     MOZ_ASSERT(obj->isSingleton());
   } else {
     obj->setGroup(group);
 
-    AutoSweepObjectGroup sweep(group);
-    if (PreliminaryObjectArray* preliminaryObjects =
-            group->maybePreliminaryObjects(sweep)) {
-      preliminaryObjects->registerNewObject(obj);
+    if (!IsInsideNursery(obj)) {
+      AutoSweepObjectGroup sweep(group);
+      if (PreliminaryObjectArray* preliminaryObjects =
+              group->maybePreliminaryObjects(sweep)) {
+        preliminaryObjects->registerNewObject(obj);
+      }
     }
   }
 
   return obj;
 }
 
 JSObject* js::NewObjectOperationWithTemplate(JSContext* cx,
                                              HandleObject templateObject) {
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -896,17 +896,20 @@
      *   Operands: uint32_t length
      *   Stack: => obj
      */ \
     MACRO(JSOP_NEWARRAY, 90, "newarray", NULL, 5, 0, 1, JOF_UINT32|JOF_IC) \
     /*
      * Pushes newly created object onto the stack.
      *
      * This opcode takes an object with the final shape, which can be set at
-     * the start and slots then filled in directly.
+     * the start and slots then filled in directly. We compute a group based on
+     * allocation site (or new group if the template's group is a singleton);
+     * see JSOP_NEWOBJECT_WITHGROUP for a variant that uses the same group as
+     * the template object.
      *
      *   Category: Literals
      *   Type: Object
      *   Operands: uint32_t baseobjIndex
      *   Stack: => obj
      */ \
     MACRO(JSOP_NEWOBJECT, 91, "newobject", NULL, 5, 0, 1, JOF_OBJECT|JOF_IC) \
     /*
@@ -2532,26 +2535,39 @@
      * If the value on top of the stack is not null or undefined, jumps to a 32-bit offset from the
      * current bytecode.
      *
      *   Category: Statements
      *   Type: Jumps
      *   Operands: int32_t offset
      *   Stack: cond => cond
      */ \
-    MACRO(JSOP_COALESCE, 241, "coalesce", NULL, 5, 1, 1, JOF_JUMP|JOF_DETECTING)
+    MACRO(JSOP_COALESCE, 241, "coalesce", NULL, 5, 1, 1, JOF_JUMP|JOF_DETECTING) \
+    /*
+     * Pushes newly created object onto the stack.
+     *
+     * This opcode takes an object with the final shape, which can be set at
+     * the start and slots then filled in directly. Uses the group from the
+     * template object; see JSOP_NEWOBJECT for a variant with different
+     * heuristics.
+     *
+     *   Category: Literals
+     *   Type: Object
+     *   Operands: uint32_t baseobjIndex
+     *   Stack: => obj
+     */ \
+    MACRO(JSOP_NEWOBJECT_WITHGROUP, 242, "newobjectwithgroup", NULL, 5, 0, 1, JOF_OBJECT|JOF_IC)
 
 // clang-format on
 
 /*
  * In certain circumstances it may be useful to "pad out" the opcode space to
  * a power of two.  Use this macro to do so.
  */
 #define FOR_EACH_TRAILING_UNUSED_OPCODE(MACRO) \
-  MACRO(242)                                   \
   MACRO(243)                                   \
   MACRO(244)                                   \
   MACRO(245)                                   \
   MACRO(246)                                   \
   MACRO(247)                                   \
   MACRO(248)                                   \
   MACRO(249)                                   \
   MACRO(250)                                   \