RFR: 8219233: 3x performance drop for some Clojure applications

[Claes Redestad]

Hi David,

On 2019-02-27 05:17, David Holmes wrote:
> webrev: http://cr.openjdk.java.net/~dholmes/8219233/webrev.v2/

patch looks good to me.

> bug: https://bugs.openjdk.java.net/browse/JDK-8219233

As this is only a partial fix (about a 10% improvement in the clojure
startup repro), I'd suggest moving it to a subtask and leaving
JDK-8219233 open for the time being - if only to not give the false
impression the issue is resolved in its entirety.

I've verified this recuperates performance completely in cases where
we're calling into static methods from the class itself, e.g., "Bad"[1],
and also that it doesn't help in cases where call are routed back via
another class, e.g., "AlsoBad".

Thanks!

/Claes

[1] https://cl4es.github.io/2019/02/21/Cljinit-Woes.html

> 
> We had a bug whereby the callsite to a static method was resolved as 
> soon as it was executed, allowing other threads to execute the same code 
> and skip the class initialization check. The fix to that was to not do 
> the resolution if the target class was not fully initialized. This has a 
> consequence that calls to static methods of the current class from the 
> <clinit> (including transitively) can't be resolved and effectively 
> compiled and so we can take a huge performance hit for those cases 
> (which is a rare case and regular benchmarking showed no issue). Clojure 
> unfortunately uses a style of coding that is significantly impacted by 
> this.
> 
> There is a simple fix for the direct case of calling a static method 
> from <clinit>: when we are dealing with the same class then it's okay to 
> do the resolution as the current thread must have <clinit> on its 
> callstack. No other thread can execute the code being resolved until the 
> initial thread completes the static initialization.
> 
> Unfortunately that doesn't address the main pattern used by Clojure, but 
> we have a subtask open where Vladimir Ivanov continues to look into that 
> case. And Claes Redestad has also given coding advice on how to avoid 
> the problem in the first place.
> 
> Testing: tier 1-3
> Microbenchmarking - see bug report.
> 
> Thanks,
> David
> -----