List.get() marked async because of Vector.get() ... Overzealous

[shannah]

I am working on updating to the latest (0.5.0-SNAPSHOT). I'm hitting new build errors caused by false positives of asynchronous code in a synchronous method. One of the stack traces is below:

[ERROR] Method com.codename1.impl.html5.HTML5Implementation$14.onAnimationFrame(I)V is claimed to be synchronous, but it is has invocations of asynchronous methods:
    calling com.codename1.impl.html5.HTML5Implementation$14.onAnimationFrame(I)V
    calling java.util.AbstractList$1.next()Ljava/lang/Object;
    calling java.util.Vector.get(I)Ljava/lang/Object;
    calling java.util.Vector.elementAt(I)Ljava/lang/Object;
    at com.codename1.impl.html5.HTML5Implementation$14.onAnimationFrame

The onAnimationFrame() method uses ArrayList, which internally uses List. It looks like it is marking List.get(int) as async because Vector implements that method.

If I set "stopOnErrors=false", then the build completes (with those warnings printed out), and the app works fine.

Test case: https://github.com/shannah/teavm-issue-248

[konsoletyper]

I guess, it's correct behaviour. Apparently, you are using Vector somewhere in CN1. This makes TeaVM to mark List.get as async. The solution would be to stop using Vector, it's de-facto deprecated class. Another solution is to start a new Thread from onAnimationFrame. Hopefully, it's possible to come up with a more accurate algorithm to find async methods, but it's mostly a research task. In terms of current model it's not a bug.

[shannah]

Thanks for looking at this. Unfortunately Vector is part of some public APIs and can't be totally gotten rid of. But I'm going to look through and see where I can minimize its use. In this case I can't launch a thread in onAnimationFrame because that code has to complete inside onAnimationFrame (that's how onAnimationFrame works). I may be able to work around this by explicitly using ArrayList (rather than List) there, but I think ArrayList uses List internally which would still cause problems here.

I understand what you are saying about it not being a bug. I'm going to look through the commits to see if I can identify where this started happening (as the project builds fine with the version of TeaVM we have on our build servers). Maybe I can figure out some compromise.

[shannah]

Just a note: This is the commit that broke it

6ef08ae#diff-48b91f7f62944631417e7c81669e9eeb

But as far as I can tell that commit actually "fixes" it. i.e. it works correctly now, and did not before.

Frankly, I didn't know how you were getting around the "synchronized" implementation of java.util.List methods problem before, but I just figured, you had some sort of magic that dealt with it.

For my particular problem, I'm just going to swap out the java collections and use JSArray instead in these sensitive places. But I'm curious. Do you have any intuition on how "bad" it is performance-wise to have all List and Map methods marked as "async" due to the use of a synchronized collection (like Vector) somewhere in the code)?

If it's really bad, I may try to add some pre-processing to apps before they run through TeaVM to try to remove such problems.

[shannah]

A comment on the pull request. I tried a number of workarounds (even tried changing all of my uses of java Collections to JSArray, but still ended up with those sync error warnings. It even objected to things like System.arraycopy because System.clinit was Async. This is all likely caused by Vector or some other collection that overrides common methods with synchronized versions - which causes TeaVM to have to treat all java collections as synchronized.

99% of cases of Vector don't require synchronized usage -- they are just there as legacy. I had thought to preprocess my bytecode using ASM to change all Vector, Hashtable, and Stack usage to some non-synchronized version that I implement myself - and still might do that - but in the mean time, the best solution seems to be adding the ability to mark a method that I'm fairly certain doesn't use async APIs (even though TeaVM thinks it does).

That's what that pull request is for. I mark my onAnimationFrame method with @SuppressSyncErrors, and it causes that particular error to be treated as a warning only so that compilation can go through.

[konsoletyper]

Another workaround is to stop loading resources on demand and preload them, so that methods that load resources just returned preloaded ones from memory. This should decrease number of split points.