RFR: 8201318: Introduce GCThreadLocalData to abstract GC-specific data belonging to a thread (original) (raw)

Per Liden per.liden at oracle.com
Wed Apr 11 12:38:02 UTC 2018

Thanks Martin!

/Per

On 04/11/2018 12:28 PM, Doerr, Martin wrote:

Hi Per,

thanks for simplifying the platform code. I just verified that it still builds on PPC64 and s390. Best regards, Martin

-----Original Message----- From: Per Liden [mailto:per.liden at oracle.com] Sent: Mittwoch, 11. April 2018 11:55 To: Aleksey Shipilev <shade at redhat.com>; Roman Kennke <rkennke at redhat.com>; Doerr, Martin <martin.doerr at sap.com>; hotspot-dev at openjdk.java.net; Robbin Ehn <robbin.ehn at oracle.com>; OSTERLUND,ERIK <erik.osterlund at oracle.com> Subject: Re: RFR: 8201318: Introduce GCThreadLocalData to abstract GC-specific data belonging to a thread Hi Aleksey, On 04/10/2018 05:04 PM, Aleksey Shipilev wrote: On 04/10/2018 02:51 PM, Per Liden wrote: A couple of commits were pushed, which causes conflicts with this change, so here's a rebased version:

http://cr.openjdk.java.net/~pliden/8201318/webrev.1 *) I wonder if we can make the special accessors for the composite offsets? E.g. instead of Address inprogress(thread, inbytes(G1ThreadLocalData::satbmarkqueueoffset() + SATBMarkQueue::byteoffsetofactive())); Address queueindex(thread, inbytes(G1ThreadLocalData::satbmarkqueueoffset() + SATBMarkQueue::byteoffsetofindex())); Address buffer(thread, inbytes(G1ThreadLocalData::satbmarkqueueoffset() + SATBMarkQueue::byteoffsetofbuf())); ...do: Address inprogress(thread, inbytes(G1ThreadLocalData::satbmarkqueueactiveoffset())); Address queueindex(thread, inbytes(G1ThreadLocalData::satbmarkqueueindexoffset())); Address buffer(thread, inbytes(G1ThreadLocalData::satbmarkqueuebufoffset())); That probably eliminates the header dependency on SATBMarkQueue/DirtyCardQueue from compiler/assembler stubs? Done. ) Should these new methods accept JavaThread? I.e. we are expecting the GC thread-local data only for JavaThreads, like attach/detach does? virtual void onthreadcreate(Thread* thread); virtual void onthreaddestroy(Thread* thread); virtual void onthreadattach(JavaThread* thread); virtual void onthreaddetach(JavaThread* thread); The intention is that onthreadcreate/destroy is called for all threads, where as onthreadattach/detach is only called for JavaThreads (i.e. threads that are added to the "threads list"). There's been discussions about treating all threads in the same way, i.e. they would all be on the threads list and they would all receive attach/detach calls. It's not clear yet if that is a good idea, but if that change is made in the future, then we might want to revisit this interface. ) Wait, why do we call JVMCI members JavaThread? 64 static int JavaThreadsatbmarkqueueoffset; 65 static int JavaThreaddirtycardqueueoffset; Good catch! I switched to using the declareconstantwithvalue() construct, so these members are now gone. *) I would probably add assert(UseG1GC) to G1ThreadLocalData accessor methods Done. Note however that we can't add asserts to the *offset() functions, as those will be called by Graal regardless of weather G1 is enabled or not. As discussed else where in this thread, the reason for keeping the GCThreadLocalData member early in Thread is to allow GC barriers to generate compact code. As Erik indicates, this may not matter that much in the end, but I also don't think it hurts to let it sit there. Also, I've shrunk GCThreadLocalData to 112 bytes, which is what G1 currently needs. ZGC currently uses 156 bytes (IIRC), but that's a change that should stay the ZGC repo for now. Updated webrev here: Diff: http://cr.openjdk.java.net/~pliden/8201318/webrev.1vs2/ Full: http://cr.openjdk.java.net/~pliden/8201318/webrev.2/ Thanks for reviewing. /Per Thanks, -Aleksey

More information about the hotspot-dev mailing list