[9] RFR (M): 8063137: Never-taken branches should be pruned when GWT LambdaForms are shared

[John Rose]

On Jan 20, 2015, at 11:09 AM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
> 
>> What I'm mainly poking at here is that 'isGWT' is not informative about
>> the intended use of the flag.
> I agree. It was an interim solution. Initially, I planned to introduce customization and guide the logic based on that property. But it's not there yet and I needed something for GWT case. Unfortunately, I missed the case when GWT is edited. In that case, isGWT flag is missed and no annotation is set.
> So, I removed isGWT flag and introduced a check for selectAlternative occurence in LambdaForm shape, as you suggested.

Good.

I think there is a sweeter spot just a little further on.  Make profileBranch be an LF intrinsic and expose it like this:
  GWT(p,t,f;S) := let(a=new int[3]) in lambda(*: S) { selectAlternative(profileBranch(p.invoke( *), a), t, f).invoke( *); }

Then selectAlternative triggers branchy bytecodes in the IBGen, and profileBranch injects profiling in C2.
The presence of profileBranch would then trigger the @Shared annotation, if you still need it.

After thinking about it some more, I still believe it would be better to detect the use of profileBranch during a C2 compile task, and feed that to the too_many_traps logic.  I agree it is much easier to stick the annotation on in the IBGen; the problem is that because of a minor phase ordering problem you are introducing an annotation which flows from the JDK to the VM.  Here's one more suggestion at reducing this coupling…

Note that C->set_trap_count is called when each Parse phase processes a whole method.  This means that information about the contents of the nmethod accumulates during the parse.  Likewise, add a flag method C->{has,set}_injected_profile, and set the flag whenever the parser sees a profileBranch intrinsic (with or without a constant profile array; your call).  Then consult that flag from too_many_traps.  It is true that code which is parsed upstream of the very first profileBranch will potentially issue a non-trapping fallback, but by definition that code would be unrelated to the injected profile, so I don't see a harm in that.  If this approach works, then you can remove the annotation altogether, which is clearly preferable.  We understand the annotation now, but it has the danger of becoming a maintainer's puzzlement.

> 
>> In 'updateCounters', if the counter overflows, you'll get continuous
>> creation of ArithmeticExceptions.  Will that optimize or will it cause a
>> permanent slowdown?  Consider a hack like this on the exception path:
>>    counters[idx] = Integer.MAX_VALUE / 2;
> I had an impression that VM optimizes overflows in Math.exact* intrinsics, but it's not the case - it always inserts an uncommon trap. I used the workaround you proposed.

Good.

> 
>> On the Name Bikeshed:  It looks like @IgnoreProfile (ignore_profile in
>> the VM) promises too much "ignorance", since it suppresses branch counts
>> and traps, but allows type profiles to be consulted.  Maybe something
>> positive like "@ManyTraps" or "@SharedMegamorphic"?  (It's just a name,
>> and this is just a suggestion.)
> What do you think about @LambdaForm.Shared?

That's fine.  Suggest changing the JVM accessor to is_lambda_form_shared, because the term "shared" is already overused in the VM.

Or, to be much more accurate, s/@Shared/@CollectiveProfile/.  Better yet, get rid of it, as suggested above.

(I just realized that profile pollution looks logically parallel to the http://en.wikipedia.org/wiki/Tragedy_of_the_commons <http://en.wikipedia.org/wiki/Tragedy_of_the_commons> .)

Also, in the comment explaining the annotation:
  s/mostly useless/probably polluted by conflicting behavior from multiple call sites/

I very much like the fact that profileBranch is the VM intrinsic, not selectAlternative.  A VM intrinsic should be nice and narrow like that.  In fact, you can delete selectAlternative from vmSymbols while you are at it.

(We could do profileInteger and profileClass in a similar way, if that turned out to be useful.)

— John