RFR: Update thread local bad mask during root scanning

Stefan Karlsson stefan.karlsson at oracle.com
Thu Feb 22 10:26:11 UTC 2018


On 2018-02-22 08:37, Per Liden wrote:
> Patch to update the thread local address bad mask during root scanning 
> while we're anyway walking the thread list (in parallel), instead of 
> doing a separate single threaded walk over that list.
> 
> This patch also removes some platform dependent code, and updates the 
> thread local bad mask on all platforms, even if it's strictly not needed 
> on e.g. Solaris/SPARC. However, it doesn't hurt performance wise, and 
> keeps the model nicely homogeneous across platforms.
> 
> Webrev: 
> http://cr.openjdk.java.net/~pliden/zgc/update_thread_local_address_bad_mask/webrev.0 

I'm OK with this patch, but Per and I discussed this briefly:

This patch has an optimization to reduce the number of times we iterate 
over the threads, by combining the thread mask setting and the root 
scanning. This is good for performance, but it breaks the abstraction of 
ZRootsIterator by letting it perform a task it's not intended to do.

The two tasks (root scanning and mask updating) are hard-coded in 
ZRootsIterator::do_threads and ZRootsIteratorThreadClosure, but we could 
at least decouple this, by extracting the mask updating in a closure 
that gets injected into ZRootsIterator. If more things were to be added 
to be done during the threads iteration, only the injected closure would 
have to be changed, but not the ZRootsIterator.

The decoupling has the unfortunate property that we would have to 
propagate the closure through a couple of layers, so for now we are 
leaving the code as proposed in the patch.

The layering would be something like this:

void ZHeap::mark_start() {
...
   ZAddressMasks::flip_to_marked(); // Original mask flipping location
...
   _mark.start(flip_to_marked_thread_closure()); // New closure
}

void ZMark::start(ThreadClosure* flipped_to_marked_cl) {
...
   ZMarkRootsTask task(this, flipped_to_marked_cl);
   _workers->run_parallel(&task);
}

ZMarkRootsTask(ZMark* mark, ThreadClosure* flipped_to_marked_cl) :
     ZTask("ZMarkRootsTask"),
     _mark(mark),
     _roots(flipped_to_marked_cl) {}

ZRootsIterator::ZRootsIterator(ThreadClosure* extra_thread_cl) :

and then ZRootsIterator::do_threads could apply the closure.

StefanK

> 
> 
> cheers,
> Per
> 


More information about the zgc-dev mailing list