From b9f5e9be1cc702cbb785c2cbde23de8c3b1151d8 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Fri, 4 Aug 2023 20:40:57 +0200 Subject: [PATCH] JS: optimize case when JS method returns java.lang.Object and then treated as JS object --- .../model/analysis/BaseTypeInference.java | 12 +++++++++- .../org/teavm/jso/impl/JSClassProcessor.java | 23 +++++++++++++------ .../jso/impl/JSObjectClassTransformer.java | 2 +- .../org/teavm/jso/impl/JSTypeInference.java | 22 +++++++++++++++++- .../org/teavm/jso/impl/JSValueMarshaller.java | 13 +++++++---- .../teavm/jso/impl/JSWrapperGenerator.java | 9 +++++++- .../org/teavm/jso/test/JSWrapperTest.java | 7 ++++++ 7 files changed, 73 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/teavm/model/analysis/BaseTypeInference.java b/core/src/main/java/org/teavm/model/analysis/BaseTypeInference.java index 0684b4d39..565e42ac7 100644 --- a/core/src/main/java/org/teavm/model/analysis/BaseTypeInference.java +++ b/core/src/main/java/org/teavm/model/analysis/BaseTypeInference.java @@ -160,6 +160,10 @@ public abstract class BaseTypeInference { protected abstract T elementType(T t); + protected T methodReturnType(MethodReference methodRef) { + return mapType(methodRef.getReturnType()); + } + private class InitialTypeVisitor extends AbstractInstructionVisitor { private GraphBuilder graphBuilder; private GraphBuilder arrayGraphBuilder; @@ -280,7 +284,7 @@ public abstract class BaseTypeInference { @Override public void visit(InvokeInstruction insn) { - type(insn.getReceiver(), insn.getMethod().getReturnType()); + type(insn.getReceiver(), methodReturnType(insn.getMethod())); } @Override @@ -338,5 +342,11 @@ public abstract class BaseTypeInference { } } } + + void type(Variable target, T type) { + if (target != null && type != null) { + types[target.getIndex()] = type; + } + } } } diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java b/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java index f4107a802..986fea14e 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java @@ -224,7 +224,7 @@ class JSClassProcessor { void processProgram(MethodHolder methodToProcess) { setCurrentProgram(methodToProcess.getProgram()); - types = new JSTypeInference(typeHelper, program, methodToProcess.getReference()); + types = new JSTypeInference(typeHelper, classSource, program, methodToProcess.getReference()); types.ensure(); for (int i = 0; i < program.basicBlockCount(); ++i) { var block = program.basicBlockAt(i); @@ -565,7 +565,8 @@ class JSClassProcessor { newInvoke.setArguments(newArgs.toArray(new Variable[0])); replacement.add(newInvoke); if (result != null) { - result = marshaller.unwrapReturnValue(callLocation, result, method.getResultType(), returnByRef); + result = marshaller.unwrapReturnValue(callLocation, result, method.getResultType(), returnByRef, + canBeOnlyJava(invoke.getReceiver())); copyVar(result, invoke.getReceiver(), invoke.getLocation()); } @@ -585,7 +586,8 @@ class JSClassProcessor { Variable result = invoke.getReceiver() != null ? program.createVariable() : null; addPropertyGet(propertyName, invoke.getInstance(), result, invoke.getLocation(), pure); if (result != null) { - result = marshaller.unwrapReturnValue(callLocation, result, method.getResultType(), false); + result = marshaller.unwrapReturnValue(callLocation, result, method.getResultType(), false, + canBeOnlyJava(invoke.getReceiver())); copyVar(result, invoke.getReceiver(), invoke.getLocation()); } return true; @@ -617,7 +619,8 @@ class JSClassProcessor { addIndexerGet(invoke.getInstance(), marshaller.wrapArgument(callLocation, invoke.getArguments().get(0), method.parameterType(0), false), result, invoke.getLocation()); if (result != null) { - result = marshaller.unwrapReturnValue(callLocation, result, method.getResultType(), false); + result = marshaller.unwrapReturnValue(callLocation, result, method.getResultType(), false, + canBeOnlyJava(invoke.getReceiver())); copyVar(result, invoke.getReceiver(), invoke.getLocation()); } return true; @@ -665,6 +668,11 @@ class JSClassProcessor { return true; } + private boolean canBeOnlyJava(Variable variable) { + var type = types.typeOf(variable); + return type != JSType.JS && type != JSType.MIXED; + } + private boolean processMethod(MethodReader method, CallLocation callLocation, InvokeInstruction invoke) { String name = method.getName(); @@ -699,7 +707,8 @@ class JSClassProcessor { newInvoke.setArguments(newArguments.toArray(new Variable[0])); replacement.add(newInvoke); if (result != null) { - result = marshaller.unwrapReturnValue(callLocation, result, method.getResultType(), false); + result = marshaller.unwrapReturnValue(callLocation, result, method.getResultType(), false, + canBeOnlyJava(invoke.getReceiver())); copyVar(result, invoke.getReceiver(), invoke.getLocation()); } @@ -880,12 +889,12 @@ class JSClassProcessor { replacement.clear(); if (!callee.hasModifier(ElementModifier.STATIC)) { insn.setInstance(marshaller.unwrapReturnValue(location, program.variableAt(paramIndex++), - ValueType.object(calleeRef.getClassName()), false)); + ValueType.object(calleeRef.getClassName()), false, true)); } Variable[] args = new Variable[callee.parameterCount()]; for (int i = 0; i < callee.parameterCount(); ++i) { args[i] = marshaller.unwrapReturnValue(location, program.variableAt(paramIndex++), - callee.parameterType(i), false); + callee.parameterType(i), false, true); } insn.setArguments(args); if (callee.getResultType() != ValueType.VOID) { diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JSObjectClassTransformer.java b/jso/impl/src/main/java/org/teavm/jso/impl/JSObjectClassTransformer.java index c8e45f009..42c3add50 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JSObjectClassTransformer.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JSObjectClassTransformer.java @@ -130,7 +130,7 @@ class JSObjectClassTransformer implements ClassHolderTransformer { for (int i = 0; i < method.parameterCount(); ++i) { variablesToPass[i] = marshaller.unwrapReturnValue(callLocation, variablesToPass[i], - method.parameterType(i), false); + method.parameterType(i), false, true); } basicBlock.addAll(marshallInstructions); diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JSTypeInference.java b/jso/impl/src/main/java/org/teavm/jso/impl/JSTypeInference.java index 44acf3bc8..1c83e9310 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JSTypeInference.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JSTypeInference.java @@ -15,6 +15,8 @@ */ package org.teavm.jso.impl; +import org.teavm.jso.JSBody; +import org.teavm.model.ClassReaderSource; import org.teavm.model.MethodReference; import org.teavm.model.Program; import org.teavm.model.ValueType; @@ -22,10 +24,12 @@ import org.teavm.model.analysis.BaseTypeInference; class JSTypeInference extends BaseTypeInference { private JSTypeHelper typeHelper; + private ClassReaderSource classes; - JSTypeInference(JSTypeHelper typeHelper, Program program, MethodReference reference) { + JSTypeInference(JSTypeHelper typeHelper, ClassReaderSource classes, Program program, MethodReference reference) { super(program, reference); this.typeHelper = typeHelper; + this.classes = classes; } @Override @@ -79,4 +83,20 @@ class JSTypeInference extends BaseTypeInference { protected JSType elementType(JSType jsType) { return jsType instanceof JSType.ArrayType ? ((JSType.ArrayType) jsType).elementType : JSType.MIXED; } + + @Override + protected JSType methodReturnType(MethodReference methodRef) { + if (!methodRef.getReturnType().isObject(Object.class)) { + return mapType(methodRef.getReturnType()); + } + return isJsMethod(methodRef) ? JSType.MIXED : JSType.JAVA; + } + + private boolean isJsMethod(MethodReference methodRef) { + if (typeHelper.isJavaScriptClass(methodRef.getClassName())) { + return true; + } + var method = classes.resolveImplementation(methodRef); + return method != null && method.getAnnotations().get(JSBody.class.getName()) != null; + } } diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JSValueMarshaller.java b/jso/impl/src/main/java/org/teavm/jso/impl/JSValueMarshaller.java index 1ca9f8302..e88b9413c 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JSValueMarshaller.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JSValueMarshaller.java @@ -37,6 +37,10 @@ import org.teavm.model.instructions.InvokeInstruction; import org.teavm.model.instructions.StringConstantInstruction; class JSValueMarshaller { + private static final MethodReference JS_TO_JAVA = new MethodReference(JSWrapper.class, "jsToJava", + JSObject.class, Object.class); + private static final MethodReference LIGHTWEIGHT_JS_TO_JAVA = new MethodReference(JSWrapper.class, + "dependencyJsToJava", JSObject.class, Object.class); private static final ValueType stringType = ValueType.parse(String.class); private ReferenceCache referenceCache = new ReferenceCache(); private Diagnostics diagnostics; @@ -225,7 +229,8 @@ class JSValueMarshaller { return JSMethods.ARRAY_WRAPPER; } - Variable unwrapReturnValue(CallLocation location, Variable var, ValueType type, boolean byRef) { + Variable unwrapReturnValue(CallLocation location, Variable var, ValueType type, boolean byRef, + boolean strictJava) { if (byRef) { return unwrapByRef(location, var, type); } @@ -237,7 +242,7 @@ class JSValueMarshaller { return unwrapFunctor(location, var, cls); } } - return unwrap(location, var, type); + return unwrap(location, var, type, strictJava); } private Variable unwrapByRef(CallLocation location, Variable var, ValueType type) { @@ -263,7 +268,7 @@ class JSValueMarshaller { return invokeMethod(location, JSMethods.DATA_TO_ARRAY, var); } - Variable unwrap(CallLocation location, Variable var, ValueType type) { + Variable unwrap(CallLocation location, Variable var, ValueType type, boolean strictJava) { if (type instanceof ValueType.Primitive) { switch (((ValueType.Primitive) type).getKind()) { case BOOLEAN: @@ -296,7 +301,7 @@ class JSValueMarshaller { var wrapNative = new InvokeInstruction(); wrapNative.setLocation(location.getSourceLocation()); wrapNative.setType(InvocationType.SPECIAL); - wrapNative.setMethod(new MethodReference(JSWrapper.class, "jsToJava", JSObject.class, Object.class)); + wrapNative.setMethod(strictJava ? JS_TO_JAVA : LIGHTWEIGHT_JS_TO_JAVA); wrapNative.setArguments(var); wrapNative.setReceiver(program.createVariable()); replacement.add(wrapNative); diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JSWrapperGenerator.java b/jso/impl/src/main/java/org/teavm/jso/impl/JSWrapperGenerator.java index 0fe4a6758..85a79e499 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JSWrapperGenerator.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JSWrapperGenerator.java @@ -16,6 +16,7 @@ package org.teavm.jso.impl; import java.io.IOException; +import org.teavm.backend.javascript.rendering.Precedence; import org.teavm.backend.javascript.spi.Injector; import org.teavm.backend.javascript.spi.InjectorContext; import org.teavm.dependency.DependencyAgent; @@ -36,11 +37,17 @@ public class JSWrapperGenerator implements Injector, DependencyPlugin { case "dependencyJsToJava": case "wrapperToJs": case "jsToWrapper": - context.writeExpr(context.getArgument(0)); + context.writeExpr(context.getArgument(0), context.getPrecedence()); break; case "isJava": + if (context.getPrecedence().ordinal() >= Precedence.COMPARISON.ordinal()) { + context.getWriter().append("("); + } context.writeExpr(context.getArgument(0)); context.getWriter().append(" instanceof ").append("$rt_objcls").append("()"); + if (context.getPrecedence().ordinal() >= Precedence.COMPARISON.ordinal()) { + context.getWriter().append(")"); + } break; } } diff --git a/tests/src/test/java/org/teavm/jso/test/JSWrapperTest.java b/tests/src/test/java/org/teavm/jso/test/JSWrapperTest.java index 5817406d6..6f3beb4f2 100644 --- a/tests/src/test/java/org/teavm/jso/test/JSWrapperTest.java +++ b/tests/src/test/java/org/teavm/jso/test/JSWrapperTest.java @@ -232,10 +232,12 @@ public class JSWrapperTest { @Test public void passJavaToJS() { var a = processObject(new A(23)); + assertEquals("A(23)", a.toString()); assertTrue(a instanceof A); assertEquals(23, ((A) a).getX()); a = processObject(JSString.valueOf("qwe")); + assertEquals("qwe", a.toString()); assertTrue(a instanceof JSString); assertEquals("qwe", ((JSString) a).stringValue()); @@ -263,5 +265,10 @@ public class JSWrapperTest { int getX() { return x; } + + @Override + public String toString() { + return "A(" + x + ")"; + } } }