RFR - Avoid G1 write barriers on newly allocated objects

[Mikael Gerdin]

Staffan,

On 10/07/2014 09:18 PM, Staffan Friberg wrote:
> 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.

It looks like this change is through the review process. If you send me 
a changeset patch with the final version of the change I'll push it to 
jdk9/hs-gc

/Mikael

>
> //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
>