Huge Performance Regression introduced September 18th

[Andreas Woess]

Hi Stefan, Tom:

We canonicalize a checkcast of an interface with a single leaf-class
implementor to a checkcast of that class, which is the case for
MaterializedFrame (and safe). The problem here is that there's no such
cast (in the bytecode) before the interface method calls on
MaterializedFrame (and we don't have type profiles). The next
complication is that the methods are defined in Frame, not
MaterializedFrame, so single implementor call devirtualization doesn't
trigger either.

As a workaround, you can manually insert a cast. A simple
"(MaterializedFrame) context" does not work, however, since javac would
just remove the unnecessary cast; you have to either do
MaterializedFrame.class.cast(context), or change the type of the context
field to Object, or have a helper method do the cast. So this patch
should help [1]. I realize this is suboptimal, and we should find a way
to let Graal insert the guard.

- andreas

[1]
> diff --git a/src/som/vmobjects/SBlock.java b/src/som/vmobjects/SBlock.java
> index fb7a685..5564f57 100644
> --- a/src/som/vmobjects/SBlock.java
> +++ b/src/som/vmobjects/SBlock.java
> @@ -95,12 +95,11 @@ public abstract class SBlock extends SAbstractObject {
>  
>    public final  MaterializedFrame getContext() {
>      assert context != null;
> -    return context;
> +    return MaterializedFrame.class.cast(context);
>    }
>  
>    public final Object getOuterSelf() {
> -    assert context != null;
> -    return SArguments.rcvr(context);
> +    return SArguments.rcvr(getContext());
>    }
>  
>    public static SInvokable getEvaluationPrimitive(final int
> numberOfArguments,


On 2014-11-19 01:17, Tom Rodriguez wrote:
> I confirmed it’s the trusted interface stuff by changing HotSpotResolvedObjectTypeImpl.isTrustedInterfaceType to always return true and I could see it speed up again.  A little more tweaking indicates it’s caused by MaterializedFrame not being trusted.  I don’t have a good answer for why though.  Presumably it’s inlining 
>
> The meaning behind the untrusted interface stuff is that the verifier treats interface types like Object, so there’s no guarantee that an argument or field with an interface type really implements that interface.  You have to perform a type check of some sort to know that it really is of the expected type.  The trusted interface stuff admits that by erasing interface types from our internal stamps, except in cases where we are using interface types in a slightly magical way within the WordTypeRewriter.
>
> Maybe erasing to object is being too conservative.  Could the interface type provide a hint about where to search in the hierarchy for target methods, even if you need a check to use them?  I would expect type profiling to help with this case but isn’t that disabled in truffle compiles?
>
> tom
>
> On Nov 18, 2014, at 1:49 PM, Stefan Marr <java at stefan-marr.de> wrote:
>
>> Hi:
>>
>> I got a problem with a change introduced on September 18th.
>> It introduces a >10x performance regression on my Mandelbrot benchmark.
>>
>> The last working and fast revision is
>> http://hg.openjdk.java.net/graal/graal/rev/646ddd52d79a
>>
>> Afterwards you got
>> http://hg.openjdk.java.net/graal/graal/rev/7716c6993546
>> and 
>> http://hg.openjdk.java.net/graal/graal/rev/a8eb7473d58a
>> which just result in errors when Graal is trying to compile my benchmark.
>>
>> And the next working version is:
>> http://hg.openjdk.java.net/graal/graal/rev/ac6e25901d62
>> [Add trusted interface concept and use it for WordBase, fix a NPE and some tests]
>>
>>
>>
>> Perhaps those changes again point out bugs in TruffleSOM, so would be good if someone could explain me what is going on there.
>>
>> On a recent Graal, you can see Mandelbrot being really slow by getting the source like usually:
>>
>> git clone --recursive https://github.com/SOM-st/TruffleSOM
>> cd TruffleSOM
>> ant jar
>> ../graal/mxtool/mx —vm server vm -Xbootclasspath/a:build/classes:libs/truffle.jar som.vm.Universe -cp Smalltalk Examples/Benchmarks/BenchmarkHarness.som Mandelbrot 1000 0 750
>>
>>
>> Thanks
>> Stefan
>>
>> -- 
>> Stefan Marr
>> INRIA Lille - Nord Europe
>> http://stefan-marr.de/research/
>>
>>
>>