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

[Vladimir Kozlov]

Sorry, I missed 8015635, lets do max size check as separate fix.

You don't need next comment in flag description:

"Decreasing the value can crash the VM when starting up."

Main flag's text could be:

"Minimum code cache size (in bytes) required to start VM"

Comment in arguments.cpp should be:

// Template Interpreter code is approximately 3X larger in debug builds.

Use vm_page_size in first check for CodeCacheExpansionSize:

parse_memory_size(tail, &long_CodeCacheExpansionSize, os::vm_page_size());

We don't check for value is multiplier because we do rounding (it is 
normal practice in Hotspot):

src/share/vm/code/codeCache.cpp:  CodeCacheExpansionSize = 
round_to(CodeCacheExpansionSize, os::vm_page_size());

So don't do second CodeCacheExpansionSize check.

And don't forget to set CodeCacheMinimumUseSpace in closed sources.

Thanks,
Vladimir

On 6/26/13 5:25 AM, Albert Noll wrote:
> 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) .
>>>
>