JEP 270 concerns

[Andrew Haley]

Hi,

On 11/01/17 23:47, Frederic Parain wrote:
> 
> JEP 270 tried to resolve a very specific problem with stack overflows
> occurring in critical sections. When an annotated method m1 is inlined,
> in a method m2, m2 includes the stack requirements of m1 in the
> computation of its stack requirements. So no stack overflow is
> possible when m1 is "called (not really a call because of inlining).
> However, it only works if m1 is a leaf. If m1 calls another method
> and this method is not inlined, then I agree there is a case that
> has been missed here.

Right, but even in the jtreg case, m1 is not a leaf: it is
nonfairTryAcquire.  This calls getExclusiveOwnerThread, which is the
method that overflows the stack.  And
look_for_reserved_stack_annotated_method does not see the method which is
at the top anyway, so I think that a ReservedStackAccess method cannot
itself use the reserved area: it has to be one of its callees.

>> I thought that this was a simple omission, so I changed
>> look_for_reserved_stack_annotated_method to walk through Scopes
>> instead of frames, and this indeed detects that a ReservedStackAccess
>> method has been inlined.  However, there is a deeper flaw: once
>> control is returned to the method which inlines ReservedStackAccess,
>> the reserved zone remains disabled, so the next time that a method is
>> called the protection will not be in place.
> 
> Regarding the very particular usage of the annotation, it could be
> acceptable that a method which inlines an annotated method keeps
> access to the ReservedStackArea. This is not ideal, but it won't
> be worse than the situation before, and it would help anyway in
> some cases.

I guess so, but IMO the logic which handles a stack overflow when the
reserved area is unprotected would have to be fixed so that it doesn't
blindly re-protect the reserved area when it finishes.

> Please, could you sent me your code using Scopes? I'd like to
> experiment with it.

I will.

>> I think that the only reasonable way to fix this is to force methods
>> annotated with ReservedStackAccess not to be inlined.
> 
> JEP 270's goal is definitively not to prevent in-lining of the
> critical methods using the annotation.

OK.  It's the easy way to make this stuff all come out correctly,
but I understand why not.

>> It would be possible to fix this in a better way by changing the
>> logic so that a runtime call to re-enable reserved zone is inserted
>> at the return of every ReservedStackAccess-annotated method, but
>> this would be more complex.
> 
> This looks too expensive to me regarding the problem the
> ReservedStackArea tries to mitigate. It is possible to test with
> assembly code if the reserved area has been disabled or not (with
> the JavaThread::_reserved_stack_activation).

It wouldn't need to be any more expensive: a JIT is quite capable of
inlining the exact same assembly code.  It's just more work.

> It seems that there's a trade-off here between guaranteeing the
> exact granularity of the annotation and preserving the performances
> of in-lining. I think the later is the more important.
> 
> Could widening the scope of the of the annotation in case of in-lining,
> to include methods in which annotated methods are in-lined, be an
> acceptable solution?

That has a bad code smell, don't you think?  To do that you'll have to
change the test so that it works when inlining is enabled: the test
case works around the bug.  That feels wrong.  I don't think it's in
any sense impossible to make JEP 270 work as designed, even when stuff
is inlined.

Andrew.