konsoletyper / teavm

Compiles Java bytecode to JavaScript, WebAssembly and C
https://teavm.org
Apache License 2.0
2.55k stars 260 forks source link

JSObject.toString() does not compile correctly under Java 21 #898

Closed stikdragon closed 2 months ago

stikdragon commented 3 months ago

We've got a fairly large, mature application written in java and compiled by TeaVM, up until today it's been going very well. We've just tried to upgrade from Java 17 to 21, and now we're getting some odd behaviour. It's been difficult to pinpoint exactly what is going on because this is a large code base, but so far it seems to be problems with relatively low level java/js interactions, in particular we've got a case where a string operation is failing. here is the java code:

    public static String stringOrNull(JSObject o) {
            return o == null || JSObjects.isUndefined(o) ? null : o.toString();
    }

and here is the JS that's generated by TeaVM when running under Java 17

    function ceppuu_ClientUtil_stringOrNull($o) {
        ceppuu_ClientUtil_$callClinit();
        return $o !== null && !(typeof $o === 'undefined' ? 1 : 0) ? (otji_JSWrapper_wrap($o)).$toString() : null;
    }

Here is the same JS when running under Java 21:

    function ceppuu_ClientUtil_stringOrNull($o) {
        ceppuu_ClientUtil_$callClinit();
        return $o !== null && !(typeof $o === 'undefined' ? 1 : 0) ? $o.$toString() : null;
    }

Notice that the otji_JSWrapper_wrap call has been ommitted. This throws an exception (because $toString isn't a function of js strings). I've done a text compare on the two classes.js files and there's a lot of differences, although it's hard to be certain how many because of the size of it (my text compare tools get confused if they differ too much)

If we re-work our code to not require this stringOrNull method then we get similar issues in other places, for instance JSObject.equals starts to behave oddly in places too, although i haven't invetigated that in very much detail yet, but i suspect a similar thing is going on.

There is a second layer to this problem, too, which added to the difficulty in comprehending what's happening. Our build process involves an Ant script that compiles the java code, then invokes teavm to produce the js. We also have a "dev server" of our own creation that invokes this build script when it detects changes, so we can have compile-on-demand features while developing. we've noticed that when the dev server triggers the teavm compilation to happen then this problem is not present. this has confused me even more, since all the dev server does is spawn a new windows process and execute ant compile, so why does it behave so differently?

I have a theory - we're running all this from within Eclipse which has its own java compiler. It seems like the output from this is sometimes subtly different to the "real" one in the JDK. I've pulled apart the .class file containing this method and here are the differences:

    // From javac: (this produces invalid TeaVM output)
    // Version: 21.0.2+13-58
public static java.lang.String stringOrNull(org.teavm.jso.JSObject);
    Code:
       0: aload_0
       1: ifnull        11
       4: aload_0
       5: invokestatic  #164                // Method org/teavm/jso/core/JSObjects.isUndefined:(Ljava/lang/Object;)Z
       8: ifeq          15
      11: aconst_null
      12: goto          21
      15: aload_0
      16: invokeinterface #175,  1          // InterfaceMethod org/teavm/jso/JSObject.toString:()Ljava/lang/String;
      21: areturn
    // From Eclipse's compiler: (this is ok)
    // Version: 2024-03 (4.31.0)
public static java.lang.String stringOrNull(org.teavm.jso.JSObject);
    Code:
        0: aload_0
        1: ifnull        11
        4: aload_0
        5: invokestatic  #255                // Method org/teavm/jso/core/JSObjects.isUndefined:(Ljava/lang/Object;)Z
        8: ifeq          15
        11: aconst_null
        12: goto          19
        15: aload_0
        16: invokevirtual #270                // Method java/lang/Object.toString:()Ljava/lang/String;
        19: areturn

Notice that the final opcode at offset 16 from o.toString differs

Have you got any thoughts on this? Is this some new behaviour in Java 21 (or 18+) that TeaVM is tripping over?

stikdragon commented 3 months ago

Some additional info - this behaviour of javac seems to have changed between 17.0.2 and 18GA. I've not found anything in the java release notes yet

EDIT: in fact, here's the change: https://github.com/openjdk/jdk/pull/5165#issuecomment-916003703

konsoletyper commented 2 months ago

BTW, for performance reason I strongly suggest you to use JSObjects.toString(obj) instead of obj.toString, in case you sure that obj is JSObject. As for now, TeaVM always produces wrapper object when you call toString. I hope that at some point I find some time to refactor TeaVM optimizer and make it omit unnecessary wrapper generation. The only reason to use Object.toString with JavaScript objects is to write common logic that applies both to JS and Java objects (like List.toString, where List can potentially contain both Object and JSObject). This should also be a work-around for this issue in you particular case.