On Jan 20, 2015, at 11:09 AM, Vladimir Ivanov <vladimir.x.iva...@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
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Reply via email to