RFR(S): 8160717: MethodHandles.loop() does not check for excessive signature

[Claes Redestad]

On 2016-07-06 12:45, Paul Sandoz wrote:
>
>> On 6 Jul 2016, at 12:04, Michael Haupt <michael.haupt at oracle.com> wrote:
>>
>> Hi Paul,
>>
>> thanks for your comments.
>>
>> Lazy initialisation of the PerfCounter is good, as is the warning suppression.
>>
>> I'll let Claes comment on the broader PerfCounter question, as he suggested using them. I think PerfCounter is a convenient abstraction for what we want to achieve, but the way it's used here may smell a bit abusive.
>>
>
> Ok.

I know of a number of Java-side PerfCounters created early (and they're 
rather lean on dependencies in the first place, a select number of 
j.u.c.atomic classes IIRC), so I wouldn't worry about much of a startup 
penalty here.

Lazy initialization might not save us much, and would hide the counter 
from showing up.

I guess what I'm saying is I'm good with webrev.00. :-)

>
>
>> What do you mean with "differentiate between the two cases"? Assuming it's Error/Exception vs. BytecodeGenerationException, here's the current rationale. There are cases when it makes sense to fall back to LFI, e.g., when, as here, methods grow too large. In many other cases it is justified to bail altogether with an InternalError. Wrapping cw.toClassBytes() is a first approximation.
>>
>
> I meant to say:
>
>>> Is it correct to say we *cannot* differentiate between the two cases. If so the danger is that we add another intrinsic and don’t realize it contains an actual error and we notice too late when we observe performance falling off a cliff.
>>
>
>
> I don’t know if we can differentiate between a valid error (reaching the certain byte code limits) and an unexpected error where say we add a new intrinsic and its code generation is faulty causing ASM to throw RuntimeException.
>
> A wild thought: is there anyway to add some context/data to the ASM processing indicating what intrinsic is being processed, so when you get the exception you can at least differentiate.

While out of scope for this change, maybe we should seek to improve 
robustness by asking the ASM project to add more distinctive exceptions, 
say, create a MethodTooLargeException or similar, in which case we could 
change the catch to be more specific.

/Claes

>
> Paul.
>