[9] RFR (M): 8086053: Address inconsistencies regarding ZeroTLAB

[Zoltán Majó]

Hi Vladimir,


On 01/12/2016 06:00 PM, Vladimir Kozlov wrote:
> Looks good. The test may still have a problem on slow platforms with 
> -Xcomp. We may need to increase timeout later.

thank you for pointing that out! I'll keep an eye on the test.

Best regards,


Zoltan

>
> Thanks,
> Vladimir
>
> On 1/12/16 12:00 AM, Zoltán Majó wrote:
>> Hi Vladimir,
>>
>>
>> On 01/11/2016 08:26 PM, Vladimir Kozlov wrote:
>>> Don't use GC flags in the test. They will conflict with flags passed 
>>> by testing infra and the test will fail. The was
>>> bug fixed by removing GC flags from all our tests.
>>> Note, Nightly testing does GC flags rotation so you don't need to do 
>>> that.
>>
>> OK, I removed all GC flags from the test. Here is the updated webrev:
>> http://cr.openjdk.java.net/~zmajo/8086053/webrev.03/
>>
>>> Otherwise looks good.
>>
>> Thank you for the review! I'll push webrev.03 today as it addresses 
>> all issues that were brought up.
>>
>> Thank you and best regards,
>>
>>
>> Zoltan
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 1/11/16 5:37 AM, Zoltán Majó wrote:
>>>> Hi,
>>>>
>>>>
>>>>
>>>> On 01/11/2016 02:11 PM, Zoltán Majó wrote:
>>>>> [...]
>>>>> Yes, that is a good idea. I added a test that launches the VM with 
>>>>> all flag combinations and also with different GCs.
>>>>> I did the same what the test does to reproduce the original failure.
>>>>>
>>>>> Here is the updated webrev:
>>>>> http://cr.openjdk.java.net/~zmajo/8086053/webrev.01/
>>>>
>>>> The test contains and unnecessary @library tag and package import. 
>>>> The year in the copyright statement must be changed
>>>> as well (to 2016).
>>>>
>>>> Here is the webrev with those changes:
>>>> http://cr.openjdk.java.net/~zmajo/8086053/webrev.02/
>>>>
>>>> Sorry for the noise.
>>>>
>>>> Thank you and best regards,
>>>>
>>>>
>>>> Zoltan
>>>>
>>>>
>>>>>
>>>>> The newly added test passes on all supported platforms.
>>>>>
>>>>> Thank you and best regards,
>>>>>
>>>>>
>>>>> Zoltan
>>>>>
>>>>>>
>>>>>> Best,
>>>>>> Tobias
>>>>>>
>>>>>>
>>>>>> On 08.01.2016 12:06, Zoltán Majó wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>>
>>>>>>> please review the patch for 8086053.
>>>>>>>
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8086053
>>>>>>>
>>>>>>> Problem: With ZeroTLAB enabled, the GC is supposed to zero-fill 
>>>>>>> newly allocated TLAB regions. With ZeroTLAB
>>>>>>> disabled, the interpreter and compiled code should assume the 
>>>>>>> responsibility to zero-fill newly allocated regions.
>>>>>>> Currently, the handling of the ZeroTLAB flag shows some 
>>>>>>> inconsistencies between the GC and the compilers. These
>>>>>>> inconsistencies lead to newly allocated regions not being filled 
>>>>>>> with zeros.
>>>>>>>
>>>>>>> Solution: Address the following:
>>>>>>> - With -XX:+FastTLABRefill, C1-compiled code refills the TLAB 
>>>>>>> without notifying the GC. As a result, the newly
>>>>>>> allocated TLAB is not initialized with zero. Add TLAB 
>>>>>>> initialization code to C1.
>>>>>>> - With -XX:+ZeroTLAB, the C2 compiler skips zero-initialization 
>>>>>>> of newly allocated objects/arrays even if TLAB
>>>>>>> allocation is disabled. Add stricter conditions to C2 on when to 
>>>>>>> skip filling objects/arrays with zero.
>>>>>>>
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~zmajo/8086053/webrev.00/
>>>>>>>
>>>>>>> Testing:
>>>>>>> - local testing (linux_x86_64) of failing test case with 
>>>>>>> -XX:+UseG1GC and -XX:+UseSerialGC;
>>>>>>> - JPRT;
>>>>>>> - all hotspot tests on all platforms affected by the change 
>>>>>>> using all combinations of +/-UseTLAB and +/-ZeroTLAB.
>>>>>>>
>>>>>>> Thank you and best regards,
>>>>>>>
>>>>>>>
>>>>>>> Zoltan
>>>>>>>
>>>>>
>>>>
>>