RFE: 8023597: Optimize G1 barriers code for unsafe load_store
martin.doerr at sap.com
Tue Aug 27 06:23:07 PDT 2013
thanks for your input and explanations.
It makes sense to keep the code more generic. Here's a new webrev:
Please review. Thanks.
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
Sent: Montag, 26. August 2013 21:30
To: Doerr, Martin
Cc: Tony Printezis; hotspot compiler; hotspot-gc-dev at openjdk.java.net
Subject: Re: RFE: 8023597: Optimize G1 barriers code for unsafe load_store
I think you need to call pre_barrier() anyway and have the NULL pointer
check in g1_write_barrier_pre() when pre_val is passed in
(do_load==false). Method pre_barrier() is common interface for all
collectors and decision about NULL_PTR could be different.
For the same reason you can move pre_barrier() for LS_xchg only if used
collector allows that. I think you need new method
GraphKit::can_move_pre_barrier() or similar.
Yes, all current collector allows that but it may be not true in a future.
> Vladimir, are you familiar with what Tony has mentioned wrt. young
> Maybe we can optimize out more barriers.
Yes, we do that (skipping GC barriers) for oop stored for CMS, see
GraphKit::write_barrier_post(). For arraycopy for all collectors (see
ReduceInitialCardMarks). And for initializing stores (collected by
For G1 store oops, it is done by generated code, if I understand
correctly, by checking in pre-barrier the field satb_mark_queue._active
and checking cardmark in post-barrier.
Yes, we can improve barriers generation for G1. For example, we have
On 8/26/13 7:13 AM, Doerr, Martin wrote:
> Hi all,
> I have created a new webrev:
> The comment
> "Transformation of a value which could be NULL pointer could be delayed during Parse"
> directly above my change seems to explain why the null reference with basic type T_ADDRESS was there.
> So I assume we should handle oldval the same way as newval.
> The new webrev elides the pre-barrier in this case in which oldval is a null reference.
> Please double-check if it's correct. Jvm2008 derby is running without assertions, now.
> Vladimir, are you familiar with what Tony has mentioned wrt. young gen writes?
> Maybe we can optimize out more barriers.
> Best regards,
> -----Original Message-----
> From: Tony Printezis [mailto:tprintezis at twitter.com]
> Sent: Montag, 26. August 2013 14:55
> To: Doerr, Martin
> Cc: Vladimir Kozlov; hotspot compiler; hotspot-gc-dev at openjdk.java.net
> Subject: Re: RFE: 8023597: Optimize G1 barriers code for unsafe load_store
> On 8/26/13 4:52 AM, Doerr, Martin wrote:
>> Regardless of the type issue, I guess we should not generate any pre-barrier code when C2 can
>> determine that oldval is a null reference.
> This is totally correct, the pre-barrier can be elided if the pre-value
> is null (no point in enqueueing null!).
> Additionally, the pre-barrier can also be elided for writes in the young
> gen. IIRC, there is (or at least used to be!) some code in C2 to elide
> card table writes for objects in the young gen (a fast path allocation
> is guaranteed to allocate the object in the young gen and the object
> cannot move until the next safepoint). So, if it's not done already, you
> could take advantage of the same mechanism to also elide pre-barriers
> (and post-barriers of course).
>> -----Original Message-----
>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>> Sent: Samstag, 24. August 2013 02:36
>> To: Doerr, Martin
>> Cc: hotspot compiler; hotspot-gc-dev at openjdk.java.net
>> Subject: Re: RFE: 8023597: Optimize G1 barriers code for unsafe load_store
>> On 8/23/13 9:09 AM, Doerr, Martin wrote:
>>> I did some more tests with our proposed webrev
>>> and it happened that the assertion
>>> "assert(pre_val->bottom_type()->basic_type() == T_OBJECT, "or we
>>> shouldn't be here");"
>>> was firing when running jvm2008 derby on linuxx86_64.
>>> What happened is that the C2 was inserting a barrier of kind=LibraryCallKit::LS_cmpxchg,
>>> but oldval was a ConP node with bottom_type()->basic_type() = T_ADDRESS.
>> What is ConP (oldval->dump())? I am worried that compareAndSwapObject()
>> is used not for object.
>>> I believe there's nothing logically wrong with the change.
>>> Should the assertion be modified or should we filter out cases in which oldval
>>> does not contain a valid Oop?
>>> I guess some advice from C2 and G1 experts is needed.
>>> Best regards,
More information about the hotspot-compiler-dev