<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">On Jan 20, 2015, at 11:09 AM, Vladimir Ivanov <<a href="mailto:vladimir.x.ivanov@oracle.com" class="">vladimir.x.ivanov@oracle.com</a>> wrote:<br class=""><div><blockquote type="cite" class=""><br class="Apple-interchange-newline"><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">What I'm mainly poking at here is that 'isGWT' is not informative about<br class="">the intended use of the flag.<br class=""></blockquote><span style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">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.</span><br style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">So, I removed isGWT flag and introduced a check for selectAlternative occurence in LambdaForm shape, as you suggested.</span><br style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div><div>Good.</div><div><br class=""></div><div>I think there is a sweeter spot just a little further on.  Make profileBranch be an LF intrinsic and expose it like this:</div><div>  GWT(p,t,f;S) := let(a=new int[3]) in lambda(*: S) { selectAlternative(profileBranch(p.invoke( *), a), t, f).invoke( *); }</div><div><br class=""></div><div>Then selectAlternative triggers branchy bytecodes in the IBGen, and profileBranch injects profiling in C2.</div><div>The presence of profileBranch would then trigger the @Shared annotation, if you still need it.</div><div><br class=""></div><div>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…</div><div><br class=""></div><div>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.</div><br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">In 'updateCounters', if the counter overflows, you'll get continuous<br class="">creation of ArithmeticExceptions.  Will that optimize or will it cause a<br class="">permanent slowdown?  Consider a hack like this on the exception path:<br class="">   counters[idx] = Integer.MAX_VALUE / 2;<br class=""></blockquote><span style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">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.</span><br style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div><div>Good.</div><br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">On the Name Bikeshed:  It looks like @IgnoreProfile (ignore_profile in<br class="">the VM) promises too much "ignorance", since it suppresses branch counts<br class="">and traps, but allows type profiles to be consulted.  Maybe something<br class="">positive like "@ManyTraps" or "@SharedMegamorphic"?  (It's just a name,<br class="">and this is just a suggestion.)<br class=""></blockquote><span style="font-family: Helvetica; font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">What do you think about @LambdaForm.Shared?</span></div></blockquote></div><br class=""><div class="">That's fine.  Suggest changing the JVM accessor to is_lambda_form_shared, because the term "shared" is already overused in the VM.</div><div class=""><br class=""></div><div class="">Or, to be much more accurate, s/@Shared/@CollectiveProfile/.  Better yet, get rid of it, as suggested above.</div><div class=""><br class=""></div><div class="">(I just realized that profile pollution looks logically parallel to the <a href="http://en.wikipedia.org/wiki/Tragedy_of_the_commons" class="">http://en.wikipedia.org/wiki/Tragedy_of_the_commons</a> .)</div><div class=""><br class=""></div><div class="">Also, in the comment explaining the annotation:</div><div class="">  s/mostly useless/probably polluted by conflicting behavior from multiple call sites/</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">(We could do profileInteger and profileClass in a similar way, if that turned out to be useful.)</div><div class=""><br class=""></div><div class="">— John</div></body></html>