RFR - Avoid G1 write barriers on newly allocated objects

Vladimir Kozlov vladimir.kozlov at oracle.com
Tue Sep 16 22:16:05 UTC 2014


Looks fine but you need to add '.' at the end of statements in the 
comment you modified. No need for re-review.

Thanks,
Vladimir

On 9/16/14 2:41 PM, Staffan Friberg wrote:
> Thanks Vladimir,
>
> Updated with your suggestions. Added a small comment in
> g1_can_remove_pre_barrier method since the alloc == NULL check is quite
> a bit earlier.
>
> New webrev, http://cr.openjdk.java.net/~sfriberg/8057737/webrev.03
>
> Cheers,
> Staffan
>
>
> On 09/15/2014 05:55 PM, Vladimir Kozlov wrote:
>> You need only check (alloc == st_alloc) because alloc != NULL at this
>> point:
>>
>> +    if (st_alloc != NULL && alloc == st_alloc) {
>>
>>
>> You can clean up g1_can_remove_pre_barrier() too (remove st_alloc ==
>> NULL check).
>>
>> I would also reverse the last check there because you bailout from
>> loop anyway:
>>
>> +        if (captured_store == NULL || captured_store ==
>> st_init->zero_memory()) {
>> +          return true;
>> +        }
>> +      }
>> +    }
>> +
>> +    // Unless there is an explicit 'continue', we must bail out here,
>>
>>
>> Vladimir
>>
>> On 9/15/14 3:31 PM, Staffan Friberg wrote:
>>> Hi Vladimir,
>>>
>>> Uploaded a new webrev with fixes to the below issues,
>>> http://cr.openjdk.java.net/~sfriberg/8057737/webrev.02
>>>
>>> Still removes expected barriers.
>>>
>>> Thanks,
>>> Staffan
>>>
>>> On 09/12/2014 04:15 PM, Vladimir Kozlov wrote:
>>>> Fix indention in 2 checks:
>>>>
>>>> +    if (use_ReduceInitialCardMarks() &&
>>>> +            g1_can_remove_*_barrier(&_gvn, adr, bt, alias_idx)) {
>>>>
>>>> Typo 'iff' in both comments:
>>>>
>>>> + * Returns true iff the barrier can be removed
>>>>
>>>> In g1_can_remove_post_barrier() use:
>>>>
>>>> +  // Start search from Store node
>>>> +  Node* ctrl = store->in(MemNode::Control);
>>>>
>>>> Control edge can't point to an other Store so you should not check it
>>>> inside loop.
>>>> As result you don't need loop.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 9/12/14 2:26 PM, Staffan Friberg wrote:
>>>>> Hi Mikael,
>>>>>
>>>>> I updated the documentation, and uploaded a new webrev,
>>>>> http://cr.openjdk.java.net/~sfriberg/8057737/webrev.01
>>>>>
>>>>> Thanks for agreeing to sponsor, and the good idea about pushing to
>>>>> hs-gc for extra coverage.
>>>>>
>>>>> Thanks,
>>>>> Staffan
>>>>>
>>>>> On 09/09/2014 07:31 AM, Mikael Gerdin wrote:
>>>>>> Hi Staffan,
>>>>>>
>>>>>> On Friday 05 September 2014 16.31.44 Staffan Friberg wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Copying both the compiler and GC alias as the optimization involves
>>>>>>> both
>>>>>>> groups.
>>>>>>>
>>>>>>> This optimization aims to avoid generating G1 barriers for newly
>>>>>>> allocated objects where the compiler can prove that the object
>>>>>>> has not
>>>>>>> been written to earlier and there is no safepoint between the
>>>>>>> allocation
>>>>>>> and the write. The bug has some further details and microbenchmark
>>>>>>> result. The new code has fairly extensive comments about the
>>>>>>> optimization.
>>>>>>>
>>>>>>> It would be great if the GC team can help validate that the
>>>>>>> premise of
>>>>>>> the optimization is correct as well.
>>>>>>>
>>>>>>> webrev: http://cr.openjdk.java.net/~sfriberg/8057737/webrev/
>>>>>> I mostly read through the comments since I can't decode what the IR
>>>>>> transforms
>>>>>> intend to do :)
>>>>>>
>>>>>> We usually stick to the terms "old generation" and "young
>>>>>> generation" instead
>>>>>> of "Old Space" and "Young Space".
>>>>>>
>>>>>> "G1 also requires to keep track of objects between different
>>>>>> + * regions to enable evacuation of old regions"
>>>>>> should probably be.
>>>>>> .."to keep track of references between different regions"
>>>>>>
>>>>>> I leave it to the compiler team to review the actual code changes.
>>>>>>
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8057737
>>>>>>>
>>>>>>> I would also need a sponsor for this change if it passes review.
>>>>>> I'll push this to hs-gc after review since we have more G1 test
>>>>>> coverage on
>>>>>> hs-gc.
>>>>>>
>>>>>> /Mikael
>>>>>>
>>>>>>> Thanks,
>>>>>>> Staffan
>>>>>
>>>
>



More information about the hotspot-gc-dev mailing list