Huge Performance Regression introduced September 18th
andreas.woess at jku.at
Wed Nov 19 10:08:09 UTC 2014
On 2014-11-19 08:01, Stefan Marr wrote:
> 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 . 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.
More information about the graal-dev