RFR(XS): 8024008: Nashorn V8 (Crypto) benchmark crashes with small CodeCache size

[Vladimir Kozlov]

Albert,

I don't see connection between not generating wrapper (which is fine, it 
is only needed for hot calls to native method) and wrong return address 
when calling i2c adapter (failed message). Can you find what return 
address is?

It could be a different problem (or not) from what this bug about. 
Several things can go bad when codecache is full. Also attached to bug 
report hs_err files shows different crushes.

Was you able to reproduce it with fastdebug VM? Switch off 
-XX:-CheckCompressedOops to get the same code generation as with product 
64-bit VM. And switch off -XX:-VerifyDependencies to speed testing (10x, 
this test is very expensive).

Also can you try to run with latest VM version which has fixes to 
produce more info for call stack in hs_err. To have only next lines is 
not useful:

C  0x0000000648baace0
[error occurred during error reporting (printing native stack), id 0xb]

Thanks,
Vladimir

On 10/3/13 11:17 AM, Albert Noll wrote:
> Hi,
>
> I think I found the reason for the crash. Please forget the previous
> suggestions.
>
> See the following webrev:
>
> Problem:
>    http://cr.openjdk.java.net/~anoll/8024008/webrev.02/
>
>    If the code cache gets full, this code in compileBroker.cpp is not
> executed.
>
>    1318       (void)
> AdapterHandlerLibrary::create_native_wrapper(method, compile_id);
>
>    As a result, we get the following error "i2c adapter must return to
> an interpreter frame"
>    if the JVM is called with "-XX:+UnlockDiagnosticVMOptions
> -XX:+VerifyAdapterCalls"
>
>    I do not yet fully understand why that's the case, so please if you
> know let me know.
>
> Solution:
>    Move the code cache is full-check  after line 1318.
>
> Many thanks,
> Albert
>
>
> Am 11.09.2013 01:07, schrieb Vladimir Kozlov:
>> Albert,
>>
>> I would suggest to run as many tests as possible to verify correctness
>> of changes because this code is very important.
>>
>> Thanks,
>> Vladimir
>>
>> On 9/10/13 5:52 AM, Albert Noll wrote:
>>> Hi,
>>>
>>> the current version causes a serious performance regression.
>>> I have to figure out what's wrong.
>>>
>>> Best,
>>> Albert
>>>
>>> On 10.09.2013 09:03, Albert Noll wrote:
>>>> Hi Vladimir,
>>>>
>>>> thanks for looking at this. I checked all call sites of
>>>> 'make_not_entrant()' and 'make_zombie()'. I do not see any problems.
>>>> The worst thing that can happen is that an nmethod for which the call
>>>> returns fals gets removed later than expected.
>>>>
>>>> I updated the comment and did some minor code refactoring.
>>>>
>>>> Also I think that it is worth considering to backport this change,
>>>> since the bug can appear any time
>>>> the code cache fills up. If that bug appears, it is extremely hard to
>>>> reproduce and debug.
>>>>
>>>> Here is the updated webrev.
>>>> http://cr.openjdk.java.net/~anoll/8024008/webrev.01/
>>>> <http://cr.openjdk.java.net/%7Eanoll/8024008/webrev.01/>
>>>>
>>>> Best,
>>>> Albert
>>>>
>>>>
>>>>
>>>> On 09.09.2013 19:47, Vladimir Kozlov wrote:
>>>>> Several places in sources do not check returned result of
>>>>> make_not_entrant_or_zombie(). And they may assume that method marked
>>>>> as non-entrant, for example, because before we returned 'false' only
>>>>> when an other thread already changed the state to one we need.
>>>>>
>>>>> Can you check all call sites to make sure they work correctly with
>>>>> this change?
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 9/9/13 6:59 AM, Albert Noll wrote:
>>>>>> Hi,
>>>>>>
>>>>>> thanks for reviewing this patch.
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8024008
>>>>>> http://cr.openjdk.java.net/~anoll/8024008/webrev.00/
>>>>>> <http://cr.openjdk.java.net/%7Eanoll/8024008/webrev.00/>
>>>>>>
>>>>>> Many thanks in advance,
>>>>>> Albert
>>>>>>
>>>>>> Problem:
>>>>>> The state of nmethods that are currently locked by the VM must not
>>>>>> change.
>>>>>> Some operations such as setting ICs (e.g.,
>>>>>> SharedRuntime::resolve_sub_helper())
>>>>>> rely on this fact.  'nmethod::make_not_entrant_or_zombie does not
>>>>>> make this
>>>>>> check.
>>>>>>
>>>>>> Solution:
>>>>>> Do method state unchanged of nmethod is locked by the VM.
>>>>>>
>>>>>> Testing: Run Octane benchmarks on top of Nashorn with small code
>>>>>> cache size (16m).
>>>>>>
>>>>
>>>
>