RFR - Avoid G1 write barriers on newly allocated objects

[Staffan Friberg]

This is handled already for humongous objects, they will generate 
deferred card marks for as part of their allocation if they are 
allocated outside of the young space.
See CollectedHeap::new_store_pre_barrier and 
OptoRuntime::new_store_pre_barrier for details.

Agree that it could be made more visible, but it is probably better 
handled that as a G1 update when required.

Thanks for the review.

//Staffan

On 10/03/2014 02:37 PM, Igor Veresov wrote:
> 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