RFR(s): 8144573: TLABWasteIncrement=max_jint fires an assert on SPARC for non-G1 GC mode

sangheon sangheon.kim at oracle.com
Thu Jan 7 00:12:12 UTC 2016


Thanks for the review.

Sangheon


On 01/06/2016 04:01 PM, Igor Veresov wrote:
> That looks good to me.
>
> igor
>
>> On Jan 6, 2016, at 3:50 PM, sangheon <sangheon.kim at oracle.com> wrote:
>>
>> Hi Igor,
>>
>> Thank you for reviewing this.
>>
>> On 01/05/2016 08:29 PM, Igor Veresov wrote:
>>> I’m not sure we care a lot about tiny bits of performance in the this instance… But, in case use wanted to keep the original code for the simm13 case you could check the range of the constant and still emit the code that was there before. It also seems suboptimal to do set64 in MacroAssembler::tlab_refill() on all paths - the result of the original add in the delay slot doesn’t seem to be used if we jump to discard_tlab, right?
>> You are right.
>> If the branch is taken, original add in the delay slot is not used.
>>
>> The reason of always calling 'set64' was to keep its behavior. i.e. same order of doing something before branch within delay slot. But as you said, it is less tighter code.
>>
>>>    So, may be you could do something like:
>>>
>>> brx(Assembler::lessEqual, false, Assembler::pt, discard_tlab);
>>> if (is_simm13(ThreadLocalAllocBuffer::refill_waste_limit_increment())) {
>>>    delayed()->add(t2, ThreadLocalAllocBuffer::refill_waste_limit_increment(), t2);
>>> } else {
>>>    delayed()->nop();
>>>    set64(ThreadLocalAllocBuffer::refill_waste_limit_increment(), t3, G0);
>>>    add(t2, t3, t2);
>>> }
>> Okay, checking its value first seems good idea.
>>
>>> Similarly, tighter code can be emitted for the interpreter in templateTable_sparc.cpp.
>> Okay, done.
>>
>> Webrev: http://cr.openjdk.java.net/~sangheki/8144573/webrev.01
>>
>> Thanks,
>> Sangheon
>>
>>
>>> igor
>>>
>>>
>>>> On Jan 5, 2016, at 4:31 PM, sangheon <sangheon.kim at oracle.com> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> Could I have reviews for the below change to remove size limitation(<4096) of TLABWasteIncrement on SPARC?
>>>>
>>>> Current implementation uses 'add(Register, int, Register)' which has 13bit limitation for 'int' parameter.
>>>> I changed to use 'set64' to load the value to register and then call 'add'. 'set64' will run cheap path as the range of TLABWasteIncrememt is (0, max_juint).
>>>>
>>>> This assert is only fired on non-G1 mode as G1 is the only GC that returns false from Universe::heap()->supports_inline_contig_alloc() by default option. And this decides to fall that routine.
>>>>
>>>> I didn't add a test as current TestOptionsWithRanges.java is enough to test this case with nightly option rotation.
>>>>
>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8144573
>>>> Webrev: http://cr.openjdk.java.net/~sangheki/8144573/webrev.00/
>>>> Testing: JPRT, manual test on SPARC[1]
>>>>
>>>> [1]: java -XX:TLABWasteIncrement=4096(and some larger values as well) -XX:+UseConcMarkSweepGC(UseParallelGC and UseSerialGC) -version
>>>>
>>>> Thanks,
>>>> Sangheon




More information about the hotspot-gc-dev mailing list