RFR (S): 8014972: Crash with specific values for -XX:InitialCodeCacheSize=500K -XX:ReservedCodeCacheSize=500k

[Albert Noll]

Hi Vladimir,
thanks for the feedback.

I implemented the suggested changes. Here is the new webrev:
http://cr.openjdk.java.net/~anoll/8014972/webrev.02/ 
<http://cr.openjdk.java.net/%7Eanoll/8014972/webrev.02/>

On 21.06.2013 19:38, Vladimir Kozlov wrote:
> I don't like name, may be CodeCacheMinimumUseSpace is better since it 
> complements CodeCacheMinimumFreeSpace.
Done.
> An other thing this flag should be develop_pd() platform dependent 
> because on different platforms you need different size. Zero or Core 
> are not using template interpreter so they can run with smaller 
> codecache.
Done. I set the CodeCacheMinimumUseSpace for Zero to 200k, since the 
template interpreter requires around 200K.

> Now back to changes. You did not add < 2*G check as I asked. No need 
> for flag.
Sorry, I forgot to mention in the last mail that I thought that checking 
the upper limit is a different bug ( 
https://jbs.oracle.com/bugs/browse/JDK-8015635). I added the check in 
the current version.

> First check should be since it can't be less then physical page size 
> (4K) InitialCodeCacheSize should be multiplier of CodeCacheExpansionSize:
>
>  if (InitialCodeCacheSize < CodeCacheExpansionSize)
>
I added an additional check in arguments.cpp that validates the 
parameter  CodeCacheExpansionSize.
CodeCacheExpansionSize is now required to be

1) larger than 0 and
2) a multiple of os::vm_page_size();

So, the first check is: if(InitialCodeCacheSize < 
(uintx)os::vm_page_size()). I hope that's fine.

Best,
Albert

> Thanks,
> Vladimir
>
> On 6/21/13 1:35 AM, Albert Noll wrote:
>> Hi,
>>
>> Vladimir, thanks for your comments. The solution was indeed more
>> complicated than necessary.
>>
>> The current version is much simpler. I think the cleanest solution is to
>> introduce the parameter "BootstrapCodeCacheSize" (an alternative name
>> would be StartupCodeCacheSize) to define the amount of memory required
>> to startup the VM. This value does not depend on InitialCodeCacheSize
>> and CodeCacheMinimumFreeSpace, which makes the necessary checks simpler.
>>
>> webrev: http://cr.openjdk.java.net/~anoll/8014972/webrev.01/
>> <http://cr.openjdk.java.net/%7Eanoll/8014972/webrev.01/>
>>
>> Thanks,
>> Albert
>>
>>
>> On 20.06.2013 22:46, Vladimir Kozlov wrote:
>>> Also it could be confusing that MinimumCodeCacheSize is less then
>>> CodeCacheMinimumFreeSpace. So the definition should be:
>>>
>>> develop(uintx, MinimumInitialCodeCacheSize, 900*K,
>>>
>>> And in arguments.cpp you need to do next since
>>> CodeCacheMinimumFreeSpace could be set on command line:
>>>
>>> uint min_code_cache_size = (K * NOT_DEBUG(400) DEBUG_ONLY(1200)) +
>>> CodeCacheMinimumFreeSpace;
>>> min_code_cache_size = MIN2(MinimumCodeCacheSize, min_code_cache_size);
>>>
>>> thanks,
>>> Vladimir
>>>
>>> On 6/20/13 1:27 PM, Vladimir Kozlov wrote:
>>>> Albert,
>>>>
>>>> Please, place new flag in globals.hpp after InitialCodeCacheSize
>>>> definition since they are related.
>>>>
>>>> MinimumInitialCodeCacheSize should be MinimumCodeCacheSize.
>>>> We also asked to check upper limit since we can use > 2Gb (MAXINT) 
>>>> size
>>>> for CodeCache.
>>>>
>>>> Also your changes could be much simpler if you just add the check
>>>> (InitialCodeCacheSize < min_code_cache_size) before existing check
>>>> (ReservedCodeCacheSize < InitialCodeCacheSize).
>>>>
>>>> And, please, don't use different message for debug build.
>>>>
>>>> '* 3' for debug build could be done as:
>>>>
>>>> uint min_code_cache_size = MinimumInitialCodeCacheSize DEBUG_ONLY(* 
>>>> 3) +
>>>> CodeCacheMinimumFreeSpace;
>>>>
>>>> thanks,
>>>> Vladimir
>>>>
>>>> On 6/20/13 4:02 AM, Albert Noll wrote:
>>>>> Hi,
>>>>>
>>>>> thanks for reviewing this small patch.
>>>>>
>>>>> Albert
>>>>>
>>>>> jbs: https://jbs.oracle.com/bugs/browse/JDK-8014972
>>>>> webrev: http://cr.openjdk.java.net/~anoll/8014972/webrev.00/
>>>>> <http://cr.openjdk.java.net/%7Eanoll/8014972/webrev.00/>
>>>>>
>>>>>
>>>>> Problem:
>>>>> If -XX:ReservedCodeCacheSize is set too small, the VM crashes when
>>>>> starting up.
>>>>>
>>>>> Solution:
>>>>> Introduce a minimum code cache size (ReservedCodeCacheSize) that
>>>>> guarantees that the VM can startup. The minimum code cache size is
>>>>> defined by a new parameter (MinimumInitialCodeCacheSize) that 
>>>>> cannot be
>>>>> changed in product build. My reasoning for this is that smaller 
>>>>> values
>>>>> make the JVM crash at startup. However, in a development build, the
>>>>> parameter can be changed.
>>>>>
>>>>> The current patch sets MinimumInitialCodeCacheSize to 400k for all
>>>>> platforms. On Solaris, MinimumInitialCodeCacheSize could be set to 
>>>>> 300k
>>>>> and the VM still starts up. However, I am not sure if it makes 
>>>>> sense to
>>>>> make MinimumInitialCodeCacheSize platform-dependent so that we can
>>>>> potentially save 100k on Solaris. If there is a reason, please let me
>>>>> know.
>>>>>
>>>>> Also note that the InitialCodeCacheSize must be > 0. The reason is 
>>>>> that
>>>>> otherwise the following  assertion fails:
>>>>>
>>>>> Internal Error
>>>>> (/tmp/workspace/2-build-solaris-i586/jdk8/4767/hotspot/src/share/vm/memory/heap.cpp:50), 
>>>>>
>>>>>
>>>>>
>>>>> pid=27190, tid=2
>>>>> #  assert(0 <= beg && beg < _number_of_committed_segments) failed:
>>>>> interval begin out of bounds
>>>>>
>>>>>
>>>>> Testing:
>>>>> With the current value of MinimumInitialCodeCacheSize, the VM 
>>>>> starts up
>>>>> on all Windows, Linux, and Solaris.
>>>>>
>>>>> Note that we can still get exceptions (and the JVM cannot continue to
>>>>> execute) if the value for CodeCacheMinimumFreeSpace is to small (in
>>>>> that
>>>>> case, here is not enough space for adapters) .
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20130626/103fbcee/attachment.html