RFR: 6979279: remove special-case code for ParallelGCThreads==0
Kim Barrett
kim.barrett at oracle.com
Thu Oct 16 17:32:52 UTC 2014
On Oct 15, 2014, at 8:31 AM, Marcus Larsson <marcus.larsson at oracle.com> wrote:
>
> Sorry for the noise. Forgot to fix some additional cases where checks if ParallelGCThreads > 0 could be removed.
>
> New webrev:
> http://cr.openjdk.java.net/~mlarsson/6979279/webrev.03/
>
> Incremental:
> http://cr.openjdk.java.net/~mlarsson/6979279/webrev.02-03/
==============================================================================
g1/g1HardCardCache.hpp - no changes?
==============================================================================
g1/satbQueue.cpp
277 void SATBMarkQueueSet::set_closure(int i, ObjectClosure* closure) {
278 assert(_closures != NULL, "Precondition");
279 _closures[i] = closure;
Perhaps add an assert that i is in proper range? That seems like a
more important check.
Also, wouldn't it be better if i were size_t rather than int? Since
you are changing the signature anyway.
==============================================================================
g1/satbQueue.hpp
108 // Register "blk" as "the closure" for all queues. Only one such closure
109 // is allowed. The "apply_closure_to_completed_buffer" method will apply
110 // this closure to a completed buffer, and "iterate_closure_all_threads"
111 // applies it to partially-filled buffers (the latter should only be done
112 // with the world stopped).
113 void set_closure(int i, ObjectClosure* closure);
I see no "blk" in the signature. [Pre-existing problem.]
The emphasis on "the closure" and "only one such closure" is left over
from the non-parallel version, and seems like it should be updated.
==============================================================================
memory/freeList.hpp
memory/freeList.cpp
assert_proper_lock_protection()
New implementation always involves a function call (to an empty
implementation #ifndef ASSERT), rather than being optimizable to
nothing #ifdef PRODUCT. So I think the part of the change to this
function around the use of PRODUCT/ASSERT is incorrect, and should be
reverted.
==============================================================================
memory/sharedHeap.cpp
71 if ((UseParNewGC ||
72 (UseConcMarkSweepGC && (CMSParallelInitialMarkEnabled ||
73 CMSParallelRemarkEnabled)) ||
74 UseG1GC) &&
75 use_parallel_gc_threads()) {
Seems like we shouldn't need call to use_parallel_gc_threads() here.
==============================================================================
gc_interface/collectedHeap.hpp
[not in current webrev]
Do we still need CollectedHeap::use_parallel_gc_threads() at all?
==============================================================================
More information about the hotspot-gc-dev
mailing list