teavm
teavm copied to clipboard
konsoletyper
#445: Added support for Concurrent[Hash]Map
Simplest fix for #445.
Actually in recent JDK (9+) ConcurrentMap only omtimizes some default methods of Map but does not add any specific methods anymore since they have all been added to Map already (IMHO even in JDK8) whereas in JDK7 ConcurrentMap was having lots of extra methods.
As even TMap already contains all these methods (e.g. getOrDefault) there is even no issue with older JDKs.
As inside the browser cuncurrency is not an issue like in JVM the simplest approach is to let ConcurrentHashMap behave exactly like HashMap.
Therefore I added this minimalistic approach that I tested with OpenJDK12 and recent TeaVM with my patch. I also did not have to "copy" any code that might be copyrighted by Oracle or so for this minimal solution.
As inside the browser cuncurrency is not an issue like in JVM the simplest approach is to let ConcurrentHashMap behave exactly like HashMap.
Wrong, concurrency matters, since TeaVM emulates it, so implementing ConcurrentHashMap is going to be non-trivial task.
Wrong, concurrency matters, since TeaVM emulates it, so implementing ConcurrentHashMap is going to be non-trivial task.
Thanks for your feedback and clarification. OK. However, currently when I use Concurrent[Hash]Map in my code, the TeaVM compiler fails. With this PR TeaVM would pass and the map behaves like a map even though it might not satisfy its contract. I never considered using creating Threads in my client code.
So can we consider agile improvements step by step and get this merged and create another issue to properly implement TConcurrentHashMap? I surely will quickly add a header to make checkstyle happy here.
OK. However, currently when I use Concurrent[Hash]Map in my code, the TeaVM compiler fails
You can workaround it by including TConcurrentHashMap to your code base.
I never considered using creating Threads in my client code
But there are people who use thread in their client code.
So can we consider agile improvements step by step and get this merged and create another issue to properly implement TConcurrentHashMap? I surely will quickly add a header to make checkstyle happy here.
No, I can't approve this. I think ConcurrentHashMap implementation should be complete. I tried to do that, but I failed to implement it in the way it's efficient in single-threaded code. Things are quite complicated there.
Please note that I have to replace usages of ConcurrentHashMap with HashMap for java.time support in #467 so your decision is not really helping for accuracy on the long run.
Can you point on a certain places where you replaced ConcurrentHashMap with HashMap?
Search for ConcurrentHashMap in this commit:
https://github.com/konsoletyper/teavm/pull/467/commits/0724f154caec10dc00fe47c94cd0df148f1a3229
@icecreamparlor no, I won't approve this PR. Instead, I'll implement ConcurrentHashMap myself some day.