Skip to content

Commit

Permalink
Literal object creation rework (#1734)
Browse files Browse the repository at this point in the history 
Improvements to literal object creation to ease implementation of "super":

* Simplify bytecode for `LITERAL_KEY_SET`
* Simplify bytecode for `LITERAL_NEW`
* Push the object on the stack before the props

Co-authored-by: Satish Srinivasan <[email protected]>
  • Loading branch information
@andreabergia @0xe
andreabergia and 0xe authored Nov 30, 2024
1 parent 83f2b81 commit 9e964fe
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 44 deletions.
11 changes: 5 additions & 6 deletions rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1319,10 +1319,9 @@ private void visitObjectLiteral(Node node, Node child) {
int nextLiteralIndex = literalIds.size();
literalIds.add(propertyIds);

addIndexOp(Icode_LITERAL_KEYS, nextLiteralIndex);
addIndexOp(Icode_LITERAL_NEW_OBJECT, nextLiteralIndex);
addUint8(hasAnyComputedProperty ? 1 : 0);
addIndexOp(Icode_LITERAL_NEW, count);
stackChange(3);
stackChange(4);

int i = 0;
while (child != null) {
Expand All @@ -1332,7 +1331,7 @@ private void visitObjectLiteral(Node node, Node child) {
// Will be a node of type Token.COMPUTED_PROPERTY wrapping the actual expression
Node computedPropertyNode = (Node) propertyId;
visitExpression(computedPropertyNode.first, 0);
addIndexOp(Icode_LITERAL_KEY_SET, i);
addIcode(Icode_LITERAL_KEY_SET);
stackChange(-1);
}

Expand All @@ -1344,15 +1343,15 @@ private void visitObjectLiteral(Node node, Node child) {

addToken(Token.OBJECTLIT);

stackChange(-2);
stackChange(-3);
}

private void visitArrayLiteral(Node node, Node child) {
int count = 0;
for (Node n = child; n != null; n = n.getNext()) {
++count;
}
addIndexOp(Icode_LITERAL_NEW, count);
addIndexOp(Icode_LITERAL_NEW_ARRAY, count);
stackChange(2);
while (child != null) {
visitLiteralValue(child);
Expand Down
16 changes: 8 additions & 8 deletions rhino/src/main/java/org/mozilla/javascript/Icode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ abstract class Icode {
Icode_INTNUMBER = Icode_SHORTNUMBER - 1,

// To create and populate array to hold values for [] and {} literals
Icode_LITERAL_NEW = Icode_INTNUMBER - 1,
Icode_LITERAL_SET = Icode_LITERAL_NEW - 1,
Icode_LITERAL_NEW_OBJECT = Icode_INTNUMBER - 1,
Icode_LITERAL_NEW_ARRAY = Icode_LITERAL_NEW_OBJECT - 1,
Icode_LITERAL_SET = Icode_LITERAL_NEW_ARRAY - 1,

// Array literal with skipped index like [1,,2]
Icode_SPARE_ARRAYLIT = Icode_LITERAL_SET - 1,
Expand Down Expand Up @@ -145,8 +146,7 @@ abstract class Icode {

// Call to GetTemplateLiteralCallSite
Icode_TEMPLATE_LITERAL_CALLSITE = Icode_REG_BIGINT4 - 1,
Icode_LITERAL_KEYS = Icode_TEMPLATE_LITERAL_CALLSITE - 1,
Icode_LITERAL_KEY_SET = Icode_LITERAL_KEYS - 1,
Icode_LITERAL_KEY_SET = Icode_TEMPLATE_LITERAL_CALLSITE - 1,

// Jump if stack head is null or undefined
Icode_IF_NULL_UNDEF = Icode_LITERAL_KEY_SET - 1,
Expand Down Expand Up @@ -234,8 +234,10 @@ static String bytecodeName(int bytecode) {
return "SHORTNUMBER";
case Icode_INTNUMBER:
return "INTNUMBER";
case Icode_LITERAL_NEW:
return "LITERAL_NEW";
case Icode_LITERAL_NEW_OBJECT:
return "LITERAL_NEW_OBJECT";
case Icode_LITERAL_NEW_ARRAY:
return "LITERAL_NEW_ARRAY";
case Icode_LITERAL_SET:
return "LITERAL_SET";
case Icode_SPARE_ARRAYLIT:
Expand Down Expand Up @@ -326,8 +328,6 @@ static String bytecodeName(int bytecode) {
return "LOAD_BIGINT4";
case Icode_TEMPLATE_LITERAL_CALLSITE:
return "TEMPLATE_LITERAL_CALLSITE";
case Icode_LITERAL_KEYS:
return "LITERAL_KEYS";
case Icode_LITERAL_KEY_SET:
return "LITERAL_KEY_SET";
case Icode_IF_NULL_UNDEF:
Expand Down
52 changes: 29 additions & 23 deletions rhino/src/main/java/org/mozilla/javascript/Interpreter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ static void dumpICode(InterpreterData idata) {
case Token.REGEXP:
out.println(tname + " " + idata.itsRegExpLiterals[indexReg]);
break;
case Icode_LITERAL_KEYS:
case Icode_LITERAL_NEW_OBJECT:
{
boolean copyArray = iCode[pc++] != 0;
Object[] keys = (Object[]) idata.literalIds[indexReg];
Expand Down Expand Up @@ -891,7 +891,7 @@ private static int bytecodeSpan(int bytecode) {
// line number
return 1 + 2;

case Icode_LITERAL_KEYS:
case Icode_LITERAL_NEW_OBJECT:
// make a copy or not flag
return 1 + 1;
}
Expand Down Expand Up @@ -2430,7 +2430,25 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
ScriptRuntime.getTemplateLiteralCallSite(
cx, frame.scope, templateLiterals, indexReg);
continue Loop;
case Icode_LITERAL_NEW:
case Icode_LITERAL_NEW_OBJECT:
{
// indexReg: index of constant with the keys
Object[] ids = (Object[]) frame.idata.literalIds[indexReg];
boolean copyArray = iCode[frame.pc] != 0;
++frame.pc;
++stackTop;
stack[stackTop] = cx.newObject(frame.scope);
++stackTop;
stack[stackTop] =
copyArray ? Arrays.copyOf(ids, ids.length) : ids;
++stackTop;
stack[stackTop] = new int[ids.length];
++stackTop;
stack[stackTop] = new Object[ids.length];
sDbl[stackTop] = 0;
continue Loop;
}
case Icode_LITERAL_NEW_ARRAY:
// indexReg: number of values in the literal
++stackTop;
stack[stackTop] = new int[indexReg];
Expand Down Expand Up @@ -2469,19 +2487,6 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
sDbl[stackTop] = i + 1;
continue Loop;
}
case Icode_LITERAL_KEYS:
{
Object[] ids = (Object[]) frame.idata.literalIds[indexReg];
++stackTop;
boolean copyArray = iCode[frame.pc] != 0;
++frame.pc;
if (copyArray) {
stack[stackTop] = Arrays.copyOf(ids, ids.length);
} else {
stack[stackTop] = ids;
}
continue Loop;
}

case Icode_LITERAL_KEY_SET:
{
Expand All @@ -2490,20 +2495,21 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
key = ScriptRuntime.wrapNumber(sDbl[stackTop]);
--stackTop;
Object[] ids = (Object[]) stack[stackTop - 2];
ids[indexReg] = key;
int i = (int) sDbl[stackTop];
ids[i] = key;
continue Loop;
}
case Token.OBJECTLIT:
{
Object[] data = (Object[]) stack[stackTop];
Object[] values = (Object[]) stack[stackTop];
--stackTop;
int[] getterSetters = (int[]) stack[stackTop];
--stackTop;
Object[] ids = (Object[]) stack[stackTop];
Object val =
ScriptRuntime.newObjectLiteral(
ids, data, getterSetters, cx, frame.scope);
stack[stackTop] = val;
Object[] keys = (Object[]) stack[stackTop];
--stackTop;
Scriptable object = (Scriptable) stack[stackTop];
ScriptRuntime.fillObjectLiteral(
object, keys, values, getterSetters, cx, frame.scope);
continue Loop;
}
case Token.ARRAYLIT:
Expand Down
28 changes: 24 additions & 4 deletions rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4815,23 +4815,44 @@ public static Scriptable newArrayLiteral(
* object literal is compiled. The next instance will be the version called from new code.
* <strong>This method only present for compatibility.</strong>
*
* @deprecated Use {@link #newObjectLiteral(Object[], Object[], int[], Context, Scriptable)}
* instead
* @deprecated Use {@link #fillObjectLiteral(Scriptable, Object[], Object[], int[], Context,
* Scriptable)} instead
*/
@Deprecated
public static Scriptable newObjectLiteral(
Object[] propertyIds, Object[] propertyValues, Context cx, Scriptable scope) {
// Passing null for getterSetters means no getters or setters
return newObjectLiteral(propertyIds, propertyValues, null, cx, scope);
Scriptable object = cx.newObject(scope);
fillObjectLiteral(object, propertyIds, propertyValues, null, cx, scope);
return object;
}

/**
* This method is here for backward compat with existing compiled code. <strong>This method only
* present for compatibility.</strong>
*
* @deprecated Use {@link #fillObjectLiteral(Scriptable, Object[], Object[], int[], Context,
* Scriptable)}
*/
@Deprecated
public static Scriptable newObjectLiteral(
Object[] propertyIds,
Object[] propertyValues,
int[] getterSetters,
Context cx,
Scriptable scope) {
Scriptable object = cx.newObject(scope);
fillObjectLiteral(object, propertyIds, propertyValues, getterSetters, cx, scope);
return object;
}

public static void fillObjectLiteral(
Scriptable object,
Object[] propertyIds,
Object[] propertyValues,
int[] getterSetters,
Context cx,
Scriptable scope) {
int end = propertyIds == null ? 0 : propertyIds.length;
for (int i = 0; i != end; ++i) {
Object id = propertyIds[i];
Expand Down Expand Up @@ -4868,7 +4889,6 @@ public static Scriptable newObjectLiteral(
so.setGetterOrSetter(key, index == null ? 0 : index, getterOrSetter, isSetter);
}
}
return object;
}

public static boolean isArrayObject(Object obj) {
Expand Down
17 changes: 14 additions & 3 deletions rhino/src/main/java/org/mozilla/javascript/optimizer/BodyCodegen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2266,6 +2266,15 @@ private void visitObjectLiteral(Node node, Node child, boolean topLevel) {
return;
}

cfw.addALoad(contextLocal);
cfw.addALoad(variableObjectLocal);
cfw.addInvoke(
ByteCode.INVOKEVIRTUAL,
"org/mozilla/javascript/Context",
"newObject",
"(Lorg/mozilla/javascript/Scriptable;)Lorg/mozilla/javascript/Scriptable;");
cfw.add(ByteCode.DUP);

addLoadProperty(node, child, properties, count);

// check if object literal actually has any getters or setters
Expand Down Expand Up @@ -2305,13 +2314,15 @@ private void visitObjectLiteral(Node node, Node child, boolean topLevel) {
cfw.addALoad(contextLocal);
cfw.addALoad(variableObjectLocal);
addScriptRuntimeInvoke(
"newObjectLiteral",
"([Ljava/lang/Object;"
"fillObjectLiteral",
"("
+ "Lorg/mozilla/javascript/Scriptable;"
+ "[Ljava/lang/Object;"
+ "[Ljava/lang/Object;"
+ "[I"
+ "Lorg/mozilla/javascript/Context;"
+ "Lorg/mozilla/javascript/Scriptable;"
+ ")Lorg/mozilla/javascript/Scriptable;");
+ ")V");
}

private void visitSpecialCall(Node node, int type, int specialType, Node child) {
Expand Down

0 comments on commit 9e964fe

Please sign in to comment.