Huge Performance Regression introduced September 18th

[Gilles Duboscq]

Hi Stefan,

I just commited a change that should solve your problem (it will come
in the nightly push in a few hours).
On my machine, the perf of your Mandelbrot example went up 10x so the
regression seems to be fixed.

-Gilles

On Wed, Nov 19, 2014 at 11:08 AM, Andreas Woess <Andreas.Woess at jku.at> wrote:
> On 2014-11-19 08:01, Stefan Marr wrote:
>> Hi:
>>
>> First, thank you guys, for immediately looking into it. :)
>>
>>> On 19 Nov 2014, at 04:04, Andreas Woess <andreas.woess at jku.at> wrote:
>>>
>>> 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).
>> I wonder why none of the explicit types in the rest of the code is sufficient?
>> The return type of the method you patched is MaterializedFrame, as well as the field in which the pointer is stored.
> As Tom mentioned, we cannot trust interface type in signatures as they
> aren't verified, and that's why we (currently) erase this type
> information in the compiler. Maybe the solution is to not erase this
> information but insert the necessary runtime checks.
>
>>> 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.
>> How does that relate to a change that removed CompilerDirectives.unsafeFrameCast?
>> Because, I used that helper in exactly that position before. But, I needed to remove it from the code on November 6th, because it was gone from the API.
> unsafeFrameCast did achieve the same thing, although in a potentially
> unsafe manner. We could put back a (safe) frameCast method, but it would
> be better not to need it.
>
> - andreas