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

Aleksey Shipilev shade at redhat.com
Wed Apr 11 10:13:55 UTC 2018

On 04/11/2018 11:54 AM, Per Liden wrote:

) 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.

Ah, I understand. After sending the reply yesterday I realized they might be legit use cases where you want to have GCTLD available e.g. for GC threads.

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.

Oh. Haven't expected we need that much. This makes me think if we should implement some sort of STATIC_ASSERT-based overflow checks? Not necessarily in this RFE.

Updated webrev here:

Diff: http://cr.openjdk.java.net/~pliden/8201318/webrev.1vs2/ Full: http://cr.openjdk.java.net/~pliden/8201318/webrev.2/

Looks good!

Minor nits (no need to re-review):

*) I'll probably do another sweep and do a tad better indenting work. Compare:

2178 Address in_progress(Rthread, in_bytes(G1ThreadLocalData::satb_mark_queue_active_offset())); 2179 Address index(Rthread, in_bytes(G1ThreadLocalData::satb_mark_queue_index_offset())); 2180 Address buffer(Rthread, in_bytes(G1ThreadLocalData::satb_mark_queue_buffer_offset()));

vs.

2178 Address in_progress(Rthread, in_bytes(G1ThreadLocalData::satb_mark_queue_active_offset())); 2179 Address index(Rthread, in_bytes(G1ThreadLocalData::satb_mark_queue_index_offset())); 2180 Address buffer(Rthread, in_bytes(G1ThreadLocalData::satb_mark_queue_buffer_offset()));

*) Typos: "provied" -> "provided", "over head" -> "overhead".

// Thread local data area for GC-specific information. Each GC // is free to decide the internal structure and contents of this // area. It is represented as a 64-bit aligned opaque blob to // avoid circular dependencies between Thread and all GCs. For // the same reason, the size of the data area is hard coded to // provied enough space for all current GCs. Adjust the size if // needed, but avoid making it excessively large as it adds to // the memory over head of creating a thread

*) Also mention the choice of <128 bytes? Like this:

// The size of GCTLD is kept under 128 bytes to let compilers encode // accesses to GCTLD members with short offsets from the Thread.

-Aleksey

More information about the hotspot-dev mailing list