RFR (S): 8183128: Update RefineCardTableEntryClosure
erik.helin at oracle.com
Thu Jul 6 12:52:27 UTC 2017
On 07/04/2017 10:24 AM, Thomas Schatzl wrote:
> Hi all,
> can I get reviews for this change that renames and cleans up the use
> of RefineCardTableEntryClosure in the code?
> RefineCardTableEntryClosure is the closure that is applied by the
> concurrent refinement threads. This change renames it slightly to
> indicate its use (G1RefineCardConcurrentlyClosure) and moves it to the
> G1RemSet files close to the closure that we use for refinement/Update
> RS during GC.
great cleanup! Looking at the code, what do you think about moving
G1RefineCardConcurrentlyClosure into concurrentG1RefineThread.cpp (and
make it a private class to ConcurrentG1RefineThread)? AFAICS,
ConcurrentG1RefineThread is the only code using this closure.
If we do it this way, then we can actually make
DirtyCardQueueSet::apply_closure_to_completed_buffer a template method,
taking the Closure a template, as in:
template <typename Closure>
bool apply_closure_to_completed_buffer(Closure* cl,
This means that closures could get inlined, which doesn't mean that much
for G1RefineCardConcurrentlyClosure, but could give a small boost for
G1RefineCardClosure (for that to work,
G1CollectedHeap::iterate_dirty_card_closure must take a
G1RefineCardClosure, but that is ok, because that is the only closure
type we pass to that method).
Also, you do not need the forward declaration in G1CollectedHeap, it
will not make use of this closure then :)
If you want to "go the extra mile", then you can also pass a G1RemSet*
as an argument to the G1RefineCardConcurrentlyClosure constructor and
store it in a field, to avoid accessing the G1CollectedHeap via the
(plus, G1RefineCardConcurrentlyClosure only needs a G1RemSet* pointer
> This change is dependent on "JDK-8183226: Remembered set summarization
> accesses not fully initialized java thread DCQS" which is also
> currently out for review - that change reorganizes G1CollectedHeap
> initialization so that the change can actually move the closure.
> jprt, local benchmarks
More information about the hotspot-gc-dev