Projects, which use JSR292

[Christian Thalinger]

On Feb 21, 2011, at 9:17 PM, Charles Oliver Nutter wrote:
> On Mon, Feb 21, 2011 at 10:08 AM, Christian Thalinger
> <christian.thalinger at oracle.com> wrote:
>> On Feb 21, 2011, at 4:43 PM, Christian Thalinger wrote:
>>> I fixed that but it didn't change performance.  I just tried bench_tak and there is still a regression.  I will look at that next.
> 
> I'll spew some nonsense here and you tell me what I've got right and
> what I've got wrong.
> 
> Hotspot weights recursive calls differently than non-recursive calls.
> I assume it treats non-recursive calls with priority, so that rather
> than inlining N levels of a method calling itself, it prefers to
> inline logic *around* the recursion that's likely doing the actual
> work of the method. Without such weighting, we'd end up with a giant
> method inlined into itself many times...but none of the work it
> actually does inlined. Correct?

As far as I can tell (please correct me if I'm wrong, John) there is no weighting of recursive vs. non-recursive calls in HotSpot and so we end up with the situation you sketched: the method gets too big.

> 
> The fib recursive call is not treated as a recursive call when invoked
> via indy because although it is logically a recursive call, it has
> intervening frames for the MH logic. As a result it gets weighted the
> same as a regular call, and so too many recursive calls get inlined
> before other stuff gets inlined at each level. This causes a
> degradation since the actual work of fib is in the *other* calls it
> makes, and they're all CALL ops with no inlined optimizations anymore.

Right.  And since the recursive inlining is infinite we just stop at some point and compile what we have, without actually doing much real work.

> 
> We do *want* fib to inline into itself, but only if it's appropriate
> and only after other "leaf" calls inline and optimize first. Your fix
> teaches hotspot that an indy recursive call should be treated as a
> recursive call, putting it back in its place.

Right.

> 
> I'm confused about the additional fix though. I assume the method gets
> too big with MaxRecursiveInlineLevel=1 because the method handle logic
> is also getting inlined and consuming the inlining budgets? And as a
> result, we're still pushing other important calls out of the inlining
> family, and performance degrades.

It looks like that.  As I said, we should not add these methods to the budget in this case.

> 
> It looks like you have a handle on things! I also eagerly await
> continued improvements, especially the logic to discount MH code from
> inlining budgets. It seems like that will be crucial for indy to
> compete with dynopt, since dynopt still performs quite a bit better
> but emits far too much bytecode right now.

I agree.

> 
> Another clarification for me: you are working against the main Hotspot
> repository now, yes? When and how can I get the fixes you've outlined?

Yeah, that's a bit complicated.  Recently I do a lot of bug fixing that come out of nightly or promotion testing, so I'm usually fixing them in the main repository (reads: hotspot-comp) without doing the extra work of adding them to mlvm (and later removing them again).  I just hope for a quick sync. of bsd-port, which doesn't seem to happen.

I could add this small fix as a patch to mlvm.  Do you want me to do that?

Actually I think I should file a CR since the infinitely-recursive inlining is a bug:

7022998: JSR 292 recursive method handle calls inline themselves infinitely

Here is the fix:

http://cr.openjdk.java.net/~twisti/7022998/webrev/

> 
> I will continue adding more invokedynamic paths and reporting how they
> behave. The improved timing for tak looks great, and with inlining
> tweaks I'm sure we can get indy-based fib and tak as fast or nearly as
> fast as jruby dynopt mode.

Great.  I'm looking forward to new data.

-- Christian