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

[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).
>>>>
>>
>