RFC(S) 8234146: [TESTBUG] compiler/jsr292/ContinuousCallSiteTargetChange.java times out on SPARC

[Reingruber, Richard]

Hi Vladimir and Tobias,

thanks for your reviews. I agree.

Regarding option #B: I'd say the current implementation is very generic but it obfuscates the
simple/intuitive rule that all dependent nmethods have to be invalidated when the call site target
is changed. So in my eyes #B is less complex.

Still: new code => potentially new bugs. So I agree to fix the issue with option #B.

Thanks, Richard. 

-----Original Message-----
From: Vladimir Ivanov <vladimir.x.ivanov at oracle.com> 
Sent: Dienstag, 10. März 2020 19:08
To: Reingruber, Richard <richard.reingruber at sap.com>; hotspot-compiler-dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
Subject: Re: RFC(S) 8234146: [TESTBUG] compiler/jsr292/ContinuousCallSiteTargetChange.java times out on SPARC

Hi Richard,

Nice analysis!

I'd prefer to go with the test fix (option #A) for now.

The idea to short-circuit repeated traversals over CallSite context 
looks clever, but I'm not yet persuaded it's worth the added complexity 
for such a pathological case.

So, I propose to fix the test case and file an RFE for the inefficiency 
you spotted in dependency invalidation.

[...]

> Option A: Force the NMethodSweeper using the whitebox api.
> Webrev A: http://cr.openjdk.java.net/~rrich/webrevs/8234146/A/webrev.0/

Looks good.

Best regards,
Vladimir Ivanov


> 
> Option B: Specialize DependencyContext::mark_dependent_nmethods() for call site target changes,
> i.e. flag list elements that are marked for deoptimization. Upon the next target change only new
> dependents are marked for deoptimization. The list iteration is ended as soon as an element is
> encountered that is already flagged.
> Webrev B: http://cr.openjdk.java.net/~rrich/webrevs/8234146/B/webrev.0/
> 
> Personally I like B better, because I think it is simple and straight forward: if the target
> changes, then all dependents need to be invalidated. By construction of the list we can stop the
> invalidation if we find an element that was invalidated before. This specialization is of course
> less generic, and there might be new types of call site dependencies for which it is illegal, but
> currently I can't think of any.
> 
> Curious about your comments...
> 
> Thanks,
> Richard.
> 
> [1] To simulate load you can pin the jvm process to one cpu (e.g. on Linux with "taskset 1 java ...")
>      or
>      wget http://cr.openjdk.java.net/~rrich/webrevs/8234146/CPULoadGenerator.java && java CPULoadGenerator.java N
>      where N is the number of CPUs of your box
>