RFR (XXS): 8079156: 32 bit Java 9-fastdebug hit assertion in client mode with StackShadowPages flag value from 32 to 50

[gerard ziemski]

David, Coleen,

Should I post a new webrev with the code Validmir just suggested, ie:

CodeBuffer   buffer("deopt_blob", 1536, 1024);

or are we OK to proceed as is?


cheers

On 07/13/2015 07:42 PM, Vladimir Kozlov wrote:
> On David's question. The size of code is rounded to 64 bytes (minimum 
> allocation unit in CodeCache). So you don't need to rounding.
>
> We do calculate additional padding on sparc taking into account 
> instructions sizes and number of StackShadowPages pages (which banging 
> code is looping on):
>
> #ifdef ASSERT
>   if (UseStackBanging) {
>     pad += StackShadowPages*16 + 32;
>   }
> #endif
>
> But on x86 we have variable instructions size so it is not easy 
> pre-calculate it.
> So we simple set big number and if we hit the assert in codeBuffer.hpp 
> we increase it:
>
> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/3e15bdb908cb/src/cpu/x86/vm/sharedRuntime_x86_64.cpp#l2841 
>
>
> I would suggest simple increase it by 512 (32-bit code is smaller then 
> 64-bit) and done with it:
>
> CodeBuffer   buffer("deopt_blob", 1536, 1024);
>
> Thanks,
> Vladimir
>
> On 7/13/15 10:38 AM, Coleen Phillimore wrote:
>>
>> It would be nice to get an opinion from the compiler group.  The 
>> deopt_blob had this stack banging as some sort of
>> assertion check that the underlying compiled code already had the 
>> stack banging. I'm not sure how helpful it is if it
>> crashes here in finding any problems, because I'm not sure how well 
>> stack walking for implicit exceptions works through
>> a deopt blob.  My thought is that the code never expects an implicit 
>> exception in a deopt blob so wouldn't work very well.
>>
>> If this is the case, Gerald could simply remove this code from deopt 
>> blob, avoiding the sizing issue entirely.
>>
>> Thanks,
>> Coleen
>>
>>
>> On 7/13/15 10:17 AM, gerard ziemski wrote:
>>> hi David,
>>>
>>> On 07/13/2015 12:34 AM, David Holmes wrote:
>>>> Hi Gerard,
>>>>
>>>> On 11/07/2015 7:09 AM, gerard ziemski wrote:
>>>>> (resending - forgot to include the issue number in the title)
>>>>>
>>>>> Hi all,
>>>>>
>>>>> Please review this very small fix:
>>>>>
>>>>>     bug: https://bugs.openjdk.java.net/browse/JDK-8079156
>>>>> webrev: http://cr.openjdk.java.net/~gziemski/8079156_rev0
>>>>
>>>> I'd like to hear from the compiler folk about this bug as I'm 
>>>> unclear on why StackBanging adds to the code buffer,
>>>> and why this suddenly seems to be a new problem - did something 
>>>> change? Or have we not exercised the range of values
>>>> before?
>>>
>>> My best educated quess is that the issue was always in there, but we 
>>> never exercised that path - Dmity's only added
>>> his testing framework for exercising the ranges after we got the 
>>> range/constraints check feature in recently and
>>> that's when we found it.
>>>
>>>
>>>>
>>>> I'd also like to understand whether the code buffer resizing should 
>>>> be rounded up (or whether it will be rounded up
>>>> internally)? e.g. power of two, multiple of nK for some n etc.
>>>
>>> I checked the sizes of existing CodeBuffers instr sizes and some 
>>> values I saw are: 416,536,544,560,568, so I'm not
>>> sure what the constraint here is if there really is one (other than 
>>> divisible by 4, which 112 is as well)
>>>
>>>
>>>>
>>>> The 112 value seems odd for 50 pages - is this 2 bytes (words?) per 
>>>> page plus some fixed overhead? Can it be
>>>> expressed as a function of StackShadowPages rather than hardwiring 
>>>> to 112 which only works for values < 50?
>>>
>>> Hardcoding the value for 50 pages should be OK here since that's the 
>>> max value that StackShadowPages can take.
>>>
>>> Expressing it as some function would not be all that simple - you 
>>> would need to take in account that the default size
>>> is enough for some StackShadowPages (ie.32), then find out the fixed 
>>> size for the stack banging function. In the end
>>> you would end up with some hardcoded values anyhow, so why not make 
>>> it super simple as we did here?
>>>
>>> The other way is to calculate things dynamically and I actually did 
>>> that: my first fix was based on creating a temp
>>> CodeBuffer and feeding it only shadow stack banging code to find out 
>>> the exact size requirement for that code, but I
>>> was told that this might confuse some compiler code later that 
>>> wouldn't expect it. The other unknown was whether the
>>> temp code buffer code actually made it into in the cache (is it 
>>> flushed by the destructor?). I tried to find a way to
>>> wipe out the instr section before the destructor, but couldn't find 
>>> any APIs for doing so.
>>>
>>> I don't know the answers to those issues, so even though I liked the 
>>> idea of using a temp buffer to find out precisely
>>> how much more memory we used, in the end I settled on the simplest 
>>> solution that works.
>>>
>>> Would folks from the compiler like to comment?
>>>
>>>
>>> cheers
>>>
>>
>
>