Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions src/hotspot/share/gc/g1/g1ConcurrentMark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2341,20 +2341,16 @@ void G1CMTask::drain_local_queue(bool partially) {
}
}

size_t G1CMTask::start_partial_array_processing(oop obj) {
size_t G1CMTask::start_partial_array_processing(objArrayOop obj) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the arg name can be obj_array to emphasize that it's an objArray.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe.

I prefer obj since it is a more common name. It seems like only this function and its counter part in the full GC named this obj_array. There's almost 50 occurrences that just call them obj.

I also don't like giving local variables multi-word names unless it really helps with the readability and understanding of the function. I don't think it does in this case.

assert(should_be_sliced(obj), "Must be an array object %d and large %zu", obj->is_objArray(), obj->size());

objArrayOop obj_array = objArrayOop(obj);
size_t array_length = obj_array->length();

size_t initial_chunk_size = _partial_array_splitter.start(_task_queue, obj_array, nullptr, array_length);

// Mark objArray klass metadata
if (_cm_oop_closure->do_metadata()) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check has been removed. Marking closures should follow the metadata and there's no need to check if it should.

_cm_oop_closure->do_klass(obj_array->klass());
}
process_klass(obj->klass());

size_t array_length = obj->length();
size_t initial_chunk_size = _partial_array_splitter.start(_task_queue, obj, nullptr, array_length);

process_array_chunk(obj_array, 0, initial_chunk_size);
process_array_chunk(obj, 0, initial_chunk_size);

// Include object header size
return objArrayOopDesc::object_size(checked_cast<int>(initial_chunk_size));
Expand Down
7 changes: 4 additions & 3 deletions src/hotspot/share/gc/g1/g1ConcurrentMark.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -844,12 +844,10 @@ class G1CMTask : public TerminatorTerminator {
// mark bitmap scan, and so needs to be pushed onto the mark stack.
bool is_below_finger(oop obj, HeapWord* global_finger) const;

template<bool scan> void process_grey_task_entry(G1TaskQueueEntry task_entry, bool stolen);

static bool should_be_sliced(oop obj);
// Start processing the given objArrayOop by first pushing its continuations and
// then scanning the first chunk including the header.
size_t start_partial_array_processing(oop obj);
size_t start_partial_array_processing(objArrayOop obj);
// Process the given continuation. Returns the number of words scanned.
size_t process_partial_array(const G1TaskQueueEntry& task, bool stolen);
// Apply the closure to the given range of elements in the objArray.
Expand Down Expand Up @@ -918,6 +916,9 @@ class G1CMTask : public TerminatorTerminator {
template <class T>
inline bool deal_with_reference(T* p);

// Scan the klass and visit its children.
inline void process_klass(Klass* klass);

// Scans an object and visits its children.
inline void process_entry(G1TaskQueueEntry task_entry, bool stolen);

Expand Down
44 changes: 20 additions & 24 deletions src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,27 @@ inline void G1CMMarkStack::iterate(Fn fn) const {
}
#endif

inline void G1CMTask::process_klass(Klass* klass) {
_cm_oop_closure->do_klass(klass);
}

// It scans an object and visits its children.
inline void G1CMTask::process_entry(G1TaskQueueEntry task_entry, bool stolen) {
process_grey_task_entry<true>(task_entry, stolen);
assert(task_entry.is_partial_array_state() || _mark_bitmap->is_marked(cast_from_oop<HeapWord*>(task_entry.to_oop())),
"Any stolen object should be a slice or marked");

if (task_entry.is_partial_array_state()) {
_words_scanned += process_partial_array(task_entry, stolen);
} else {
oop obj = task_entry.to_oop();
if (should_be_sliced(obj)) {
_words_scanned += start_partial_array_processing(objArrayOop(obj));
} else {
_words_scanned += obj->oop_iterate_size(_cm_oop_closure);
}
}

check_limits();
}

inline void G1CMTask::push(G1TaskQueueEntry task_entry) {
Expand Down Expand Up @@ -160,27 +178,6 @@ inline bool G1CMTask::is_below_finger(oop obj, HeapWord* global_finger) const {
return objAddr < global_finger;
}

template<bool scan>
inline void G1CMTask::process_grey_task_entry(G1TaskQueueEntry task_entry, bool stolen) {
assert(scan || (!task_entry.is_partial_array_state() && task_entry.to_oop()->is_typeArray()), "Skipping scan of grey non-typeArray");
assert(task_entry.is_partial_array_state() || _mark_bitmap->is_marked(cast_from_oop<HeapWord*>(task_entry.to_oop())),
"Any stolen object should be a slice or marked");

if (scan) {
if (task_entry.is_partial_array_state()) {
_words_scanned += process_partial_array(task_entry, stolen);
} else {
oop obj = task_entry.to_oop();
if (should_be_sliced(obj)) {
_words_scanned += start_partial_array_processing(obj);
} else {
_words_scanned += obj->oop_iterate_size(_cm_oop_closure);
}
}
}
check_limits();
}

inline bool G1CMTask::should_be_sliced(oop obj) {
return obj->is_objArray() && ((objArrayOop)obj)->length() >= (int)ObjArrayMarkingStride;
}
Expand Down Expand Up @@ -272,7 +269,6 @@ inline bool G1CMTask::make_reference_grey(oop obj) {
// be pushed on the stack. So, some duplicate work, but no
// correctness problems.
if (is_below_finger(obj, global_finger)) {
G1TaskQueueEntry entry(obj);
if (obj->is_typeArray()) {
// Immediately process arrays of primitive types, rather
// than pushing on the mark stack. This keeps us from
Expand All @@ -284,8 +280,8 @@ inline bool G1CMTask::make_reference_grey(oop obj) {
// by only doing a bookkeeping update and avoiding the
// actual scan of the object - a typeArray contains no
// references, and the metadata is built-in.
process_grey_task_entry<false>(entry, false /* stolen */);
} else {
G1TaskQueueEntry entry(obj);
push(entry);
}
}
Expand Down