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

[Albert Noll]

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/20130621/e9186c3a/attachment-0001.html