-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8256516: Simplify clearing References #1286
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -88,8 +88,8 @@ static oop reference_referent(oop reference) { | |
return CompressedOops::decode(heap_oop); | ||
} | ||
|
||
static void reference_set_referent(oop reference, oop referent) { | ||
java_lang_ref_Reference::set_referent_raw(reference, referent); | ||
static void reference_clear_referent(oop reference) { | ||
java_lang_ref_Reference::clear_referent(reference); | ||
} | ||
|
||
template <typename T> | ||
|
@@ -316,7 +316,7 @@ void ShenandoahReferenceProcessor::make_inactive(oop reference, ReferenceType ty | |
reference_set_next(reference, reference); | ||
} else { | ||
// Clear referent | ||
reference_set_referent(reference, NULL); | ||
reference_clear_referent(reference); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now I am looking at this code and wonder if we could just inline There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to keep the |
||
} | ||
} | ||
|
||
|
@@ -590,4 +590,4 @@ void ShenandoahReferenceProcessor::collect_statistics() { | |
discovered[REF_SOFT], discovered[REF_WEAK], discovered[REF_FINAL], discovered[REF_PHANTOM]); | ||
log_info(gc,ref)("Enqueued references: Soft: " SIZE_FORMAT ", Weak: " SIZE_FORMAT ", Final: " SIZE_FORMAT ", Phantom: " SIZE_FORMAT, | ||
enqueued[REF_SOFT], enqueued[REF_WEAK], enqueued[REF_FINAL], enqueued[REF_PHANTOM]); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if moving this from the
.hpp
to.cpp
has performance implications for callers. Maybe move to.inline.hpp
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for moving it out of the .hpp was of course that the
change to call java_lang_ref_Reference::referent_addr_raw needs to #include
javaClasses.inline.hpp.
I don't see this move having any measureable performance difference, and not
even sure what the sign of any change might be. A better refactoring might
be to package up the common remove/make_referent_alive/move_to_next sequence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it is probably only affects the path that is infrequently taken, i.e. marking through the resurrected referents during weak reference processing, that is probably only finalizers. It is fine to have it in
.cpp
.