RFR - Avoid G1 write barriers on newly allocated objects

[Igor Veresov]

Sorry that I couldn’t get this for so long.
The C2 part seems very good. The only comment is that the code (the post-barrier elimination part) depends on the fact the the new object is allocated in the young gen. Would it make sense to harden the code a little bit in case G1 gets single-generation or changes its allocation scheme? Like adding G1CollectedHeap::new_objects_require_post_barrier() or something like that? Anyways, up to you, reviewed.

igor

On Sep 16, 2014, at 2:41 PM, Staffan Friberg <staffan.friberg at oracle.com> 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
>>>>> 
>>> 
>