RFR (S): CR 8009120: Fuzz instruction scheduling in Hotspot compilers

[Aleksey Shipilev]

Thanks!

Here's the updated webrev:
 http://cr.openjdk.java.net/~shade/8009120/webrev.02/

Testing:
 - release/fastdebug builds on my laptop: concurrency tests are failing
with -XX:+StressLCM, as expected
 - hotspot-comp JPRT is running now; I'll report on success/failure

Comments below:

On 03/01/2013 12:46 AM, Vladimir Kozlov wrote:
> First, about your fuzzing code. fuzz_accept() will always return 'true'
> for count == 1, after that probability of 'true' will decrease with each
> count's increment. So you will always select first candidate which is
> not random. Why you don't want a uniform distribution? Also why you want
> only one candidate to be selected (if I understand code correctly)?

There is a confusion about the contract for this method. phase.hpp
before, and compile.cpp now has the example of how this method is used.
Both LCM and GCM are not storing the list of candidates, which we then
can magically use to choose from uniformly.

Instead, they update the best candidate as they munch through the
candidate list, and we even don't know in advance how much candidates
there'll be. In this scheme, you need to adjust the randomicity, or else
you will end up biasing the selection towards the latter candidates.

Quick back-envelope calculation shows that for the list of n candidates
the equal probability for the candidate to persist as "best" can be
achieved by replacing it with "next" k-th candidate with the probability
of 1/k. It can be easily shown that by the end of the run, the
probability for all candidates are converged to 1/n, thus giving the
uniform distribution among all the candidates.

> Could you change "fuzzing", "fuzz" to "stressing", "stress" in flags
> names and comments. It will be in the line with other stress flags/code
> we have already.

Although I think "fuzz" declares the intent more clearly, I changed
everything to "stress".

> I also think fuzz_accept() could be different name, may be
> fuzz_selected(cand) or something?

"randomized_select" it is now.

> define flags as 'develop' (not 'notproduct') so they will be accessible
> (but not modifiable) in product code. 

Yes, cool.

> bool fuzz_candidate = FuzzGCM && fuzz_selected(++fuzz_candidate_cnt);
> 
>       if (LCA_freq < least_freq     || // Better Frequency
> +         fuzz_candidate ||
>           ( !in_latency          &&    // No block containing latency

Nope, that defeats the purpose. I want the latency-based policy to be
disabled at all when stressing is enabled. Otherwise both LCM and GCM
will converge to lowest latency, and stressing will have the nil effect.
Hence I left it as is, with develop flag cleanup.

> Phase class is bad place for fuzz_accept() method. We usually put such
> methods into Compile class if you want to use this method in other
> places in C2.

I was wondering where to put it. It is in the Compile now.

> Why you need C2_PHASE_ in C2_PHASE_FUZZ_DOMAIN name? Do we have other
> FUZZ_DOMAIN? You don't need to worry about that if you move method body
> and FUZZ_DOMAIN definition into .cpp file.

Fixed.

> Also why not choose 2^n value so that '%' could be simple masking.

Improved.

-Aleksey.