From eed44998f07054030825af942ae8a3d12800c57d Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Tue, 26 Sep 2023 17:48:29 +0200 Subject: [PATCH] jso: trying to improve optimization of JSWrapper --- .../org/teavm/jso/impl/JSClassProcessor.java | 39 ++++++----- .../jso/impl/JSObjectClassTransformer.java | 2 +- .../org/teavm/jso/impl/JSValueMarshaller.java | 29 ++++---- .../java/org/teavm/jso/impl/JSWrapper.java | 2 +- .../org/teavm/jso/test/JSWrapperTest.java | 67 +++++++++++++++++++ 5 files changed, 109 insertions(+), 30 deletions(-) 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 986fea14e..dfe30361f 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 @@ -553,13 +553,15 @@ class JSClassProcessor { newInvoke.setLocation(invoke.getLocation()); List newArgs = new ArrayList<>(); if (invoke.getInstance() != null) { - Variable arg = marshaller.wrapArgument(callLocation, invoke.getInstance(), - ValueType.object(method.getOwnerName()), false); + var arg = invoke.getInstance(); + arg = marshaller.wrapArgument(callLocation, arg, + ValueType.object(method.getOwnerName()), types.typeOf(arg), false); newArgs.add(arg); } for (int i = 0; i < invoke.getArguments().size(); ++i) { - Variable arg = marshaller.wrapArgument(callLocation, invoke.getArguments().get(i), - method.parameterType(i), byRefParams[i]); + var arg = invoke.getArguments().get(i); + arg = marshaller.wrapArgument(callLocation, invoke.getArguments().get(i), + method.parameterType(i), types.typeOf(arg), byRefParams[i]); newArgs.add(arg); } newInvoke.setArguments(newArgs.toArray(new Variable[0])); @@ -597,9 +599,10 @@ class JSClassProcessor { if (propertyName == null) { propertyName = cutPrefix(method.getName(), 3); } - Variable wrapped = marshaller.wrapArgument(callLocation, invoke.getArguments().get(0), - method.parameterType(0), false); - addPropertySet(propertyName, invoke.getInstance(), wrapped, invoke.getLocation(), pure); + var value = invoke.getArguments().get(0); + value = marshaller.wrapArgument(callLocation, value, + method.parameterType(0), types.typeOf(value), false); + addPropertySet(propertyName, invoke.getInstance(), value, invoke.getLocation(), pure); return true; } diagnostics.error(callLocation, "Method {{m0}} is not a proper native JavaScript property " @@ -616,8 +619,9 @@ class JSClassProcessor { private boolean processIndexer(MethodReader method, CallLocation callLocation, InvokeInstruction invoke) { if (isProperGetIndexer(method.getDescriptor())) { Variable result = invoke.getReceiver() != null ? program.createVariable() : null; - addIndexerGet(invoke.getInstance(), marshaller.wrapArgument(callLocation, invoke.getArguments().get(0), - method.parameterType(0), false), result, invoke.getLocation()); + var index = invoke.getArguments().get(0); + addIndexerGet(invoke.getInstance(), marshaller.wrapArgument(callLocation, index, + method.parameterType(0), types.typeOf(index), false), result, invoke.getLocation()); if (result != null) { result = marshaller.unwrapReturnValue(callLocation, result, method.getResultType(), false, canBeOnlyJava(invoke.getReceiver())); @@ -626,10 +630,11 @@ class JSClassProcessor { return true; } if (isProperSetIndexer(method.getDescriptor())) { - Variable index = marshaller.wrapArgument(callLocation, invoke.getArguments().get(0), - method.parameterType(0), false); - Variable value = marshaller.wrapArgument(callLocation, invoke.getArguments().get(1), - method.parameterType(1), false); + var index = invoke.getArguments().get(0); + marshaller.wrapArgument(callLocation, index, method.parameterType(0), types.typeOf(index), false); + var value = invoke.getArguments().get(1); + value = marshaller.wrapArgument(callLocation, value, method.parameterType(1), + types.typeOf(value), false); addIndexerSet(invoke.getInstance(), index, value, invoke.getLocation()); return true; } @@ -700,8 +705,9 @@ class JSClassProcessor { invoke.getLocation())); newInvoke.setLocation(invoke.getLocation()); for (int i = 0; i < invoke.getArguments().size(); ++i) { - Variable arg = marshaller.wrapArgument(callLocation, invoke.getArguments().get(i), - method.parameterType(i), byRefParams[i]); + var arg = invoke.getArguments().get(i); + arg = marshaller.wrapArgument(callLocation, arg, + method.parameterType(i), types.typeOf(arg), byRefParams[i]); newArguments.add(arg); } newInvoke.setArguments(newArguments.toArray(new Variable[0])); @@ -906,7 +912,8 @@ class JSClassProcessor { ExitInstruction exit = new ExitInstruction(); if (insn.getReceiver() != null) { replacement.clear(); - exit.setValueToReturn(marshaller.wrap(insn.getReceiver(), callee.getResultType(), null, false)); + exit.setValueToReturn(marshaller.wrap(insn.getReceiver(), callee.getResultType(), JSType.MIXED, + null, false)); block.addAll(replacement); } block.add(exit); 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 42c3add50..9fe1e9f57 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 @@ -147,7 +147,7 @@ class JSObjectClassTransformer implements ClassHolderTransformer { if (method.getResultType() != ValueType.VOID) { invocation.setReceiver(program.createVariable()); exit.setValueToReturn(marshaller.wrapArgument(callLocation, invocation.getReceiver(), - method.getResultType(), false)); + method.getResultType(), JSType.MIXED, false)); basicBlock.addAll(marshallInstructions); marshallInstructions.clear(); } 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 e88b9413c..dcb595d74 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 @@ -58,7 +58,7 @@ class JSValueMarshaller { this.replacement = replacement; } - Variable wrapArgument(CallLocation location, Variable var, ValueType type, boolean byRef) { + Variable wrapArgument(CallLocation location, Variable var, ValueType type, JSType jsType, boolean byRef) { if (type instanceof ValueType.Object) { String className = ((ValueType.Object) type).getClassName(); ClassReader cls = classSource.get(className); @@ -66,7 +66,7 @@ class JSValueMarshaller { return wrapFunctor(location, var, cls); } } - return wrap(var, type, location.getSourceLocation(), byRef); + return wrap(var, type, jsType, location.getSourceLocation(), byRef); } boolean isProperFunctor(ClassReader type) { @@ -98,7 +98,7 @@ class JSValueMarshaller { return functor; } - Variable wrap(Variable var, ValueType type, TextLocation location, boolean byRef) { + Variable wrap(Variable var, ValueType type, JSType jsType, TextLocation location, boolean byRef) { if (byRef) { InvokeInstruction insn = new InvokeInstruction(); insn.setMethod(JSMethods.ARRAY_DATA); @@ -112,14 +112,19 @@ class JSValueMarshaller { if (type instanceof ValueType.Object) { String className = ((ValueType.Object) type).getClassName(); if (className.equals("java.lang.Object")) { - var unwrapNative = new InvokeInstruction(); - unwrapNative.setLocation(location); - unwrapNative.setType(InvocationType.SPECIAL); - unwrapNative.setMethod(new MethodReference(JSWrapper.class, "javaToJs", Object.class, JSObject.class)); - unwrapNative.setArguments(var); - unwrapNative.setReceiver(program.createVariable()); - replacement.add(unwrapNative); - return unwrapNative.getReceiver(); + if (jsType != JSType.NULL && jsType != JSType.JS) { + var unwrapNative = new InvokeInstruction(); + unwrapNative.setLocation(location); + unwrapNative.setType(InvocationType.SPECIAL); + unwrapNative.setMethod(new MethodReference(JSWrapper.class, + "javaToJs", Object.class, JSObject.class)); + unwrapNative.setArguments(var); + unwrapNative.setReceiver(program.createVariable()); + replacement.add(unwrapNative); + return unwrapNative.getReceiver(); + } else { + return var; + } } if (!className.equals("java.lang.String")) { return var; @@ -516,7 +521,7 @@ class JSValueMarshaller { } Variable addStringWrap(Variable var, TextLocation location) { - return wrap(var, stringType, location, false); + return wrap(var, stringType, JSType.MIXED, location, false); } Variable addString(String str, TextLocation location) { diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JSWrapper.java b/jso/impl/src/main/java/org/teavm/jso/impl/JSWrapper.java index 1978ca723..b9f6ca151 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JSWrapper.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JSWrapper.java @@ -244,6 +244,6 @@ public final class JSWrapper { @Override public String toString() { - return JSObjects.toString(js); + return JSObjects.isUndefined(js) ? "undefined" : JSObjects.toString(js); } } 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 6f3beb4f2..cf4bb2bb7 100644 --- a/tests/src/test/java/org/teavm/jso/test/JSWrapperTest.java +++ b/tests/src/test/java/org/teavm/jso/test/JSWrapperTest.java @@ -244,6 +244,53 @@ public class JSWrapperTest { a = processObject(JSNumber.valueOf(23)); assertTrue(a instanceof JSString); assertEquals("number", ((JSString) a).stringValue()); + + a = processObject(processObject(new A(24))); + assertEquals("A(24)", a.toString()); + assertTrue(a instanceof A); + assertEquals(24, ((A) a).getX()); + + a = processObject(identity(processObject(new A(25)))); + assertEquals("A(25)", a.toString()); + assertTrue(a instanceof A); + assertEquals(25, ((A) a).getX()); + + a = processObject(identity(processObject(JSString.valueOf("asd")))); + assertEquals("asd", a.toString()); + assertTrue(a instanceof JSString); + assertEquals("asd", ((JSString) a).stringValue()); + + a = processObject(processObject(identity(JSString.valueOf("zxc")))); + assertEquals("zxc", a.toString()); + assertTrue(a instanceof JSString); + assertEquals("zxc", ((JSString) a).stringValue()); + } + + @Test + public void exportedObject() { + var r = new ReturningObject() { + @Override + public Object get() { + var o = createEmpty(); + setProperty(o, "foo", JSNumber.valueOf(23)); + return o; + } + }; + var o = extract(r); + var foo = getProperty(o, "foo"); + assertTrue(foo instanceof JSNumber); + assertEquals(JSNumber.valueOf(23), foo); + } + + @Test + public void setProperty() { + var o = createEmpty(); + callSetProperty(o, JSNumber.valueOf(23)); + assertEquals(JSNumber.valueOf(23), getProperty(o, "foo")); + } + + private void callSetProperty(Object instance, Object o) { + setProperty(instance, "foo", o); } @JSBody(script = "return null;") @@ -255,6 +302,22 @@ public class JSWrapperTest { @JSBody(params = "o", script = "return typeof o === 'number' ? 'number' : o;") private static native Object processObject(Object o); + private Object identity(Object o) { + return o; + } + + @JSBody(params = { "o", "name" }, script = "return o[name];") + private static native Object getProperty(Object o, String name); + + @JSBody(params = { "o", "name", "value" }, script = "o[name] = value;") + private static native void setProperty(Object o, String name, Object value); + + @JSBody(script = "return {};") + private static native Object createEmpty(); + + @JSBody(params = "o", script = "return o.get();") + private static native Object extract(ReturningObject o); + static class A { private int x; @@ -271,4 +334,8 @@ public class JSWrapperTest { return "A(" + x + ")"; } } + + interface ReturningObject extends JSObject { + Object get(); + } }